gcc-14 Wflex-array-member-not-at-end may-be-ub in struct pthread

gcc-14 Wflex-array-member-not-at-end may-be-ub in struct pthread

[Cristian Rodríguez]

[-- Attachment #1: Type: text/plain, Size: 819 bytes --]

gcc-14 says that

struct pthread {
...
  struct rseq rseq_area;

  /* This member must be last.  */
  char end_padding[]; --> This is incorrect, since struct rseq contains a
flexible array it must be the last member..

}

The current docs say:

"A structure containing a C99 flexible array member, or a union containing
such a structure, is not the last field of another structure, for example:

struct flex  { int length; char data[]; };

struct mid_flex { int m; struct flex flex_data; int n; };

In the above, accessing a member of the array mid_flex.flex_data.data[] might
have undefined behavior. Compilers do not handle such a case consistently.
Any code relying on this case should be modified to ensure that flexible
array members only end up at the ends of structures."


Who is right.. ? is this intentional?

Re: gcc-14 Wflex-array-member-not-at-end may-be-ub in struct pthread

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 853 bytes --]

On 2023-07-05 06:46, Cristian Rodríguez via Libc-alpha wrote:
>    char end_padding[]; --> This is incorrect, since struct rseq contains a
> flexible array it must be the last member..

? struct rseq doesn't contain a flexible array on the master branch, so 
there's no error here.

Perhaps you tried to nest 'struct pthread' inside another struct? If so, 
the C standard doesn't allow that but the attached untested patch should 
be a trivial fix.

Here's the wording in the C23 draft standard section 6.7.2.1 paragraph 
3, which is what GCC is trying to enforce here:

"the last member of a structure with more than one named member may have 
incomplete array type; such a structure (and any union containing, 
possibly recursively, a member that is such a structure) shall not be a 
member of a structure or an element of an array."

[-- Attachment #2: 0001-make-struct-pthread-a-complete-type.patch --]
[-- Type: text/x-patch, Size: 1259 bytes --]

From b4eb0714b3bf6e078314af6bbe5331d8f111b671 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Wed, 5 Jul 2023 08:50:26 -0700
Subject: [PATCH] =?UTF-8?q?make=20=E2=80=98struct=20pthread=E2=80=99=20a?=
 =?UTF-8?q?=20complete=20type?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* nptl/descr.h (struct pthread): Remove end_padding member, which
made this type incomplete.
(PTHREAD_STRUCT_END_PADDING): Stop using end_padding.
---
 nptl/descr.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/nptl/descr.h b/nptl/descr.h
index 746a4b9e4a..d06abd6ad9 100644
--- a/nptl/descr.h
+++ b/nptl/descr.h
@@ -408,11 +408,11 @@ struct pthread
   /* rseq area registered with the kernel.  */
   struct rseq rseq_area;
 
-  /* This member must be last.  */
-  char end_padding[];
-
+  /* Amount of end padding, if any, in this structure.
+     This definition relies on rseq_area being last.  */
 #define PTHREAD_STRUCT_END_PADDING \
-  (sizeof (struct pthread) - offsetof (struct pthread, end_padding))
+  (sizeof (struct pthread) - offsetof (struct pthread, rseq_area) \
+   + sizeof (struct rseq))
 } __attribute ((aligned (TCB_ALIGNMENT)));
 
 static inline bool
-- 
2.39.2

Re: gcc-14 Wflex-array-member-not-at-end may-be-ub in struct pthread

[Cristian Rodríguez]

[-- Attachment #1: Type: text/plain, Size: 1007 bytes --]

On Wed, Jul 5, 2023 at 11:53 AM Paul Eggert <eggert@cs.ucla.edu> wrote:

> On 2023-07-05 06:46, Cristian Rodríguez via Libc-alpha wrote:
> >    char end_padding[]; --> This is incorrect, since struct rseq contains
> a
> > flexible array it must be the last member..
>
> ? struct rseq doesn't contain a flexible array on the master branch, so
> there's no error here.
>

I may be missing something but I have struct rseq containing a final
flexible array..

/*
         * Flexible array member at end of structure, after last feature
field.
         */
        char end[];
}

