Re: [peterz-queue:perf/core 26/39] arch/x86/include/asm/perf_event.h:290:19: error: flexible array member in a struct with no named members

[Gustavo A. R. Silva <gustavoars@kernel.org>]

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

On Tue, Jul 07, 2020 at 12:24:43PM +0200, Peter Zijlstra wrote:
> On Mon, Jul 06, 2020 at 09:08:41PM -0400, Liang, Kan wrote:
> 
> > >     288	
> > >     289	struct pebs_lbr {
> > >   > 290		struct lbr_entry lbr[]; /* Variable length */
> > >     291	};
> > >     292	
> > 
> > The struct pebs_lbr only has one member. To fix the warning, it seems we
> > have to define the struct as below.
> > 
> > 
> > diff --git a/arch/x86/include/asm/perf_event.h
> > b/arch/x86/include/asm/perf_event.h
> > index 2338200..a387e14 100644
> > --- a/arch/x86/include/asm/perf_event.h
> > +++ b/arch/x86/include/asm/perf_event.h
> > @@ -283,7 +283,7 @@ struct pebs_xmm {
> >  };
> > 
> >  struct pebs_lbr {
> > -	struct lbr_entry lbr[]; /* Variable length */
> > +	struct lbr_entry lbr[0]; /* Variable length */
> >  };
> > 
> 
> Right, but then we get trouble like the below again. There's basically

Yep, zero-length and one-element arrays are being deprecated[1].

> no good solution here :-(
> 
> Gustavo, any clues?
> 

Yep; the issue is that "Flexible array members may only appear as the
last member of a struct that is otherwise non-empty"[2].

The solution is to declare something like this:

struct pebs_lbr {
	size_t num_lbr; /* count for number of lbr entries */
	struct lbr_entry lbr[];
};

and allocate memory as follows:

struct pebs_lbr *lbr_entries;

...

lbr_entries = kmalloc(struct_size(lbr_entries, lbr, count), GFP_KERNEL);
lbr_entries->num_lbr = count;

...

and then you can access each entry through the use of an index
delimited by lbr_entries->num_lbr (which is self-contained and
not any other random variable):

for (i = 0; i < lbr_entries->num_lbr; i++)
{
	...

	whatever = lbr_entries->lbr[i];

	...
}

and so forth...

--
Gustavo

[1] https://lore.kernel.org/lkml/20200608213711.GA22271(a)embeddedor/
[2] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html

> ---
> commit 04f5c362ec6d3ff0e14f1c05230b550da7f528a4
> Author: Gustavo A. R. Silva <gustavoars@kernel.org>
> Date:   Thu May 7 14:21:41 2020 -0500
> 
>     sched/fair: Replace zero-length array with flexible-array
>     
>     The current codebase makes use of the zero-length array language
>     extension to the C90 standard, but the preferred mechanism to declare
>     variable-length types such as these ones is a flexible array member[1][2],
>     introduced in C99:
>     
>     struct foo {
>             int stuff;
>             struct boo array[];
>     };
>     
>     By making use of the mechanism above, we will get a compiler warning
>     in case the flexible array does not occur last in the structure, which
>     will help us prevent some kind of undefined behavior bugs from being
>     inadvertently introduced[3] to the codebase from now on.
>     
>     Also, notice that, dynamic memory allocations won't be affected by
>     this change:
>     
>     "Flexible array members have incomplete type, and so the sizeof operator
>     may not be applied. As a quirk of the original implementation of
>     zero-length arrays, sizeof evaluates to zero."[1]
>     
>     sizeof(flexible-array-member) triggers a warning because flexible array
>     members have incomplete type[1]. There are some instances of code in
>     which the sizeof operator is being incorrectly/erroneously applied to
>     zero-length arrays and the result is zero. Such instances may be hiding
>     some bugs. So, this work (flexible-array member conversions) will also
>     help to get completely rid of those sorts of issues.
>     
>     This issue was found with the help of Coccinelle.
>     
>     [1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
>     [2] https://github.com/KSPP/linux/issues/21
>     [3] commit 76497732932f ("cxgb3/l2t: Fix undefined behaviour")
>     
>     Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
>     Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
>     Link: https://lkml.kernel.org/r/20200507192141.GA16183(a)embeddedor
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 44b0c8edc260..01f94cf52783 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -1094,7 +1094,7 @@ struct numa_group {
>  	 * more by CPU use than by memory faults.
>  	 */
>  	unsigned long *faults_cpu;
> -	unsigned long faults[0];
> +	unsigned long faults[];
>  };
>  
>  /*
> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> index 21416b30c520..2bd2a222318a 100644
> --- a/kernel/sched/sched.h
> +++ b/kernel/sched/sched.h
> @@ -1462,7 +1462,7 @@ struct sched_group {
>  	 * by attaching extra space to the end of the structure,
>  	 * depending on how many CPUs the kernel has booted up with)
>  	 */
> -	unsigned long		cpumask[0];
> +	unsigned long		cpumask[];
>  };
>  
>  static inline struct cpumask *sched_group_span(struct sched_group *sg)