Message ID | 87lfv5sn6h.fsf@oldenburg2.str.redhat.com |
---|---|
State | Committed |
Commit | 1471fa556afb428c4a4c46cf5543a4101d5bcf91 |
Headers |
Received: (qmail 91707 invoked by alias); 3 Sep 2019 10:13:04 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <libc-alpha.sourceware.org> List-Unsubscribe: <mailto:libc-alpha-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:libc-alpha-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 91668 invoked by uid 89); 3 Sep 2019 10:13:03 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-18.5 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=strange, H*i:sk:4679ba6, H*f:sk:4679ba6 X-HELO: mx1.redhat.com From: Florian Weimer <fweimer@redhat.com> To: Szabolcs Nagy <Szabolcs.Nagy@arm.com> Cc: nd <nd@arm.com>, Carlos O'Donell <carlos@redhat.com>, Martin Sebor <msebor@gmail.com>, Steve Ellcey <sellcey@marvell.com>, "libc-alpha\@sourceware.org" <libc-alpha@sourceware.org>, "msebor\@redhat.com" <msebor@redhat.com> Subject: Re: ToT glibc build problem with ToT GCC References: <486309d08583ed1c27a001d946205850b421f7ad.camel@marvell.com> <53b36206-3f07-432b-9d0f-02520debe4b8@gmail.com> <d96c9e5d-18f6-9c55-cd49-334c3d449a5b@redhat.com> <2e5c0137-4109-e7ce-8d1d-9c268e086f81@gmail.com> <f67a431b-0d15-644a-c5cd-8cf2ccf8e270@redhat.com> <87ef126bvu.fsf@oldenburg2.str.redhat.com> <ba965603-cc2a-3c53-4a1b-9edaf356f283@arm.com> <87mufq4og4.fsf@oldenburg2.str.redhat.com> <4679ba62-85c2-cc52-1a9d-5c75b3005344@arm.com> Date: Tue, 03 Sep 2019 12:12:54 +0200 In-Reply-To: <4679ba62-85c2-cc52-1a9d-5c75b3005344@arm.com> (Szabolcs Nagy's message of "Tue, 3 Sep 2019 09:51:55 +0000") Message-ID: <87lfv5sn6h.fsf@oldenburg2.str.redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain |
Commit Message
Florian Weimer
Sept. 3, 2019, 10:12 a.m. UTC
* Szabolcs Nagy: > On 30/08/2019 17:18, Florian Weimer wrote: >> >> localedef: Use initializer for flexible array member [BZ #24950] >> > > i would mention that you put the struct in .rodata, > that is a non-obvious change. Please see below. > i'm fine with using gcc extension for the initializer. > i think the union solution is a bit safer though. > either way, it would be nice to fix this build error. I don't think this is a GCC extension. I believe it's part of C99. Thanks, Florian localedef: Use initializer for flexible array member [BZ #24950] struct charseq used a zero-length array instead of a flexible array member. This required a strange construct to initialize struct charseq objects, and GCC 10 warns about that: cc1: error: writing 1 byte into a region of size 0 [-Werror=stringop-overflow=] In file included from programs/repertoire.h:24, from programs/localedef.h:32, from programs/ld-ctype.c:35: programs/charmap.h:63:17: note: destination object declared here 63 | unsigned char bytes[0]; | ^~~~~ cc1: error: writing 1 byte into a region of size 0 [-Werror=stringop-overflow=] programs/charmap.h:63:17: note: destination object declared here cc1: error: writing 1 byte into a region of size 0 [-Werror=stringop-overflow=] programs/charmap.h:63:17: note: destination object declared here cc1: error: writing 1 byte into a region of size 0 [-Werror=stringop-overflow=] programs/charmap.h:63:17: note: destination object declared here The change makes the object physically const, but it is not expected to be modified. 2019-08-30 Florian Weimer <fweimer@redhat.com> [BZ #24950] * locale/programs/charmap.h (struct charseq): Turn bytes into a flexible array member. * locale/programs/ld-ctype.c (ctype_finish): Use initializer for replace.
Comments
On 03/09/2019 11:12, Florian Weimer wrote: > * Szabolcs Nagy: > >> On 30/08/2019 17:18, Florian Weimer wrote: >>> >>> localedef: Use initializer for flexible array member [BZ #24950] >>> >> >> i would mention that you put the struct in .rodata, >> that is a non-obvious change. > > Please see below. > >> i'm fine with using gcc extension for the initializer. >> i think the union solution is a bit safer though. >> either way, it would be nice to fix this build error. > > I don't think this is a GCC extension. I believe it's part of C99. iso c has this example: http://port70.net/~nsz/c/c11/n1570.html#6.7.2.1p21 so the initializer relies on a gcc extension. > > Thanks, > Florian > > localedef: Use initializer for flexible array member [BZ #24950] > > struct charseq used a zero-length array instead of a flexible array > member. This required a strange construct to initialize struct > charseq objects, and GCC 10 warns about that: > > cc1: error: writing 1 byte into a region of size 0 [-Werror=stringop-overflow=] > In file included from programs/repertoire.h:24, > from programs/localedef.h:32, > from programs/ld-ctype.c:35: > programs/charmap.h:63:17: note: destination object declared here > 63 | unsigned char bytes[0]; > | ^~~~~ > cc1: error: writing 1 byte into a region of size 0 [-Werror=stringop-overflow=] > programs/charmap.h:63:17: note: destination object declared here > cc1: error: writing 1 byte into a region of size 0 [-Werror=stringop-overflow=] > programs/charmap.h:63:17: note: destination object declared here > cc1: error: writing 1 byte into a region of size 0 [-Werror=stringop-overflow=] > programs/charmap.h:63:17: note: destination object declared here > > The change makes the object physically const, but it is not expected > to be modified. > > 2019-08-30 Florian Weimer <fweimer@redhat.com> > > [BZ #24950] > * locale/programs/charmap.h (struct charseq): Turn bytes into a > flexible array member. > * locale/programs/ld-ctype.c (ctype_finish): Use initializer for > replace. > > diff --git a/locale/programs/charmap.h b/locale/programs/charmap.h > index 870a9e9577..70db330d29 100644 > --- a/locale/programs/charmap.h > +++ b/locale/programs/charmap.h > @@ -60,7 +60,7 @@ struct charseq > const char *name; > uint32_t ucs4; > int nbytes; > - unsigned char bytes[0]; > + unsigned char bytes[]; > }; > > > diff --git a/locale/programs/ld-ctype.c b/locale/programs/ld-ctype.c > index cfc9c43fd5..9123f64a56 100644 > --- a/locale/programs/ld-ctype.c > +++ b/locale/programs/ld-ctype.c > @@ -842,8 +842,6 @@ no input digits defined and none of the standard names in the charmap")); > for (cnt = 0; cnt < 10; ++cnt) > if (ctype->mboutdigits[cnt] == NULL) > { > - static struct charseq replace[2]; > - > if (!warned) > { > record_error (0, 0, _("\ > @@ -851,10 +849,12 @@ not all characters used in `outdigit' are available in the charmap")); > warned = 1; > } > > - replace[0].nbytes = 1; > - replace[0].bytes[0] = '?'; > - replace[0].bytes[1] = '\0'; > - ctype->mboutdigits[cnt] = &replace[0]; > + static const struct charseq replace = > + { > + .nbytes = 1, > + .bytes = "?", > + }; > + ctype->mboutdigits[cnt] = (struct charseq *) &replace; > } > > warned = 0; >
On Sep 03 2019, Szabolcs Nagy <Szabolcs.Nagy@arm.com> wrote: > On 03/09/2019 11:12, Florian Weimer wrote: >> * Szabolcs Nagy: >> >>> On 30/08/2019 17:18, Florian Weimer wrote: >>>> >>>> localedef: Use initializer for flexible array member [BZ #24950] >>>> >>> >>> i would mention that you put the struct in .rodata, >>> that is a non-obvious change. >> >> Please see below. >> >>> i'm fine with using gcc extension for the initializer. >>> i think the union solution is a bit safer though. >>> either way, it would be nice to fix this build error. >> >> I don't think this is a GCC extension. I believe it's part of C99. > > iso c has this example: > > http://port70.net/~nsz/c/c11/n1570.html#6.7.2.1p21 > > so the initializer relies on a gcc extension. Which is documented in (gcc) Zero Length. Andreas.
* Szabolcs Nagy: > On 03/09/2019 11:12, Florian Weimer wrote: >> * Szabolcs Nagy: >> >>> On 30/08/2019 17:18, Florian Weimer wrote: >>>> >>>> localedef: Use initializer for flexible array member [BZ #24950] >>>> >>> >>> i would mention that you put the struct in .rodata, >>> that is a non-obvious change. >> >> Please see below. >> >>> i'm fine with using gcc extension for the initializer. >>> i think the union solution is a bit safer though. >>> either way, it would be nice to fix this build error. >> >> I don't think this is a GCC extension. I believe it's part of C99. > > iso c has this example: > > http://port70.net/~nsz/c/c11/n1570.html#6.7.2.1p21 > > so the initializer relies on a gcc extension. Okay, I stand corrected. What about the updated commit message I sent earlier? Shall I push this? Thanks, Florian
On 03/09/2019 11:53, Florian Weimer wrote: > * Szabolcs Nagy: > >> On 03/09/2019 11:12, Florian Weimer wrote: >>> * Szabolcs Nagy: >>> >>>> On 30/08/2019 17:18, Florian Weimer wrote: >>>>> >>>>> localedef: Use initializer for flexible array member [BZ #24950] >>>>> >>>> >>>> i would mention that you put the struct in .rodata, >>>> that is a non-obvious change. >>> >>> Please see below. >>> >>>> i'm fine with using gcc extension for the initializer. >>>> i think the union solution is a bit safer though. >>>> either way, it would be nice to fix this build error. >>> >>> I don't think this is a GCC extension. I believe it's part of C99. >> >> iso c has this example: >> >> http://port70.net/~nsz/c/c11/n1570.html#6.7.2.1p21 >> >> so the initializer relies on a gcc extension. > > Okay, I stand corrected. > > What about the updated commit message I sent earlier? Shall I push > this? i think the patch is ok as posted. > > Thanks, > Florian >
diff --git a/locale/programs/charmap.h b/locale/programs/charmap.h index 870a9e9577..70db330d29 100644 --- a/locale/programs/charmap.h +++ b/locale/programs/charmap.h @@ -60,7 +60,7 @@ struct charseq const char *name; uint32_t ucs4; int nbytes; - unsigned char bytes[0]; + unsigned char bytes[]; }; diff --git a/locale/programs/ld-ctype.c b/locale/programs/ld-ctype.c index cfc9c43fd5..9123f64a56 100644 --- a/locale/programs/ld-ctype.c +++ b/locale/programs/ld-ctype.c @@ -842,8 +842,6 @@ no input digits defined and none of the standard names in the charmap")); for (cnt = 0; cnt < 10; ++cnt) if (ctype->mboutdigits[cnt] == NULL) { - static struct charseq replace[2]; - if (!warned) { record_error (0, 0, _("\ @@ -851,10 +849,12 @@ not all characters used in `outdigit' are available in the charmap")); warned = 1; } - replace[0].nbytes = 1; - replace[0].bytes[0] = '?'; - replace[0].bytes[1] = '\0'; - ctype->mboutdigits[cnt] = &replace[0]; + static const struct charseq replace = + { + .nbytes = 1, + .bytes = "?", + }; + ctype->mboutdigits[cnt] = (struct charseq *) &replace; } warned = 0;