this happens because:

#ifdef __has_include
# if __has_include ("linux/rseq.h")
#  define __GLIBC_HAVE_KERNEL_RSEQ --> that is defined
# endif


> Perhaps you tried to nest 'struct pthread' inside another struct? If so,
> the C standard doesn't allow that but the attached untested patch should
> be a trivial fix.
>

Your patch does indeed silence this particular warning, thanks.



>
>

Re: gcc-14 Wflex-array-member-not-at-end may-be-ub in struct pthread

[Cristian Rodríguez]

[-- Attachment #1: Type: text/plain, Size: 1287 bytes --]

On Wed, Jul 5, 2023 at 2:13 PM Cristian Rodríguez <cristian@rodriguez.im>
wrote:

>
>
> On Wed, Jul 5, 2023 at 11:53 AM Paul Eggert <eggert@cs.ucla.edu> wrote:
>
>> On 2023-07-05 06:46, Cristian Rodríguez via Libc-alpha wrote:
>> >    char end_padding[]; --> This is incorrect, since struct rseq
>> contains a
>> > flexible array it must be the last member..
>>
>> ? struct rseq doesn't contain a flexible array on the master branch, so
>> there's no error here.
>>
>
> I may be missing something but I have struct rseq containing a final
> flexible array..
>
> /*
>          * Flexible array member at end of structure, after last feature
> field.
>          */
>         char end[];
> }
>
> this happens because:
>
> #ifdef __has_include
> # if __has_include ("linux/rseq.h")
> #  define __GLIBC_HAVE_KERNEL_RSEQ --> that is defined
> # endif
>
>
>> Perhaps you tried to nest 'struct pthread' inside another struct? If so,
>> the C standard doesn't allow that but the attached untested patch should
>> be a trivial fix.
>>
>
> Your patch does indeed silence this particular warning, thanks.
>

I suggest you add this patch to the release queue. because as far as I can
tell, It is UB when the kernel headers are present on the system.

Re: gcc-14 Wflex-array-member-not-at-end may-be-ub in struct pthread

[Adhemerval Zanella Netto]

On 08/07/23 16:21, Cristian Rodríguez via Libc-alpha wrote:
> On Wed, Jul 5, 2023 at 2:13 PM Cristian Rodríguez <cristian@rodriguez.im>
> wrote:
> 
>>
>>
>> On Wed, Jul 5, 2023 at 11:53 AM Paul Eggert <eggert@cs.ucla.edu> wrote:
>>
>>> On 2023-07-05 06:46, Cristian Rodríguez via Libc-alpha wrote:
>>>>    char end_padding[]; --> This is incorrect, since struct rseq
>>> contains a
>>>> flexible array it must be the last member..
>>>
>>> ? struct rseq doesn't contain a flexible array on the master branch, so
>>> there's no error here.
>>>
>>
>> I may be missing something but I have struct rseq containing a final
>> flexible array..
>>
>> /*
>>          * Flexible array member at end of structure, after last feature
>> field.
>>          */
>>         char end[];
>> }
>>
>> this happens because:
>>
>> #ifdef __has_include
>> # if __has_include ("linux/rseq.h")
>> #  define __GLIBC_HAVE_KERNEL_RSEQ --> that is defined
>> # endif
>>
>>
>>> Perhaps you tried to nest 'struct pthread' inside another struct? If so,
>>> the C standard doesn't allow that but the attached untested patch should
>>> be a trivial fix.
>>>
>>
>> Your patch does indeed silence this particular warning, thanks.
>>
> 
> I suggest you add this patch to the release queue. because as far as I can
> tell, It is UB when the kernel headers are present on the system.

The flexible array on rseq was added on linux 6.3, should we treat this as
release blocker?

Re: gcc-14 Wflex-array-member-not-at-end may-be-ub in struct pthread

[Adhemerval Zanella Netto]

