Bug 50909 - Process "#pragma options align=reset" correctly on Mac OS X
Summary: Process "#pragma options align=reset" correctly on Mac OS X
Status: NEW
Alias: None
Product: gcc
Classification: Unclassified
Component: objc (show other bugs)
Version: 4.6.2
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-10-29 05:02 UTC by roy
Modified: 2021-10-25 11:01 UTC (History)
5 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2014-06-23 00:00:00


Attachments
The patch. (1.55 KB, application/octet-stream)
2011-10-29 05:02 UTC, roy
Details

Note You need to log in before you can comment on or make changes to this bug.
Description roy 2011-10-29 05:02:53 UTC
Created attachment 25656 [details]
The patch.

I recently discovered that this program

  #include <IOKit/usb/USB.h>

  #pragma pack(1)
  int main() {
      return 0;
  }
  #pragma options align=reset

doesn't compile and gives the error

  too many #pragma options align=reset

Notice that the program includes "IOKit/usb/USB.h". In that file, pragmas are used like:

  #pragma pack(1)
  ...
  #pragma options align=reset

This gives the error because "#pragma pack(1)" is invisible to the relevant logic in "gcc/config/darwin-c.c". As such, I've attached a patch, "patch-pragmas.diff", that unifies said logic with the main pragma logic in "gcc/c-family/c-pragma.c".

If the currently observed behavior for the test program is the intended behavior, please close this bug and ignore the patch.
Comment 1 Carl Norum 2012-05-23 20:58:33 UTC
This same problem happened to me today on 4.7.0 - is it just expected behaviour or is this patch worth taking?
Comment 2 Roman 2012-08-08 08:26:07 UTC
This patch is works. I tested it on gcc 4.7.1 and 4.6.3. Mac OS version 10.6.8
Comment 3 Dean Churchill 2013-09-13 18:53:53 UTC
I am trying to apply this patch on OSX Lion (10.8.5) to gcc 4.6.2, but the diff command on OSX doesn't accept the --git option, and am not sure how to rewrite the patch for Lion.  Can you help?
Comment 4 Dominique d'Humieres 2013-09-13 19:05:46 UTC
> I am trying to apply this patch on OSX Lion (10.8.5) to gcc 4.6.2, 
> but the diff command on OSX doesn't accept the --git option, 
> and am not sure how to rewrite the patch for Lion.  Can you help?

If the patch applies cleanly on 4.6.2, from the source directory

patch -p0 -i patch_name

should work.
Comment 5 Dean Churchill 2013-09-13 20:32:30 UTC
That worked. thanks
Comment 6 Alex 2014-06-21 16:38:27 UTC
3 years later and it still doesn't work... gcc version 4.8.3, installed using homebrew on OSX 10.9.3.
Comment 7 Dominique d'Humieres 2014-06-23 14:51:57 UTC
> 3 years later and it still doesn't work... gcc version 4.8.3, installed
> using homebrew on OSX 10.9.3.

Well, you did not fix it, did you?

Reducing the test in comment 0 to

  #include <IOKit/usb/USB.h>

gives

In file included from pr50909_red.c:1:0:
/System/Library/Frameworks/IOKit.framework/Headers/usb/USB.h:584:9: error: too many #pragma options align=reset
 #pragma options align=reset
         ^
In file included from pr50909_red.c:1:0:
/System/Library/Frameworks/IOKit.framework/Headers/usb/USB.h:754:9: error: too many #pragma options align=reset
 #pragma options align=reset
         ^
/System/Library/Frameworks/IOKit.framework/Headers/usb/USB.h:806:9: error: too many #pragma options align=reset
 #pragma options align=reset
         ^
/System/Library/Frameworks/IOKit.framework/Headers/usb/USB.h:824:9: error: too many #pragma options align=reset
 #pragma options align=reset
         ^
/System/Library/Frameworks/IOKit.framework/Headers/usb/USB.h:844:9: error: too many #pragma options align=reset
 #pragma options align=reset
         ^

