Project

General

Profile

Bug #3373

gcc >= 4.5 concerns about offsetof()

Added by Igor Pashev about 8 years ago. Updated over 4 years ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Category:
-
Start date:
2012-11-18
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
needs-triage
Gerrit CR:

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

illumos-3373-gcc-offsetof.patch (9.45 KB) illumos-3373-gcc-offsetof.patch Igor Pashev, 2012-11-23 08:33 PM
uts-xen-__CONST_RING_SIZE.patch (2.24 KB) uts-xen-__CONST_RING_SIZE.patch Igor Pashev, 2012-11-23 08:33 PM

Related issues

Related to illumos gate - Bug #3062: illumos should build/work with GCC 4.6 or newerNewToomas Soome2012-08-07

Actions
Has duplicate illumos gate - Bug #5457: Variably modified at file scopeClosed2014-12-21

Actions
#1

Updated by Rich Lowe about 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

#2

Updated by Yuri Pankov about 8 years ago

This doesn't look like a "bug", more likely a "feature". Please provide patches to fix it.

#3

Updated by Rich Lowe about 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.

#4

Updated by Igor Pashev about 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)            \\
#5

Updated by Igor Kozhukhov about 8 years ago

have to update file too:
usr/src/head/iso/stddef_iso.h

#6

Updated by Igor Pashev about 8 years ago

Igor Kozhukhov wrote:

have to update file too:
usr/src/head/iso/stddef_iso.h

GCC uses its own stddef.h

#7

Updated by Rich Lowe about 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.

#9

Updated by Rich Lowe about 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,

#10

Updated by Andrew Stormont over 5 years ago

  • Related to Bug #5461: #pragma align before the declaration added
#11

Updated by Andrew Stormont over 5 years ago

  • Related to deleted (Bug #5461: #pragma align before the declaration)
#12

Updated by Rich Lowe almost 5 years ago

  • Has duplicate Bug #5457: Variably modified at file scope added
#13

Updated by Igor Kozhukhov almost 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

#14

Updated by Electric Monk over 4 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>

Also available in: Atom PDF