On 19/07/23 11:17, Adhemerval Zanella Netto wrote:
> 
> 
> On 08/07/23 16:21, Cristian Rodríguez via Libc-alpha wrote:
>> On Wed, Jul 5, 2023 at 2:13 PM Cristian Rodríguez <cristian@rodriguez.im>
>> wrote:
>>
>>>
>>>
>>> On Wed, Jul 5, 2023 at 11:53 AM Paul Eggert <eggert@cs.ucla.edu> wrote:
>>>
>>>> On 2023-07-05 06:46, Cristian Rodríguez via Libc-alpha wrote:
>>>>>    char end_padding[]; --> This is incorrect, since struct rseq
>>>> contains a
>>>>> flexible array it must be the last member..
>>>>
>>>> ? struct rseq doesn't contain a flexible array on the master branch, so
>>>> there's no error here.
>>>>
>>>
>>> I may be missing something but I have struct rseq containing a final
>>> flexible array..
>>>
>>> /*
>>>          * Flexible array member at end of structure, after last feature
>>> field.
>>>          */
>>>         char end[];
>>> }
>>>
>>> this happens because:
>>>
>>> #ifdef __has_include
>>> # if __has_include ("linux/rseq.h")
>>> #  define __GLIBC_HAVE_KERNEL_RSEQ --> that is defined
>>> # endif
>>>
>>>
>>>> Perhaps you tried to nest 'struct pthread' inside another struct? If so,
>>>> the C standard doesn't allow that but the attached untested patch should
>>>> be a trivial fix.
>>>>
>>>
>>> Your patch does indeed silence this particular warning, thanks.
>>>
>>
>> I suggest you add this patch to the release queue. because as far as I can
>> tell, It is UB when the kernel headers are present on the system.
> 
> The flexible array on rseq was added on linux 6.3, should we treat this as
> release blocker?

Also, glibc currently does not handle AT_RSEQ_FEATURE_SIZE/AT_RSEQ_ALIGN. Is
this a potential issue for newer kernels?

Re: gcc-14 Wflex-array-member-not-at-end may-be-ub in struct pthread

[Mathieu Desnoyers]

On 7/19/23 10:22, Adhemerval Zanella Netto wrote:
> 
> 
> On 19/07/23 11:17, Adhemerval Zanella Netto wrote:
>>
>>
>> On 08/07/23 16:21, Cristian Rodríguez via Libc-alpha wrote:
>>> On Wed, Jul 5, 2023 at 2:13 PM Cristian Rodríguez <cristian@rodriguez.im>
>>> wrote:
>>>
>>>>
>>>>
>>>> On Wed, Jul 5, 2023 at 11:53 AM Paul Eggert <eggert@cs.ucla.edu> wrote:
>>>>
>>>>> On 2023-07-05 06:46, Cristian Rodríguez via Libc-alpha wrote:
>>>>>>     char end_padding[]; --> This is incorrect, since struct rseq
>>>>> contains a
>>>>>> flexible array it must be the last member..
>>>>>
>>>>> ? struct rseq doesn't contain a flexible array on the master branch, so
>>>>> there's no error here.
>>>>>
>>>>
>>>> I may be missing something but I have struct rseq containing a final
>>>> flexible array..
>>>>
>>>> /*
>>>>           * Flexible array member at end of structure, after last feature
>>>> field.
>>>>           */
>>>>          char end[];
>>>> }
>>>>
>>>> this happens because:
>>>>
>>>> #ifdef __has_include
>>>> # if __has_include ("linux/rseq.h")
>>>> #  define __GLIBC_HAVE_KERNEL_RSEQ --> that is defined
>>>> # endif
>>>>
>>>>
>>>>> Perhaps you tried to nest 'struct pthread' inside another struct? If so,
>>>>> the C standard doesn't allow that but the attached untested patch should
>>>>> be a trivial fix.
>>>>>
>>>>
>>>> Your patch does indeed silence this particular warning, thanks.
>>>>
>>>
>>> I suggest you add this patch to the release queue. because as far as I can
>>> tell, It is UB when the kernel headers are present on the system.
>>
>> The flexible array on rseq was added on linux 6.3, should we treat this as
>> release blocker?
> 
> Also, glibc currently does not handle AT_RSEQ_FEATURE_SIZE/AT_RSEQ_ALIGN. Is
> this a potential issue for newer kernels?

