pwm: cros-ec: Avoid -Wflex-array-member-not-at-end warnings

[PATCH][next] pwm: cros-ec: Avoid -Wflex-array-member-not-at-end warnings

Posted by Gustavo A. R. Silva 1 month, 3 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 warnings:

drivers/pwm/pwm-cros-ec.c:53:40: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
drivers/pwm/pwm-cros-ec.c:87:40: 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 members onto the FAM while preserving the original
memory layout.

Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
---
 drivers/pwm/pwm-cros-ec.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/pwm/pwm-cros-ec.c b/drivers/pwm/pwm-cros-ec.c
index 189301dc395e..67cfa17f58e0 100644
--- a/drivers/pwm/pwm-cros-ec.c
+++ b/drivers/pwm/pwm-cros-ec.c
@@ -49,10 +49,9 @@ static int cros_ec_pwm_set_duty(struct cros_ec_pwm_device *ec_pwm, u8 index,
 				u16 duty)
 {
 	struct cros_ec_device *ec = ec_pwm->ec;
-	struct {
-		struct cros_ec_command msg;
+	TRAILING_OVERLAP(struct cros_ec_command, msg, data,
 		struct ec_params_pwm_set_duty params;
-	} __packed buf;
+	) __packed buf;
 	struct ec_params_pwm_set_duty *params = &buf.params;
 	struct cros_ec_command *msg = &buf.msg;
 	int ret;
@@ -83,13 +82,12 @@ static int cros_ec_pwm_set_duty(struct cros_ec_pwm_device *ec_pwm, u8 index,
 
 static int cros_ec_pwm_get_duty(struct cros_ec_device *ec, bool use_pwm_type, u8 index)
 {
-	struct {
-		struct cros_ec_command msg;
+	TRAILING_OVERLAP(struct cros_ec_command, msg, data,
 		union {
 			struct ec_params_pwm_get_duty params;
 			struct ec_response_pwm_get_duty resp;
 		};
-	} __packed buf;
+	) __packed buf;
 	struct ec_params_pwm_get_duty *params = &buf.params;
 	struct ec_response_pwm_get_duty *resp = &buf.resp;
 	struct cros_ec_command *msg = &buf.msg;
-- 
2.43.0

Re: [PATCH][next] pwm: cros-ec: Avoid -Wflex-array-member-not-at-end warnings

Posted by Uwe Kleine-König 1 month, 3 weeks ago

Hello,

On Tue, Aug 12, 2025 at 11:35:41PM +0900, 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 warnings:
> 
> drivers/pwm/pwm-cros-ec.c:53:40: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
> drivers/pwm/pwm-cros-ec.c:87:40: 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 members onto the FAM while preserving the original
> memory layout.
> 
> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> ---
>  drivers/pwm/pwm-cros-ec.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pwm/pwm-cros-ec.c b/drivers/pwm/pwm-cros-ec.c
> index 189301dc395e..67cfa17f58e0 100644
> --- a/drivers/pwm/pwm-cros-ec.c
> +++ b/drivers/pwm/pwm-cros-ec.c
> @@ -49,10 +49,9 @@ static int cros_ec_pwm_set_duty(struct cros_ec_pwm_device *ec_pwm, u8 index,
>  				u16 duty)
>  {
>  	struct cros_ec_device *ec = ec_pwm->ec;
> -	struct {
> -		struct cros_ec_command msg;
> +	TRAILING_OVERLAP(struct cros_ec_command, msg, data,

It's a bit ugly to have to pass the name of the flexible array member.

I think the following would work:

diff --git a/include/linux/stddef.h b/include/linux/stddef.h
index dab49e2ec8c0..8ca9df87a523 100644
--- a/include/linux/stddef.h
+++ b/include/linux/stddef.h
@@ -108,7 +108,7 @@ enum {
 	union {									\
 		TYPE NAME;							\
 		struct {							\
-			unsigned char __offset_to_##FAM[offsetof(TYPE, FAM)];	\
+			unsigned char __offset_to_##FAM[sizeof(TYPE)];		\
 			MEMBERS							\
 		};								\
 	}

which only leaves one usage of FAM in the name of the padding struct
member. I'm sure someone is able to come up with something nice here to
get rid of FAM completely or point out what I'm missing.

Best regards
Uwe

Re: [PATCH][next] pwm: cros-ec: Avoid -Wflex-array-member-not-at-end warnings

Posted by Gustavo A. R. Silva 1 month, 3 weeks ago

> diff --git a/include/linux/stddef.h b/include/linux/stddef.h
> index dab49e2ec8c0..8ca9df87a523 100644
> --- a/include/linux/stddef.h
> +++ b/include/linux/stddef.h
> @@ -108,7 +108,7 @@ enum {
>   	union {									\
>   		TYPE NAME;							\
>   		struct {							\
> -			unsigned char __offset_to_##FAM[offsetof(TYPE, FAM)];	\
> +			unsigned char __offset_to_##FAM[sizeof(TYPE)];		\
>   			MEMBERS							\
>   		};								\
>   	}
> 
> which only leaves one usage of FAM in the name of the padding struct
> member. I'm sure someone is able to come up with something nice here to
> get rid of FAM completely or point out what I'm missing.

Flexible structures (structs that contain a FAM) may have trailing padding.
Under that scenario sizeof(TYPE) causes the overlay between FAM and MEMBERS
to be misaligned.

On the other hand, offsetof(TYPE, FAM) precisely positions the trailing
MEMBERS where the FAM begins, which is correct and safe.

Thanks
-Gustavo

Re: [PATCH][next] pwm: cros-ec: Avoid -Wflex-array-member-not-at-end warnings

Posted by Uwe Kleine-König 1 month, 3 weeks ago

On Thu, Aug 14, 2025 at 08:48:23PM +0900, Gustavo A. R. Silva wrote:
> 
> > diff --git a/include/linux/stddef.h b/include/linux/stddef.h
> > index dab49e2ec8c0..8ca9df87a523 100644
> > --- a/include/linux/stddef.h
> > +++ b/include/linux/stddef.h
> > @@ -108,7 +108,7 @@ enum {
> >   	union {									\
> >   		TYPE NAME;							\
> >   		struct {							\
> > -			unsigned char __offset_to_##FAM[offsetof(TYPE, FAM)];	\
> > +			unsigned char __offset_to_##FAM[sizeof(TYPE)];		\
> >   			MEMBERS							\
> >   		};								\
> >   	}
> > 
> > which only leaves one usage of FAM in the name of the padding struct
> > member. I'm sure someone is able to come up with something nice here to
> > get rid of FAM completely or point out what I'm missing.
> 
> Flexible structures (structs that contain a FAM) may have trailing padding.
> Under that scenario sizeof(TYPE) causes the overlay between FAM and MEMBERS
> to be misaligned.

That sounds wrong to me; are you sure? In that case allocating space for
such a struct using

	struct mystruct {
		unsigned short len;
		unsigned int array[];
	};

	s = malloc(sizeof(struct mystruct) + n * sizeof(unsigned int));

wouldn't do the right thing. 

I found in the net (e.g.
https://rgambord.github.io/c99-doc/sections/6/7/2/1/index.html):

	In most situations, the flexible array member is ignored. In
	particular, the size of the structure is as if the flexible
	array member were omitted except that it may have more trailing
	padding than the omission would imply.

So I'd claim that sizeof does work here as intended.

gcc here also behaves fine:

	uwe@taurus:~$ cat test.c
	#include <stdio.h>

	struct mystruct {
		unsigned short len;
		unsigned int array[];
	};

	struct mystruct2 {
		unsigned short len;
	};

	int main()
	{
		printf("sizeof(struct mystruct) = %zu\n", sizeof(struct mystruct));
		printf("sizeof(struct mystruct2) = %zu\n", sizeof(struct mystruct2));
		return 0;
	}

	uwe@taurus:~$ make test
	cc    -c -o test.o test.c
	cc   test.o   -o test

	uwe@taurus:~$ ./test
	sizeof(struct mystruct) = 4
	sizeof(struct mystruct2) = 2

Best regards
Uwe

Re: [PATCH][next] pwm: cros-ec: Avoid -Wflex-array-member-not-at-end warnings

Posted by Uwe Kleine-König 2 weeks, 5 days ago

Hello,

On Thu, Aug 14, 2025 at 04:08:57PM +0200, Uwe Kleine-König wrote:
> On Thu, Aug 14, 2025 at 08:48:23PM +0900, Gustavo A. R. Silva wrote:
> > 
> > > diff --git a/include/linux/stddef.h b/include/linux/stddef.h
> > > index dab49e2ec8c0..8ca9df87a523 100644
> > > --- a/include/linux/stddef.h
> > > +++ b/include/linux/stddef.h
> > > @@ -108,7 +108,7 @@ enum {
> > >   	union {									\
> > >   		TYPE NAME;							\
> > >   		struct {							\
> > > -			unsigned char __offset_to_##FAM[offsetof(TYPE, FAM)];	\
> > > +			unsigned char __offset_to_##FAM[sizeof(TYPE)];		\
> > >   			MEMBERS							\
> > >   		};								\
> > >   	}
> > > 
> > > which only leaves one usage of FAM in the name of the padding struct
> > > member. I'm sure someone is able to come up with something nice here to
> > > get rid of FAM completely or point out what I'm missing.
> > 
> > Flexible structures (structs that contain a FAM) may have trailing padding.
> > Under that scenario sizeof(TYPE) causes the overlay between FAM and MEMBERS
> > to be misaligned.
> 
> That sounds wrong to me; are you sure? In that case allocating space for
> such a struct using
> 
> 	struct mystruct {
> 		unsigned short len;
> 		unsigned int array[];
> 	};
> 
> 	s = malloc(sizeof(struct mystruct) + n * sizeof(unsigned int));
> 
> wouldn't do the right thing. 
> 
> I found in the net (e.g.
> https://rgambord.github.io/c99-doc/sections/6/7/2/1/index.html):
> 
> 	In most situations, the flexible array member is ignored. In
> 	particular, the size of the structure is as if the flexible
> 	array member were omitted except that it may have more trailing
> 	padding than the omission would imply.
> 
> So I'd claim that sizeof does work here as intended.
> 
> gcc here also behaves fine:
> 
> 	uwe@taurus:~$ cat test.c
> 	#include <stdio.h>
> 
> 	struct mystruct {
> 		unsigned short len;
> 		unsigned int array[];
> 	};
> 
> 	struct mystruct2 {
> 		unsigned short len;
> 	};
> 
> 	int main()
> 	{
> 		printf("sizeof(struct mystruct) = %zu\n", sizeof(struct mystruct));
> 		printf("sizeof(struct mystruct2) = %zu\n", sizeof(struct mystruct2));
> 		return 0;
> 	}
> 
> 	uwe@taurus:~$ make test
> 	cc    -c -o test.o test.c
> 	cc   test.o   -o test
> 
> 	uwe@taurus:~$ ./test
> 	sizeof(struct mystruct) = 4
> 	sizeof(struct mystruct2) = 2

My claim is wrong, while sizeof() never gives a value that is too small,
it might be too big. E.g. for

	struct mystruct {
		unsigned short a;
		unsigned char b;
		unsigned char c[];
	};

there is sizeof(mystruct) = 4, but c starts at offset 3.

Anyhow, I applied the original patch now to

https://git.kernel.org/pub/scm/linux/kernel/git/ukleinek/linux.git pwm/for-next

; the discussion here was somewhat orthogonal anyhow.

Best regards
Uwe

Re: [PATCH][next] pwm: cros-ec: Avoid -Wflex-array-member-not-at-end warnings

Posted by Tzung-Bi Shih 1 month, 3 weeks ago

On Tue, Aug 12, 2025 at 11:35:41PM +0900, 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 warnings:
> 
> drivers/pwm/pwm-cros-ec.c:53:40: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
> drivers/pwm/pwm-cros-ec.c:87:40: 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 members onto the FAM while preserving the original
> memory layout.
> 
> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>

Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org>