ToT glibc build problem with ToT GCC

  * 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.
  

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

>