I would expect that if a libc expects struct rseq to be fixed-size, it 
should use its own copy of the kernel headers. A libc expecting the 
fixed-size struct rseq still works with newer kernels.

Having libc support the feature size/align AT_ queries for extensible 
RSEQ will become necessary as we use all the 12 bytes of padding that
were present at the end of the original struct rseq for new fields.

So supporting the AT_RSEQ extensible RSEQ should be done sooner than 
later, but I don't see it as a release blocker for libc.

Thanks,

Mathieu

-- 
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com

Re: gcc-14 Wflex-array-member-not-at-end may-be-ub in struct pthread

[Adhemerval Zanella Netto]

On 19/07/23 11:55, Mathieu Desnoyers wrote:
> On 7/19/23 10:22, Adhemerval Zanella Netto wrote:
>>
>>
>> On 19/07/23 11:17, Adhemerval Zanella Netto wrote:
>>>
>>>
>>> On 08/07/23 16:21, Cristian Rodríguez via Libc-alpha wrote:
>>>> On Wed, Jul 5, 2023 at 2:13 PM Cristian Rodríguez <cristian@rodriguez.im>
>>>> wrote:
>>>>
>>>>>
>>>>>
>>>>> On Wed, Jul 5, 2023 at 11:53 AM Paul Eggert <eggert@cs.ucla.edu> wrote:
>>>>>
>>>>>> On 2023-07-05 06:46, Cristian Rodríguez via Libc-alpha wrote:
>>>>>>>     char end_padding[]; --> This is incorrect, since struct rseq
>>>>>> contains a
>>>>>>> flexible array it must be the last member..
>>>>>>
>>>>>> ? struct rseq doesn't contain a flexible array on the master branch, so
>>>>>> there's no error here.
>>>>>>
>>>>>
>>>>> I may be missing something but I have struct rseq containing a final
>>>>> flexible array..
>>>>>
>>>>> /*
>>>>>           * Flexible array member at end of structure, after last feature
>>>>> field.
>>>>>           */
>>>>>          char end[];
>>>>> }
>>>>>
>>>>> this happens because:
>>>>>
>>>>> #ifdef __has_include
>>>>> # if __has_include ("linux/rseq.h")
>>>>> #  define __GLIBC_HAVE_KERNEL_RSEQ --> that is defined
>>>>> # endif
>>>>>
>>>>>
>>>>>> Perhaps you tried to nest 'struct pthread' inside another struct? If so,
>>>>>> the C standard doesn't allow that but the attached untested patch should
>>>>>> be a trivial fix.
>>>>>>
>>>>>
>>>>> Your patch does indeed silence this particular warning, thanks.
>>>>>
>>>>
>>>> I suggest you add this patch to the release queue. because as far as I can
>>>> tell, It is UB when the kernel headers are present on the system.
>>>
>>> The flexible array on rseq was added on linux 6.3, should we treat this as
>>> release blocker?
>>
>> Also, glibc currently does not handle AT_RSEQ_FEATURE_SIZE/AT_RSEQ_ALIGN. Is
>> this a potential issue for newer kernels?
> 
> I would expect that if a libc expects struct rseq to be fixed-size, it should use its own copy of the kernel headers. A libc expecting the fixed-size struct rseq still works with newer kernels.

Since glibc uses the user exported sys/rseq, it ends up using the kernel header if present
for the 'struct rseq' definition.

> 
> Having libc support the feature size/align AT_ queries for extensible RSEQ will become necessary as we use all the 12 bytes of padding that
> were present at the end of the original struct rseq for new fields.

Current we use the 'sizeof (struct rseq)' from struct pthread as the value for __rseq_size,
so it seems that it should be ok regardless whether kernel header is used or not.

The only issue is whether the UB of having a middle struct with flexible array might
trigger any code generation issue.

> 
> So supporting the AT_RSEQ extensible RSEQ should be done sooner than later, but I don't see it as a release blocker for libc.