Now looking at the gcc manual I found at https://gcc.gnu.org/onlinedocs/gcc/Darwin-Pragmas.html#Darwin-Pragmas

options align=alignment
This pragma sets the alignment of fields in structures. The values of alignment may be mac68k, to emulate m68k alignment, or power, to emulate PowerPC alignment. Uses of this pragma nest properly; to restore the previous setting, use reset for the alignment. 

in line with the code in gcc/config/darwin-c.c. I also found at https://gcc.gnu.org/onlinedocs/gcc/Structure-Packing-Pragmas.html

6.57.8 Structure-Packing Pragmas

For compatibility with Microsoft Windows compilers, GCC supports a set of #pragma directives that change the maximum alignment of members of structures (other than zero-width bit-fields), unions, and classes subsequently defined. The n value below always is required to be a small power of two and specifies the new alignment in bytes.

#pragma pack(n) simply sets the new alignment.
#pragma pack() sets the alignment to the one that was in effect when compilation started (see also command-line option -fpack-struct[=n] see Code Gen Options).
#pragma pack(push[,n]) pushes the current alignment setting on an internal stack and then optionally sets the new alignment.
#pragma pack(pop) restores the alignment setting to the one saved at the top of the internal stack (and removes that stack entry). Note that #pragma pack([n]) does not influence this internal stack; thus it is possible to have #pragma pack(push) followed by multiple #pragma pack(n) instances and finalized by a single #pragma pack(pop).
Some targets, e.g. i386 and PowerPC, support the ms_struct #pragma which lays out a structure as the documented __attribute__ ((ms_struct)).

#pragma ms_struct on turns on the layout for structures declared.
#pragma ms_struct off turns off the layout for structures declared.
#pragma ms_struct reset goes back to the default layout.

So AFAIU gcc behaves as documented, while the behavior has been changed in clang (for which the tests compile).

IMO this PR should be closed as INVALID or WONTFIX, unless someone want to implement the clang behavior.
Comment 8 Alex 2014-06-23 15:48:35 UTC
I am writing cross platform code with some platform specific USB enumeration functionality. IOKit was just what I needed on the mac, libudev on linux. I inevitably stumbled over this issue when trying to compile the mac portion. 

I think it would be sad if gcc wouldn't support these deviations from the standard in the future, considering anybody trying to use IOKit with gcc will come across this "bug".

Are you saying I should not use gcc on a mac with IOKit?
If I should, then gcc should not give me such an error. It is easy to work around but why punish the developer when Apple wrote the non-compliant header that IOKit users must include.
I think this is a clear case where gcc could & should change its leniency for the sake of its developers. Or, am I supposed to write my own USB library from the ground up for mac just because the pragmas aren't supported? 


I "fixed" my setup without recompiling gcc:
I made a backup of USB.h and then changed all occurrences of

#pragma options align=reset

to 

#pragma pack()

which seemed to compile and run. 

