bcache: Avoid -Wflex-array-member-not-at-end warning

[PATCH][next] bcache: Avoid -Wflex-array-member-not-at-end warning

Posted by Gustavo A. R. Silva 2 months, 4 weeks ago

-Wflex-array-member-not-at-end was introduced in GCC-14, and we are
getting ready to enable it, globally.

Use the new TRAILING_OVERLAP() helper to fix the following warning:

drivers/md/bcache/bset.h:330:27: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]

This helper creates a union between a flexible-array member (FAM) and a
set of MEMBERS that would otherwise follow it.

This overlays the trailing MEMBER struct btree_iter_set stack_data[MAX_BSETS];
onto the FAM struct btree_iter::data[], while keeping the FAM and the start
of MEMBER aligned.

The static_assert() ensures this alignment remains, and it's
intentionally placed inmediately after the corresponding structures --no
blank line in between.

Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
---
 drivers/md/bcache/bset.h | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/md/bcache/bset.h b/drivers/md/bcache/bset.h
index 011f6062c4c0..6ee2c6a506a2 100644
--- a/drivers/md/bcache/bset.h
+++ b/drivers/md/bcache/bset.h
@@ -327,9 +327,13 @@ struct btree_iter {
 /* Fixed-size btree_iter that can be allocated on the stack */
 
 struct btree_iter_stack {
-	struct btree_iter iter;
-	struct btree_iter_set stack_data[MAX_BSETS];
+	/* Must be last as it ends in a flexible-array member. */
+	TRAILING_OVERLAP(struct btree_iter, iter, data,
+		struct btree_iter_set stack_data[MAX_BSETS];
+	);
 };
+static_assert(offsetof(struct btree_iter_stack, iter.data) ==
+	      offsetof(struct btree_iter_stack, stack_data));
 
 typedef bool (*ptr_filter_fn)(struct btree_keys *b, const struct bkey *k);
 
-- 
2.43.0

Re: [PATCH][next] bcache: Avoid -Wflex-array-member-not-at-end warning

Posted by Coly Li 2 months, 4 weeks ago

On Mon, Nov 10, 2025 at 07:58:58PM +0800, Gustavo A. R. Silva wrote:
> -Wflex-array-member-not-at-end was introduced in GCC-14, and we are
> getting ready to enable it, globally.
> 
> Use the new TRAILING_OVERLAP() helper to fix the following warning:
> 
> drivers/md/bcache/bset.h:330:27: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
> 
> This helper creates a union between a flexible-array member (FAM) and a
> set of MEMBERS that would otherwise follow it.
> 
> This overlays the trailing MEMBER struct btree_iter_set stack_data[MAX_BSETS];
> onto the FAM struct btree_iter::data[], while keeping the FAM and the start
> of MEMBER aligned.
> 
> The static_assert() ensures this alignment remains, and it's
> intentionally placed inmediately after the corresponding structures --no
> blank line in between.
> 
> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> ---
>  drivers/md/bcache/bset.h | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/md/bcache/bset.h b/drivers/md/bcache/bset.h
> index 011f6062c4c0..6ee2c6a506a2 100644
> --- a/drivers/md/bcache/bset.h
> +++ b/drivers/md/bcache/bset.h
> @@ -327,9 +327,13 @@ struct btree_iter {
>  /* Fixed-size btree_iter that can be allocated on the stack */
>  
>  struct btree_iter_stack {
> -	struct btree_iter iter;
> -	struct btree_iter_set stack_data[MAX_BSETS];
> +	/* Must be last as it ends in a flexible-array member. */
> +	TRAILING_OVERLAP(struct btree_iter, iter, data,
> +		struct btree_iter_set stack_data[MAX_BSETS];
> +	);
>  };
> +static_assert(offsetof(struct btree_iter_stack, iter.data) ==
> +	      offsetof(struct btree_iter_stack, stack_data));
> 

I have to say this is ugly. Not the patch, but the gcc 14 warning option
of such coding style. Look at TRAILING_OVERLAP() usage here, this is not
C, this is something to fix a gcc bug which cannot handle FAM properly.

Gustavo, this complain is not to you, just I feel a bit sad how GCC makes
the code comes to such an ugly way, and it makes things much complicated.
For anyone doesn't have deep understanding of TRAILING_OVERLAP(), I
highly suspect whether he or she can understand what happens here.

Andrew and Gustavo, is this a mandatary to fix FAM in such way? If yes
I take the patch and keep my own opinion. If not, I'd like to see gcc
fixes its bug, for the this code I don't see the author does things
wrong.

Coly Li

Re: [PATCH][next] bcache: Avoid -Wflex-array-member-not-at-end warning

Posted by Gustavo A. R. Silva 2 months, 4 weeks ago


On 11/11/25 22:17, Coly Li wrote:
> On Mon, Nov 10, 2025 at 07:58:58PM +0800, Gustavo A. R. Silva wrote:
>> -Wflex-array-member-not-at-end was introduced in GCC-14, and we are
>> getting ready to enable it, globally.
>>
>> Use the new TRAILING_OVERLAP() helper to fix the following warning:
>>
>> drivers/md/bcache/bset.h:330:27: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
>>
>> This helper creates a union between a flexible-array member (FAM) and a
>> set of MEMBERS that would otherwise follow it.
>>
>> This overlays the trailing MEMBER struct btree_iter_set stack_data[MAX_BSETS];
>> onto the FAM struct btree_iter::data[], while keeping the FAM and the start
>> of MEMBER aligned.
>>
>> The static_assert() ensures this alignment remains, and it's
>> intentionally placed inmediately after the corresponding structures --no
>> blank line in between.
>>
>> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
>> ---
>>   drivers/md/bcache/bset.h | 8 ++++++--
>>   1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/md/bcache/bset.h b/drivers/md/bcache/bset.h
>> index 011f6062c4c0..6ee2c6a506a2 100644
>> --- a/drivers/md/bcache/bset.h
>> +++ b/drivers/md/bcache/bset.h
>> @@ -327,9 +327,13 @@ struct btree_iter {
>>   /* Fixed-size btree_iter that can be allocated on the stack */
>>   
>>   struct btree_iter_stack {
>> -	struct btree_iter iter;
>> -	struct btree_iter_set stack_data[MAX_BSETS];
>> +	/* Must be last as it ends in a flexible-array member. */
>> +	TRAILING_OVERLAP(struct btree_iter, iter, data,
>> +		struct btree_iter_set stack_data[MAX_BSETS];
>> +	);
>>   };
>> +static_assert(offsetof(struct btree_iter_stack, iter.data) ==
>> +	      offsetof(struct btree_iter_stack, stack_data));
>>
> 
> I have to say this is ugly. Not the patch, but the gcc 14 warning option
> of such coding style. Look at TRAILING_OVERLAP() usage here, this is not
> C, this is something to fix a gcc bug which cannot handle FAM properly.

This is not a GCC bug.

> 
> Gustavo, this complain is not to you, just I feel a bit sad how GCC makes
> the code comes to such an ugly way, and it makes things much complicated.
> For anyone doesn't have deep understanding of TRAILING_OVERLAP(), I
> highly suspect whether he or she can understand what happens here.
> 
> Andrew and Gustavo, is this a mandatary to fix FAM in such way? If yes
> I take the patch and keep my own opinion. If not, I'd like to see gcc
> fixes its bug, for the this code I don't see the author does things
> wrong.

This is a false positive that needs to be addressed in some way in order to
enable -Wflex-array-member-not-at-end in mainline.

Here you can take a look at the patches I (and others) have submitted to
modify similar code over the last year:

https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/log/?qt=grep&q=-Wflex-array-member-not-at-end

Thanks
-Gustavo

Re: [PATCH][next] bcache: Avoid -Wflex-array-member-not-at-end warning

Posted by Coly Li 2 months, 4 weeks ago

On Tue, Nov 11, 2025 at 10:28:57PM +0800, Gustavo A. R. Silva wrote:
> 
> 
> On 11/11/25 22:17, Coly Li wrote:
> > On Mon, Nov 10, 2025 at 07:58:58PM +0800, Gustavo A. R. Silva wrote:
> > > -Wflex-array-member-not-at-end was introduced in GCC-14, and we are
> > > getting ready to enable it, globally.
> > > 
> > > Use the new TRAILING_OVERLAP() helper to fix the following warning:
> > > 
> > > drivers/md/bcache/bset.h:330:27: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
> > > 
> > > This helper creates a union between a flexible-array member (FAM) and a
> > > set of MEMBERS that would otherwise follow it.
> > > 
> > > This overlays the trailing MEMBER struct btree_iter_set stack_data[MAX_BSETS];
> > > onto the FAM struct btree_iter::data[], while keeping the FAM and the start
> > > of MEMBER aligned.
> > > 
> > > The static_assert() ensures this alignment remains, and it's
> > > intentionally placed inmediately after the corresponding structures --no
> > > blank line in between.
> > > 
> > > Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> > > ---
> > >   drivers/md/bcache/bset.h | 8 ++++++--
> > >   1 file changed, 6 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/md/bcache/bset.h b/drivers/md/bcache/bset.h
> > > index 011f6062c4c0..6ee2c6a506a2 100644
> > > --- a/drivers/md/bcache/bset.h
> > > +++ b/drivers/md/bcache/bset.h
> > > @@ -327,9 +327,13 @@ struct btree_iter {
> > >   /* Fixed-size btree_iter that can be allocated on the stack */
> > >   struct btree_iter_stack {
> > > -	struct btree_iter iter;
> > > -	struct btree_iter_set stack_data[MAX_BSETS];
> > > +	/* Must be last as it ends in a flexible-array member. */
> > > +	TRAILING_OVERLAP(struct btree_iter, iter, data,
> > > +		struct btree_iter_set stack_data[MAX_BSETS];
> > > +	);
> > >   };
> > > +static_assert(offsetof(struct btree_iter_stack, iter.data) ==
> > > +	      offsetof(struct btree_iter_stack, stack_data));
> > > 
> > 
> > I have to say this is ugly. Not the patch, but the gcc 14 warning option
> > of such coding style. Look at TRAILING_OVERLAP() usage here, this is not
> > C, this is something to fix a gcc bug which cannot handle FAM properly.
> 
> This is not a GCC bug.
> 
> > 
> > Gustavo, this complain is not to you, just I feel a bit sad how GCC makes
> > the code comes to such an ugly way, and it makes things much complicated.
> > For anyone doesn't have deep understanding of TRAILING_OVERLAP(), I
> > highly suspect whether he or she can understand what happens here.
> > 
> > Andrew and Gustavo, is this a mandatary to fix FAM in such way? If yes
> > I take the patch and keep my own opinion. If not, I'd like to see gcc
> > fixes its bug, for the this code I don't see the author does things
> > wrong.
> 
> This is a false positive that needs to be addressed in some way in order to
> enable -Wflex-array-member-not-at-end in mainline.
> 
> Here you can take a look at the patches I (and others) have submitted to
> modify similar code over the last year:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/log/?qt=grep&q=-Wflex-array-member-not-at-end
> 

I see. I take this patch, with the above complain...

Coly Li

Re: [PATCH][next] bcache: Avoid -Wflex-array-member-not-at-end warning

Posted by Gustavo A. R. Silva 2 months, 3 weeks ago

> I see. I take this patch, with the above complain...

Thanks, Coly.

-Gustavo

Re: [PATCH][next] bcache: Avoid -Wflex-array-member-not-at-end warning

Posted by Gustavo A. R. Silva 2 months, 2 weeks ago

On 11/13/25 13:24, Gustavo A. R. Silva wrote:
> 
>> I see. I take this patch, with the above complain...
> 
> Thanks, Coly.

BTW, I'm currently looking into the following struct, and I wonder if
it's okay to move struct bkey end; to the end.

drivers/md/bcache/bset.h:157:struct bset_tree {
drivers/md/bcache/bset.h-158-   /*
drivers/md/bcache/bset.h-159-    * We construct a binary tree in an array as if the array
drivers/md/bcache/bset.h-160-    * started at 1, so that things line up on the same cachelines
drivers/md/bcache/bset.h-161-    * better: see comments in bset.c at cacheline_to_bkey() for
drivers/md/bcache/bset.h-162-    * details
drivers/md/bcache/bset.h-163-    */
drivers/md/bcache/bset.h-164-
drivers/md/bcache/bset.h-165-   /* size of the binary tree and prev array */
drivers/md/bcache/bset.h-166-   unsigned int            size;
drivers/md/bcache/bset.h-167-
drivers/md/bcache/bset.h-168-   /* function of size - precalculated for to_inorder() */
drivers/md/bcache/bset.h-169-   unsigned int            extra;
drivers/md/bcache/bset.h-170-
drivers/md/bcache/bset.h-171-   /* copy of the last key in the set */
drivers/md/bcache/bset.h-172-   struct bkey             end;
drivers/md/bcache/bset.h-173-   struct bkey_float       *tree;
drivers/md/bcache/bset.h-174-
drivers/md/bcache/bset.h-175-   /*
drivers/md/bcache/bset.h-176-    * The nodes in the bset tree point to specific keys - this
drivers/md/bcache/bset.h-177-    * array holds the sizes of the previous key.
drivers/md/bcache/bset.h-178-    *
drivers/md/bcache/bset.h-179-    * Conceptually it's a member of struct bkey_float, but we want
drivers/md/bcache/bset.h-180-    * to keep bkey_float to 4 bytes and prev isn't used in the fast
drivers/md/bcache/bset.h-181-    * path.
drivers/md/bcache/bset.h-182-    */
drivers/md/bcache/bset.h-183-   uint8_t                 *prev;
drivers/md/bcache/bset.h-184-
drivers/md/bcache/bset.h-185-   /* The actual btree node, with pointers to each sorted set */
drivers/md/bcache/bset.h-186-   struct bset             *data;
drivers/md/bcache/bset.h-187-};

Something like this:

diff --git a/drivers/md/bcache/bset.h b/drivers/md/bcache/bset.h
index 6ee2c6a506a2..e6daeb0edc02 100644
--- a/drivers/md/bcache/bset.h
+++ b/drivers/md/bcache/bset.h
@@ -169,7 +169,6 @@ struct bset_tree {
         unsigned int            extra;

         /* copy of the last key in the set */
-       struct bkey             end;
         struct bkey_float       *tree;

         /*
@@ -184,6 +183,9 @@ struct bset_tree {

         /* The actual btree node, with pointers to each sorted set */
         struct bset             *data;
+
+       /* Must be last as it ends in a flexible-array member. */
+       struct bkey             end;
  };

This would fix 15 more -Wflex-array-member-not-at-end warnings.

Thanks
-Gustavo