Bug #3373
closed
gcc >= 4.5 concerns about offsetof()
100%
Description
In UTS sources offsetof() is defined as
#define offsetof(s, m) ((size_t)(&((s *)0)->m))
and used when defining static arrays, e. g.:
https://github.com/illumos/illumos-gate/blob/master/usr/src/uts/i86pc/os/fakebop.c#L147
https://github.com/illumos/illumos-gate/blob/master/usr/src/uts/i86pc/sys/fastboot.h#L70
gcc >= 4.5 will no accept it and will produce a warning:
warning: variably modified ‘saved_drives’ at file scope
This warning is lethal with Werror ;)
__builtin_offsetof() works here without a warning.
Files
Related issues
Updated by Rich Lowe over 8 years ago
Yeah, you need to use builtin_offsetof for this.
You'll also need to fix the use in Xen to be actually constant, and you'll need to fix the fact we have a billion different places that define their own offsetof
Updated by Yuri Pankov over 8 years ago
This doesn't look like a "bug", more likely a "feature". Please provide patches to fix it.
Updated by Rich Lowe over 8 years ago
The hard part about fixing it properly, is dealing with the fact we have way way too many offsetof definitions.
The second hard part is the macros in some of the Xen code I mentioned above.
I have chunks of code for this (and the similar problems I imagine Igor will run in to next), I just have a bunch of other stuff to fix for the sake of a newer GCC before polishing the illumos branch.
Updated by Igor Pashev over 8 years ago
Maybe test for GCC version as well:
--- uts.orig/usr/src/uts/common/sys/sysmacros.h 2012-10-08 00:26:00.000000000 +0000 +++ uts/usr/src/uts/common/sys/sysmacros.h 2012-11-18 20:23:08.087883575 +0000 @@ -370,7 +370,11 @@ #if defined(_KERNEL) && !defined(_KMEMUSER) #if !defined(offsetof) -#define offsetof(s, m) ((size_t)(&(((s *)0)->m))) +# if defined(__GNUC__) +# define offsetof(s, m) __builtin_offsetof(s, m) +# else +# define offsetof(s, m) ((size_t)(&(((s *)0)->m))) +# endif /* __GNUC__ */ #endif /* !offsetof */ #define container_of(m, s, name) \\
Updated by Igor Kozhukhov over 8 years ago
have to update file too:
usr/src/head/iso/stddef_iso.h
Updated by Igor Pashev over 8 years ago
Igor Kozhukhov wrote:
have to update file too:
usr/src/head/iso/stddef_iso.h
GCC uses its own stddef.h
Updated by Rich Lowe over 8 years ago
GCC does not use it's own stddef.h while building illumos.
Also, as I said, a whole lot of things we compile don't use that offsetof.
Updated by Igor Pashev over 8 years ago
Updated by Rich Lowe over 8 years ago
Those patches look correct to me (presuming include order is correct, some things are fussy about sysmacros.h).
I should mention that wsdiff is going to be noisy, because you're going to be influencing whether some files get the function or macro versions of major() and minor(), but when I'd checked previously that would have no affect on behaviour.
Thanks,
Updated by Andrew Stormont about 6 years ago
- Related to Bug #5461: #pragma align before the declaration added
Updated by Andrew Stormont about 6 years ago
- Related to deleted (Bug #5461: #pragma align before the declaration)
Updated by Rich Lowe over 5 years ago
- Has duplicate Bug #5457: Variably modified at file scope added
Updated by Igor Kozhukhov over 5 years ago
i did review a long time with it:
http://cr.dilos.org/cr/igork/il-3373/
but it was discarded as always
Updated by Electric Monk about 5 years ago
- Status changed from New to Closed
- % Done changed from 0 to 100
git commit 876de206688d9fe008ad80c116a23a56701579d1
commit 876de206688d9fe008ad80c116a23a56701579d1
Author: Richard Lowe <richlowe@richlowe.net>
Date: 2016-03-05T15:41:12.000Z
3373 gcc >= 4.5 concerns about offsetof()
Portions contributed by: Igor Pashev <pashev.igor@gmail.com>
Reviewed by: Josef 'Jeff' Sipek <jeffpc@josefsipek.net>
Reviewed by: Igor Kozhukhov <ikozhukhov@gmail.com>
Reviewed by: Andy Stormont <andyjstormont@gmail.com>
Approved by: Dan McDonald <danmcd@omniti.com>