-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Linux: Clang build fixes #17954
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Linux: Clang build fixes #17954
Conversation
module/Kbuild.in
Outdated
| # any of the other module initialization functions which depend on it. | ||
|
|
||
| ZFS_MODULE_CFLAGS += -std=gnu99 -Wno-declaration-after-statement | ||
| ZFS_MODULE_CFLAGS += -Wno-declaration-after-statement |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interestingly, this breaks the build on almalinux8. I haven't looked closely but it sure looks like it's defaulting to C89/C90 here.
/tmp/zfs-build-zfs-DRBBnzeA/BUILD/zfs-2.4.99/module/os/linux/spl/spl-kmem-cache.c: In function 'spl_slab_alloc':
/tmp/zfs-build-zfs-DRBBnzeA/BUILD/zfs-2.4.99/module/os/linux/spl/spl-kmem-cache.c:272:2: error: 'for' loop initial declarations are only allowed in C99 or C11 mode
for (int i = 0; i < sks->sks_objs; i++) {
^~~
/tmp/zfs-build-zfs-DRBBnzeA/BUILD/zfs-2.4.99/module/os/linux/spl/spl-kmem-cache.c:272:2: note: use option -std=c99, -std=gnu99, -std=c11 or -std=gnu11 to compile your code
/tmp/zfs-build-zfs-DRBBnzeA/BUILD/zfs-2.4.99/module/os/linux/spl/spl-kmem-cache.c: In function 'spl_cache_flush':
/tmp/zfs-build-zfs-DRBBnzeA/BUILD/zfs-2.4.99/module/os/linux/spl/spl-kmem-cache.c:500:2: error: 'for' loop initial declarations are only allowed in C99 or C11 mode
for (int i = 0; i < count; i++)
^~~
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh, interesting, I'd missed a detail. Kernel changed to gnu11 in 5.18.x, before that it was gnu89, far too old.
Last push changes that commit to force gnu11 for us instead. Seems to build ok on 4.19 with GCC. Well see what the builders make of it.
Linux switched from -std=gnu89 to -std=gnu11 in 5.18 (torvalds/linux@e8c07082a810f). We've always overridden that with gnu99 because we use some newer features. More recent kernels are using C11 features in headers that we include. GCC generally doesn't seem to care, but more recent versions of Clang seem to be enforcing our gnu99 override more strictly, which breaks the build in some configurations. Just bumping our "override" to match the kernel seems to be the easiest workaround. It's an effective no-op since 5.18, while still allowing us to build on older kernels. Sponsored-by: https://despairlabs.com/sponsor/ Signed-off-by: Rob Norris <robn@despairlabs.com>
Using strlen() in an static array declaration is a GCC extension. Clang calls it "gnu-folding-constant" and warns about it, which breaks the build. If it were widespread we could just turn off the warning, but since there's only one case, lets just change the array to an explicit size. Sponsored-by: https://despairlabs.com/sponsor/ Signed-off-by: Rob Norris <robn@despairlabs.com>
6.18 changes kmap_atomic() to take a const pointer. This is no problem for the places we use it, but Clang fails the test due to a warning about being unable to guarantee that uninitialised data will definitely not change. Easily solved by forcibly initialising it. Sponsored-by: https://despairlabs.com/sponsor/ Signed-off-by: Rob Norris <robn@despairlabs.com>
fc9e6bb to
8fbbeab
Compare
|
Tested it without the -std=gnu99 and now with -std=gnu11 change and both work. Solves the #17951, thanks. |
behlendorf
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assuming the CI gives it a clean bill of health this looks good.
Motivation and Context
Recent Clang is a little upset with a handful of things on the Linux side. This sorts it out.
Fixes #17951
Fixes #17959
Description
See commit messages. Summary:
How Has This Been Tested?
Compiled and basic sanity runs done with GCC 14.2 and Clang 17, 19 and 22 against Linux 6.1.x, 6.16.x and 6.18-rc.
Not my usual comprehensive set, but recompiling kernels as well adds a lot of time and building with Clang is more difficult the older they get.
Types of changes
Checklist:
Signed-off-by.