As to someone implementing clang behavior: I was under the impression that the attached patch just needed to be applied? That is why I was inquiring as to why nobody had applied it after 3 years.
Comment 9 Manuel López-Ibáñez 2014-06-23 16:30:20 UTC
(In reply to Alex from comment #8)
> As to someone implementing clang behavior: I was under the impression that
> the attached patch just needed to be applied? That is why I was inquiring as
> to why nobody had applied it after 3 years.

Writing a patch is often the easy part. The hard part is getting the patch approved and following up on reviewer's comments. See https://gcc.gnu.org/contribute.html and https://gcc.gnu.org/wiki/GCC_Research

The patch doesn't look correct to me anyway, but you will get a better review from a Darwin maintainer, see the MAINTAINERS file in the gcc sources.
Comment 10 Eric Gallager 2017-07-19 19:12:56 UTC
Confirming that the original testcase still fails with an unpatched gcc; I think it's worth fixing (by implementing the clang behavior) so I'm marking it as "NEW" instead of "INVALID" or "WONTFIX"
Comment 11 Eric Gallager 2018-04-19 14:04:57 UTC
(In reply to Manuel López-Ibáñez from comment #9)
> (In reply to Alex from comment #8)
> > As to someone implementing clang behavior: I was under the impression that
> > the attached patch just needed to be applied? That is why I was inquiring as
> > to why nobody had applied it after 3 years.
> 
> Writing a patch is often the easy part. The hard part is getting the patch
> approved and following up on reviewer's comments. See
> https://gcc.gnu.org/contribute.html and https://gcc.gnu.org/wiki/GCC_Research
> 
> The patch doesn't look correct to me anyway, but you will get a better
> review from a Darwin maintainer, see the MAINTAINERS file in the gcc sources.

Adding them on cc (Only Mike is currently listed as a Darwin maintainer, but IMO Iain ought to be listed as one, too)
Comment 12 Rudolf 2018-05-26 20:19:29 UTC
This is still an issue... 7 YEARS LEATER!?

I tried to change the permission on USB.h just to get around this bug...

with something like this:
#if defined(__clang__) || defined(__llvm__)
    #pragma options align=reset
#else
    #pragma pack()
#endif

but sadly could not do so atm.

I am using gcc 8.1.0. please fix this.
Comment 13 Eric Gallager 2018-05-27 12:13:50 UTC
(In reply to Rudolf from comment #12)
> This is still an issue... 7 YEARS LEATER!?
> 
> I tried to change the permission on USB.h just to get around this bug...
> 
> with something like this:
> #if defined(__clang__) || defined(__llvm__)
>     #pragma options align=reset
> #else
>     #pragma pack()
> #endif
> 
> but sadly could not do so atm.
> 
> I am using gcc 8.1.0. please fix this.

Maybe we could do that as a fixincludes until someone figures out how to code support for the pragma properly
Comment 14 Rudolf 2018-05-28 15:38:44 UTC
Even if the compiler would just use the workaround with pragma-pack() when he encounters this specific pragma would be fine. (If there is a warning for that!)

Some Infomrations:
http://www.msg.ucsf.edu/local/programs/IBM_Compilers/C:C++/html/compiler/ref/rnpgalin.htm

Seems like I will have to use the MAC OS Clang compiler for now...
Comment 15 Eric Gallager 2019-05-28 05:40:00 UTC
(In reply to Eric Gallager from comment #11)
> (In reply to Manuel López-Ibáñez from comment #9)
> > (In reply to Alex from comment #8)
> > > As to someone implementing clang behavior: I was under the impression that
> > > the attached patch just needed to be applied? That is why I was inquiring as
> > > to why nobody had applied it after 3 years.
> > 
> > Writing a patch is often the easy part. The hard part is getting the patch
> > approved and following up on reviewer's comments. See
> > https://gcc.gnu.org/contribute.html and https://gcc.gnu.org/wiki/GCC_Research
> > 
> > The patch doesn't look correct to me anyway, but you will get a better
> > review from a Darwin maintainer, see the MAINTAINERS file in the gcc sources.
> 
> Adding them on cc (Only Mike is currently listed as a Darwin maintainer, but
> IMO Iain ought to be listed as one, too)

...and Iain *is* in fact listed as one now, too! I was ahead of the game!
Comment 16 Evan Miller 2021-10-25 11:01:35 UTC
An unusual but viable workaround for this problem is:

  #pragma options align=power
  #pragma options align=power
  #pragma options align=power
  #pragma options align=power
  #pragma options align=power
  #include <IOKit/usb/USB.h>

This technique pre-loads the "options" alignment stack with five default values. When

  #pragma pack(1)

is encountered in the header file, the global alignment is set to 1 byte. Then when

  #pragma options align=reset

is encountered, a value is popped off the options stack and the global alignment is set to the value at the top of the stack ("power" which is implemented as the platform default). So the pack and options pragmas inside the header file end up having the desired effect.

If the number of "align=power" pragmas before the header inclusion equals the number of "align=reset" pragmas inside the header, then after inclusion, the alignment stack will be whatever it was before all the "align=power" pragmas. It probably won't hurt anything to have extra values on the alignment stack, so you can add a few more to be safe.