-Wgnu-variable-sized-type-not-at-end in include/uapi/linux/bcache.h

[nathanchance]

After llvm/llvm-project@113b0d7:

$ make -skj"$(nproc)" LLVM=1 O=out/x86_64 distclean allyesconfig usr/include/
...
In file included from <built-in>:1:
./usr/include/linux/bcache.h:286:2: warning: field '' with variable sized type 'union jset::(anonymous at ./usr/include/linux/bcache.h:286:2)' not at the end of a struct or class is a GNU extension [-Wgnu-variable-sized-type-not-at-end]
        BKEY_PADDED(uuid_bucket);
        ^
./usr/include/linux/bcache.h:134:2: note: expanded from macro 'BKEY_PADDED'
        union { struct bkey key; __u64 key ## _pad[BKEY_PAD]; }
        ^
./usr/include/linux/bcache.h:287:2: warning: field '' with variable sized type 'union jset::(anonymous at ./usr/include/linux/bcache.h:287:2)' not at the end of a struct or class is a GNU extension [-Wgnu-variable-sized-type-not-at-end]
        BKEY_PADDED(btree_root);
        ^
./usr/include/linux/bcache.h:134:2: note: expanded from macro 'BKEY_PADDED'
        union { struct bkey key; __u64 key ## _pad[BKEY_PAD]; }
        ^
2 warnings generated.

We do not see this type of warning in regular files due to -Wno-gnu being added to KBUILD_CFLAGS but the header test uses a much smaller set of flags for build tests: https://elixir.bootlin.com/linux/v5.7.2/source/usr/include/Makefile

I think this is legitimate though...

struct bkey {
	__u64	high;
	__u64	low;
	__u64	ptr[];
};

...

/* Enough for a key with 6 pointers */
#define BKEY_PAD		8

#define BKEY_PADDED(key)					\
	union { struct bkey key; __u64 key ## _pad[BKEY_PAD]; }

...

struct jset {
	__u64			csum;
	__u64			magic;
	__u64			seq;
	__u32			version;
	__u32			keys;

	__u64			last_seq;

	BKEY_PADDED(uuid_bucket);
	BKEY_PADDED(btree_root);
	__u16			btree_level;
	__u16			pad[3];

	__u64			prio_bucket[MAX_CACHES_PER_SET];

	union {
		struct bkey	start[0];
		__u64		d[0];
	};
};

https://elixir.bootlin.com/linux/v5.7.2/source/include/uapi/linux/bcache.h

Slightly simplified: https://godbolt.org/z/j2Xwvf

Not quite sure how to fix it though since this structure already has a union at the end with zero-length arrays (and flexible members aren't allowed in unions?).

cc @kees @GustavoARSilva

[nickdesaulniers]

Reproducible with:

$ make LLVM=1 LLVM_IAS=1 -j72 allyesconfig
$ make LLVM=1 LLVM_IAS=1 -j72 -s usr/include/

The members uuid_bucket and btree_root of struct jset seem to trigger this.

Looks like they were added to include/uapi/linux/bcache.h in
commit 81ab419 ("bcache: Pull on disk data structures out into a separate header")

MAINTAINERS has an entry for BCACHE.

Though, I wonder if -Wgnu-variable-sized-type-not-at-end should be moved under -Wpedantic? In fact, GCC does warn for this case with -std=c90, but only when -Wpedantic is enabled: https://godbolt.org/z/P8ejn81PW

<source>:7:17: warning: invalid use of structure with flexible array member [-Wpedantic]
    7 |     struct bkey key;
      |                 ^~~

[GustavoARSilva]

On 4/28/21 16:06, Nick Desaulniers wrote: Reproducible with: ```sh $ make LLVM=1 LLVM_IAS=1 -j72 allyesconfig $ make LLVM=1 LLVM_IAS=1 -j72 -s usr/include/ ``` The members `uuid_bucket` and `btree_root` of `struct jset` seem to trigger this. Looks like they were added to include/uapi/linux/bcache.h in commit 81ab419 ("bcache: Pull on disk data structures out into a separate header") MAINTAINERS has an entry for BCACHE. Though, I wonder if `-Wgnu-variable-sized-type-not-at-end` should be moved under `-Wpedantic`? In fact, GCC does warn for this case with `-std=c90`, but only when `-Wpedantic` is enabled: https://godbolt.org/z/P8ejn81PW ``` <source>:7:17: warning: invalid use of structure with flexible array member [-Wpedantic] 7 | struct bkey key; | ^~~ ```

It seems to me that the problem here is the union containing an object of a struct type that contains a flexible array: #define BKEY_PADDED(key) \ union { struct bkey key; __u64 key ## _pad[BKEY_PAD]; }

…

-- Gustavo

[GustavoARSilva]

I think the solution might well be something like this: https://git.kernel.org/linus/c0a744dcaa29e9537e8607ae9c965ad936124a4d

[kees]

I see there is open-coded use of ->ptr outside of this header file, so I see why this can't be just made to always be '6', etc...

We do not see this type of warning in regular files due to -Wno-gnu being added to KBUILD_CFLAGS but the header test uses a much smaller set of flags for build tests: https://elixir.bootlin.com/linux/v5.7.2/source/usr/include/Makefile

I was going to say, "I'm surprised GCC doesn't warn about this issue already."

Though, I wonder if -Wgnu-variable-sized-type-not-at-end should be moved under -Wpedantic? In fact, GCC does warn for this case with -std=c90, but only when -Wpedantic is enabled: https://godbolt.org/z/P8ejn81PW

This does seem like the right approach here -- otherwise we'll have a standing warning difference between GCC and Clang otherwise.

[arndb]

I wonder if we should just move some or all of bcache.h out of the uapi headers, this looks like it's not even meant to be done in user space, but it's hard to tell.

[nickdesaulniers]

This isn't CONFIG_WERROR related because that config doesn't set -Werror for HOSTCFLAGS.

[nathanchance]

Fixed: https://git.kernel.org/linus/cf2197ca4b8c199d188593ca6800ea1827c42171