Bug 93082 - macOS Authorization.h needs fixinclude
Summary: macOS Authorization.h needs fixinclude
Status: NEW
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 9.1.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-12-27 12:38 UTC by mcccs
Modified: 2022-05-24 15:48 UTC (History)
9 users (show)

See Also:
Host:
Target: x86_64-darwin
Build:
Known to work:
Known to fail:
Last reconfirmed: 2019-12-27 00:00:00


Attachments
work in progress patch for this case (877 bytes, text/plain)
2022-05-15 19:44 UTC, Iain Sandoe
Details

Note You need to log in before you can comment on or make changes to this bug.
Description mcccs 2019-12-27 12:38:57 UTC
This bug has existed for at least two years.

In Authorization.h:

```
static const size_t kAuthorizationExternalFormLength = 32;

typedef struct {
    char bytes[kAuthorizationExternalFormLength];
} AuthorizationExternalForm;
```

GCC prints this error:

/Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk/System/Library/Frameworks/Security.framework/Headers/Authorization.h:193:7:
error: variably modified 'bytes' at file scope

(from gcc/c/c-decl.c Line 5792)

Clang (-Wall -Wextra): *Compiles without warning*

Clang (-Wall -Wextra -pedantic): warning: variable length array folded to constant array as an extension [-Wgnu-folding-constant]

----

Impact: Golang's official compiler allows linking between C and Go. When I try to build a Go package that imports "crypto/x509" (usually indirectly), part of Go standard library on macOS, and `export CC=gcc-9` is set, Go returns the following error:

# crypto/x509
In file included from /Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk/System/Library/Frameworks/Security.framework/Headers/AuthSession.h:32,
                 from /Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk/System/Library/Frameworks/Security.framework/Headers/Security.h:42,
                 from /usr/local/Cellar/go/1.13.5/libexec/src/crypto/x509/root_cgo_darwin.go:17:
/Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk/System/Library/Frameworks/Security.framework/Headers/Authorization.h:193:7: error: variably modified 'bytes' at file scope
  193 |  char bytes[kAuthorizationExternalFormLength];
      |       ^~~~~

Other than a fixinclude, there are other ways:

- Every time after an upgrade, open Authorization.h with sudo vim and replace kAuthorizationExternalFormLength with 32
- Remove `export CC=gcc-9`
- Disable C-Go interlink (on by default)

A fixincludes entry would be quite clean:

fix = {
    hackname  = darwin_authorization;
    mach      = "*-*-darwin*";
    files     = Frameworks/Security.framework/Headers/Authorization.h;
    select    =
    "char bytes[kAuthorizationExternalFormLength];\n";
    c_fix     = format;
    c_fix_arg =
    "char bytes[32];\n";
    test_text =
    "char bytes[kAuthorizationExternalFormLength];\n";
};

However, I can't test it due to PR90835.
Comment 1 Andrew Pinski 2019-12-27 12:44:09 UTC
Why not change:
static const size_t kAuthorizationExternalFormLength = 32;

Into an enum:
enum {
  kAuthorizationExternalFormLength = 32
};

---- CUT ---
-Wgnu-folding-constant
Funny how clang thinks this was an GNU extension when it is not one.
Comment 2 mcccs 2019-12-30 06:32:13 UTC
Reported on the "other side" https://bugs.llvm.org/show_bug.cgi?id=44406

Changing it to enum works too, my only doubt is that it has a different width and sign (but better than not compiling)

Updated patch, thank you Andrew:

