From patchwork Mon May 27 19:04:03 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Florian Weimer X-Patchwork-Id: 32862 Received: (qmail 6584 invoked by alias); 27 May 2019 19:04:09 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 6576 invoked by uid 89); 27 May 2019 19:04:09 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-18.9 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, SPF_HELO_PASS autolearn=ham version=3.3.1 spammy=now, flexible, sore, investigated X-HELO: mx1.redhat.com From: Florian Weimer To: Paul Eggert Cc: libc-alpha@sourceware.org Subject: Re: [PATCH] dlfcn: Avoid one-element flexible array in Dl_serinfo References: <87k1ehzf7o.fsf@oldenburg2.str.redhat.com> <437469fe-7622-9021-0b0c-5e4a1cdb8482@cs.ucla.edu> <8736l4utui.fsf@oldenburg2.str.redhat.com> Date: Mon, 27 May 2019 21:04:03 +0200 In-Reply-To: (Paul Eggert's message of "Fri, 24 May 2019 16:47:44 -0700") Message-ID: <87imtvhg7w.fsf@oldenburg2.str.redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 * Paul Eggert: > On 5/24/19 1:42 AM, Florian Weimer wrote: >> This changes the size of the type and is not source-code-compatible. I >> have not investigated whether the change is still reasonably safe, but >> usually, wo do not make such changes. > > OK, in that case I suggest adding a comment explaining the situation, > since it is a bit of a sore thumb. Something like the following > perhaps? Or if this problem is likely to occur elsewhere, we could > package the situation up into a macro and just use the macro here. > >     /* An array of dls_cnt elements, each of type Dl_serpath.  */ >   #if 0 >     /* With no backward-compatibility concerns we’d use the following >        C99 flexible array member.  However, as this data structure >        predates C99 it had to contain a one-element array here, and we >        don't want to change the struct's size now.  */ >     Dl_serpath dls_serpath[]; >   #elif defined __GNUC__ >     /* Avoid an unwanted array subscript check by the compiler, while >        preserving the size of the type.  */ >     __extension__ union >     { >       Dl_serpath dls_serpath[0]; /* Actually longer, dls_cnt elements.  */ >       Dl_serpath __dls_serpath_pad[1]; >     }; >   #else >     Dl_serpath dls_serpath[1]; >   #endif The real issue here is that GNU C allows nested flexible array members, but not in unions. I still think this is best discussed in the commit message because such header comments tend not to age well, but I've proposed a patch below. Thanks, Florian dlfcn: Avoid one-element flexible array in Dl_serinfo The dls_serpath path field, as an array of length 1, introduces unexpected array subscript checks with some compilers. 2019-05-27 Florian Weimer [BZ #24166] * dlfcn/dlfcn.h (Dl_serinfo): Do not use array of length 1 for dls_serpath field. diff --git a/dlfcn/dlfcn.h b/dlfcn/dlfcn.h index 896ad6fc9b..7107b3ea9a 100644 --- a/dlfcn/dlfcn.h +++ b/dlfcn/dlfcn.h @@ -180,7 +180,19 @@ typedef struct { size_t dls_size; /* Size in bytes of the whole buffer. */ unsigned int dls_cnt; /* Number of elements in `dls_serpath'. */ +# ifdef __GNUC__ + /* The zero-length array avoids an unwanted array subscript check by + the compiler, while the surrounding anonymous union preserves the + historic size of the type. At the time of writing, GNU C does + not support structs with flexible array members in unions. */ + __extension__ union + { + Dl_serpath dls_serpath[0]; /* Actually longer, dls_cnt elements. */ + Dl_serpath __dls_serpath_pad[1]; + }; +# else /* !__GNUC__ */ Dl_serpath dls_serpath[1]; /* Actually longer, dls_cnt elements. */ +# endif /* !__GNUC__ */ } Dl_serinfo; #endif /* __USE_GNU */