Re: gcc-14 Wflex-array-member-not-at-end may-be-ub in struct pthread

[Andreas K. Huettel]

[-- Attachment #1: Type: text/plain, Size: 987 bytes --]

Am Mittwoch, 19. Juli 2023, 16:17:50 CEST schrieb Adhemerval Zanella Netto:
> >>
> >> this happens because:
> >>
> >> #ifdef __has_include
> >> # if __has_include ("linux/rseq.h")
> >> #  define __GLIBC_HAVE_KERNEL_RSEQ --> that is defined
> >> # endif
> >>
> >>
> >>> Perhaps you tried to nest 'struct pthread' inside another struct? If so,
> >>> the C standard doesn't allow that but the attached untested patch should
> >>> be a trivial fix.
> >>>
> >>
> >> Your patch does indeed silence this particular warning, thanks.
> >>
> > 
> > I suggest you add this patch to the release queue. because as far as I can
> > tell, It is UB when the kernel headers are present on the system.
> 
> The flexible array on rseq was added on linux 6.3, should we treat this as
> release blocker?
> 

Yes IMHO (unless someone convinces me otherwise).

-- 
Andreas K. Hüttel
dilfridge@gentoo.org
Gentoo Linux developer
(council, toolchain, base-system, perl, libreoffice)

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 981 bytes --]

Re: gcc-14 Wflex-array-member-not-at-end may-be-ub in struct pthread

[Siddhesh Poyarekar]

On 2023-07-19 14:36, Andreas K. Huettel via Libc-alpha wrote:
> Am Mittwoch, 19. Juli 2023, 16:17:50 CEST schrieb Adhemerval Zanella Netto:
>>>>
>>>> this happens because:
>>>>
>>>> #ifdef __has_include
>>>> # if __has_include ("linux/rseq.h")
>>>> #  define __GLIBC_HAVE_KERNEL_RSEQ --> that is defined
>>>> # endif
>>>>
>>>>
>>>>> Perhaps you tried to nest 'struct pthread' inside another struct? If so,
>>>>> the C standard doesn't allow that but the attached untested patch should
>>>>> be a trivial fix.
>>>>>
>>>>
>>>> Your patch does indeed silence this particular warning, thanks.
>>>>
>>>
>>> I suggest you add this patch to the release queue. because as far as I can
>>> tell, It is UB when the kernel headers are present on the system.
>>
>> The flexible array on rseq was added on linux 6.3, should we treat this as
>> release blocker?
>>
> 
> Yes IMHO (unless someone convinces me otherwise).
> 

Paul's patch fixes this by dropping the end_padding flex array and 
fixing up PTHREAD_STRUCT_END_PADDING.  That fix looks correct to me and 
is only really relevant for ia64.  Paul, could you please push your fix?

Thanks,
Sid

Re: gcc-14 Wflex-array-member-not-at-end may-be-ub in struct pthread

[Paul Eggert]

On 2023-07-19 12:04, Siddhesh Poyarekar wrote:
> Paul's patch fixes this by dropping the end_padding flex array and 
> fixing up PTHREAD_STRUCT_END_PADDING.  That fix looks correct to me and 
> is only really relevant for ia64.  Paul, could you please push your fix?

Sure, done.

Re: gcc-14 Wflex-array-member-not-at-end may-be-ub in struct pthread

[Andreas K. Huettel]

[-- Attachment #1: Type: text/plain, Size: 531 bytes --]

Am Mittwoch, 19. Juli 2023, 23:16:50 CEST schrieb Paul Eggert:
> On 2023-07-19 12:04, Siddhesh Poyarekar wrote:
> > Paul's patch fixes this by dropping the end_padding flex array and 
> > fixing up PTHREAD_STRUCT_END_PADDING.  That fix looks correct to me and 
> > is only really relevant for ia64.  Paul, could you please push your fix?
> 
> Sure, done.
> 

Thank you! One more problem resolved.

-- 
Andreas K. Hüttel
dilfridge@gentoo.org
Gentoo Linux developer
(council, toolchain, base-system, perl, libreoffice)

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 981 bytes --]