fix = {
    hackname  = darwin_authorization;
    mach      = "*-*-darwin*";
    files     = Frameworks/Security.framework/Headers/Authorization.h;
    select    =
    "static const size_t kAuthorizationExternalFormLength = 32;\n";
    c_fix     = format;
    c_fix_arg =
    "enum { kAuthorizationExternalFormLength = 32 };\n";
    test_text =
    "static const size_t kAuthorizationExternalFormLength = 32;\n";
};
Comment 3 Fabian Groffen 2020-11-24 18:28:55 UTC
The problem with this snippet is that it doesn't work on Frameworks, does it?  At least for me, it seems it searches from usr/include only?
Comment 4 mcccs 2020-11-27 21:36:19 UTC
(In reply to Fabian Groffen from comment #3)
> The problem with this snippet is that it doesn't work on Frameworks, does
> it?  At least for me, it seems it searches from usr/include only?

I'm not sure but it sounds likely. Isn't usr/include the higher-priority #include <...> search directory?
Comment 5 Sam James 2020-11-27 23:18:45 UTC
I've also reported this to Apple as FB8919799 (and on openradar - why not? https://openradar.appspot.com/radar?id=4952611266494464).
Comment 6 Eric Gallager 2021-11-12 15:30:28 UTC
This is hitting me in my fork of Apple's version of gdb; cc-ing Darwin and fixincludes maintainers
Comment 7 Eric Gallager 2022-05-14 23:28:34 UTC
(In reply to Fabian Groffen from comment #3)
> The problem with this snippet is that it doesn't work on Frameworks, does
> it?  At least for me, it seems it searches from usr/include only?

Yeah I'm pretty sure fixincludes doesn't work with Frameworks; is that worth opening a separate bug for, or should it just be tracked as part of this one?
Comment 8 Bruce Korb 2022-05-15 19:25:07 UTC
It should not be terribly difficult to add another include tree for fixinc to search. Also, unless 32 is cast as forever being the only value kAuthorizationExternalFormLength could possibly ever have:

    select    =
    "static const size_t kAuthorizationExternalFormLength = (32);\n";
    c_fix     = format;
    c_fix_arg =
    "enum { kAuthorizationExternalFormLength = %1 };\n";

BTW:
1. if you're going to simply replace text

2. on page https://gcc.gnu.org/wiki/GitMirror the "getting started" section leads to this error:

> fatal: 'svn/trunk' is not a commit and a branch 'trunk' cannot be created from it

(I don't have access to the GCC tree anymore [lost keys], so I'm starting over.)
Comment 9 Bruce Korb 2022-05-15 19:26:45 UTC
Um, I didn't finish my misthink for item#1 above. Please ignore. :)
Comment 10 Iain Sandoe 2022-05-15 19:34:50 UTC
I also have a few "clang compatibility workarounds" for GCC (including one for this specific case) - but they need putting behind some "-fclang-compatibility" flag or so.

Hopefully, will work on integrating them in GCC-13.

In some cases that is going to work better than a fix includes.

FWIW this is not actually a bug in GCC, the bug is in the headers as Andrew points out (the value is valid in C++ but not for C):

" Funny how clang thinks this was an GNU extension when it is not one."

I do not currently have a plan to try and build a second fix includes tree for Frameworks, but happy to review patches if someone else does :)
Comment 11 Iain Sandoe 2022-05-15 19:44:36 UTC
Created attachment 52980 [details]
work in progress patch for this case

 * this is not finished (needs to be conditional on a compatibility flag)
 * might not be acceptable upstream (depending on whether other maintainers are willing to accept clang extension work-arounds). 

However, AFAICS this 'extension' would affect all targets if they were presented with such input (so that this is not actually a Darwin-specific problem) - it's just that we see it in the headers.

The patch is against recent master, but I'd expect it to apply reasonably easily to 12.1, 11.3 and 10.x.
Comment 12 Eric Gallager 2022-05-24 14:27:11 UTC
(In reply to Iain Sandoe from comment #10)
> I do not currently have a plan to try and build a second fix includes tree
> for Frameworks, but happy to review patches if someone else does :)

OK I'll open a separate bug for that and self-assign
Comment 13 Eric Gallager 2022-05-24 15:14:48 UTC
(In reply to Eric Gallager from comment #12)
> OK I'll open a separate bug for that and self-assign

(that's bug 105719 now, for reference)