After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 772451 - Broken clock_gettime and mkostemp detection with XCode 8
Broken clock_gettime and mkostemp detection with XCode 8
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: cerbero
git master
Other Mac OS
: Normal blocker
: 1.10.0
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on: 770587
Blocks:
 
 
Reported: 2016-10-05 13:32 UTC by Nirbheek Chauhan
Modified: 2016-10-25 08:46 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch for gstreamer.git to conditionally use clock_gettime on OS X (4.53 KB, patch)
2016-10-05 13:49 UTC, Nirbheek Chauhan
none Details | Review
Patch for gstreamer.git to conditionally use clock_gettime on OS X (5.18 KB, patch)
2016-10-07 17:44 UTC, Nirbheek Chauhan
committed Details | Review
Cerbero patches to fix build with XCode 8 (14.19 KB, patch)
2016-10-07 17:45 UTC, Nirbheek Chauhan
committed Details | Review

Description Nirbheek Chauhan 2016-10-05 13:32:42 UTC
XCode 8 includes the macOS 10.12 SDK which adds some new API: clock_gettime, getentropy, mkostemp, etc. These are defined in a way that causes AC_CHECK_FUNCS configure checks to always detect them as available regardless of what minimum OS X version you are targetting[1][2].

1. https://github.com/mobile-shell/mosh/issues/807#issuecomment-249736455
2. https://github.com/Homebrew/homebrew-core/issues/2674

This means that binaries built with XCode 8 that conditionally use clock_gettime, getentropy, mkostemp will not run on any version of OS X older than 10.12.

This affects GStreamer itself and a bunch of recipes that we use in Cerbero. I'm slowly making my way through Cerbero and fixing recipes. I also have a WIP patch for gstreamer that I will push once I've tested it on Mac OS X 10.11 and macOS 10.12.

The way to detect this at compile-time is by building with LDFLAGS="-Wl,no_weak_imports` which was added to XCode 8 specifically for this[3]. However, this is a hammer approach and can't be used in glib or gstreamer since both of those use weak imports for legitimate purposes.

3. https://github.com/Homebrew/homebrew-core/issues/3727

We should probably not release 1.10 without fixing this otherwise binaries built on macOS 10.12 will not run on older versions of OS X, and binaries built on OS X 10.11.6 with XCode 8 will not run at all. We should also backport this to 1.8.x.
Comment 1 Sebastian Dröge (slomo) 2016-10-05 13:35:10 UTC
Can't you use LDFLAGS="-Wl,no_weak_imports` just for the relevant configure checks for these functions?
Comment 2 Nirbheek Chauhan 2016-10-05 13:38:14 UTC
How do you pass CFLAGS/LDFLAGS to AC_CHECK_FUNCS? Does it just read those environment variables?
Comment 3 Sebastian Dröge (slomo) 2016-10-05 13:43:00 UTC
yes
Comment 4 Nirbheek Chauhan 2016-10-05 13:49:07 UTC
Created attachment 336985 [details] [review]
Patch for gstreamer.git to conditionally use clock_gettime on OS X

I just remembered: we can't do that because we have to choose whether to use clock_gettime and other new API or not depending on the minimum OS X version we're targetting. If someone wants to only target macOS 10.12, they should be allowed to use this new API.

See the attached patch for my approach to this.
Comment 5 Nirbheek Chauhan 2016-10-05 13:52:51 UTC
Also we can't just do -Wl,-no_weak_imports for the configure check because the check does its own prototype declaration which bypasses the compiler check. So you'll see the error at link time with -Wl,no_weak_imports but the configure check will pass.
Comment 6 Sebastian Dröge (slomo) 2016-10-05 14:24:36 UTC
That seems extremely verbose, if we have to do that for all symbols like that...
Comment 7 Nirbheek Chauhan 2016-10-05 14:34:30 UTC
I can't really see another way to do this, unfortunately.
Comment 8 Nirbheek Chauhan 2016-10-07 17:44:55 UTC
Created attachment 337187 [details] [review]
Patch for gstreamer.git to conditionally use clock_gettime on OS X

This patch has been tested on OS X 10.11.6 with XCode 8. Going to test on macOS 10.12 next with XCode 8.
Comment 9 Nirbheek Chauhan 2016-10-07 17:45:59 UTC
Created attachment 337188 [details] [review]
Cerbero patches to fix build with XCode 8

Several recipes needed workarounds for this XCode issue. I also removed tar.recipe because it was also having issues and it turned out that we weren't actually using it at all.
Comment 10 Nirbheek Chauhan 2016-10-12 22:13:20 UTC
Both patches pushed.

gstreamer:

commit 6c1bc80d2791af9a5b07d59ce8e6f7039577eece
Author: Nirbheek Chauhan <nirbheek@centricular.com>
Date:   Mon Oct 3 20:22:53 2016 +0530

    build: Fix clock_gettime check with XCode 8
    
    With XCode 8, clock_gettime will be incorrectly detected as being
    available regardless of what OS X version we're targetting because the
    symbol is available in the .tbd library as a weak symbol.
    See: https://github.com/Homebrew/homebrew-core/issues/3727#issue-170086273
    
    It's only starting from macOS 10.12 that clock_gettime is actually
    available, so we can unconditionally disable it when targetting older
    versions. We cannot simply do AC_CHECK_FUNCS with -Wl,-no_weak_imports
    because the autoconf check does its own prototype declaration that
    doesn't trigger that compiler flag.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=772451


Cerbero:

commit 45da4d69c1ce52d7e8c8e77477ac327bdc99ca7f
Author: Nirbheek Chauhan <nirbheek@centricular.com>
Date:   Fri Oct 7 23:03:19 2016 +0530

    Workaround broken XCode 8 SDK weak symbol detection
    
    macOS 10.12 added a bunch of new symbols: clock_gettime, getentropy,
    mkostemp, etc. These are available in the macOS 10.12 SDK. However,
    XCode 8 now only ships that version of the SDK, and because of the way
    the symbols are defined, they are always detected as being available
    regardless of what OS X version you are actually targetting.
    
    Long story short, the only way around this is to workaround it by
    forcibly disabling detection of these symbols when we're targetting OS
    X versions older than 10.12. For more details, see:
    https://bugzilla.gnome.org/show_bug.cgi?id=772451

commit 94182f84b5ad6c715a70bb90fa6e4b4d488aa3dc
Author: Nirbheek Chauhan <nirbheek@centricular.com>
Date:   Fri Oct 7 23:12:59 2016 +0530

    osx: Don't build GNU tar in build-tools
    
    We don't even use tar for unpacking, and it seems BSD tar is enough for
    everything else.
Comment 11 Andoni Morales 2016-10-22 10:20:20 UTC
As a side note, the main issue is that Apple is using incorrectly their own availablity checks.

The in time.h clock_gettime function is defined this way
__CLOCK_AVAILABILITY
int clock_gettime(clockid_t __clock_id, struct timespec *__tp);

While it should be defined this way
__CLOCK_AVAILABILITY int clock_gettime(clockid_t __clock_id, struct timespec *__tp);

When __CLOCK_AVAILABILITY is in the same line as the function, the compiler correctly removes the function with -mmacosx-version-min=10.7
Comment 12 Andoni Morales 2016-10-22 19:20:59 UTC
Forget what I said, I was testing too many things at once trying to fix our build system :)
Comment 13 Nirbheek Chauhan 2016-10-23 12:55:44 UTC
The main issue is that Autoconf detects symbols in libraries by supplying its own prototype and trying to link to that symbol. This means that the prototype in the Apple headers is not used and `__CLOCK_AVAILABILITY` is not used at all, which causes the linker to always find the symbol regardless of whether you use `-Wl,-no_weak_imports` or `-mmacosx-version-min=10.7`, etc.
Comment 14 Andoni Morales 2016-10-24 09:05:54 UTC
But even a simple test case like this one compiles correctly with -mmacosx-version-min=10.7, the only way to make it fail is with -Werror=partial-availability  :(

#include <time.h>

int main () {
  struct timespec ts;
  clock_gettime (CLOCK_REALTIME, &ts);
}
Comment 15 Nirbheek Chauhan 2016-10-24 15:24:51 UTC
That's when you use the prototype provided by <time.h>. The following is the test that Autoconf uses (for various reasons; primarily to avoid having to need the header while doing libc symbol checks):

#define clock_gettime disabled_clock_gettime
#include <time.h>
#undef clock_gettime

char clock_gettime ();

int main() {
    return clock_gettime();
}


gcc -mmacosx-version-min=10.8 -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.12.sdk test.c

The guards around the time.h include are for removing the definition completely (as found in the Autoconf check). You can reproduce this even if you remove the first three lines.
Comment 16 Nirbheek Chauhan 2016-10-24 15:30:42 UTC
Oh, sorry, I misread your comment Andoni. You're right, even that simple test case compiles correctly when it really should not. Apple has shipped a completely broken SDK.
Comment 17 Nirbheek Chauhan 2016-10-24 16:31:19 UTC
I forgot to add another thing (I wish you could edit bugzilla comments): 

Apple wants you to use the new symbols as weak imports so that you can compile a single binary that can run on multiple platforms. You are supposed to check if the symbol is available at runtime and use fallback code if it does not.

I think this is a really cool feature, but they added it in a terrible way. They made it the default for all symbols. This means that people will compile their app with a new SDK and it will barf when they run it on an older OS X. This is most prominently visible on OS X 10.11.6 where if you build with the default XCode 8 SDK, your program will use symbols not available on your OS at all and fail at runtime.

To mitigate this, they added a new option that forces the compiler to not assume that your symbol usage is proper, and error out if you have any references to symbols not available in your targetted OS X version: `-Wl,-no_weak_imports`.

If you use that to compile your basic testcase, it will fail as follows:

$ clang -Wl,-no_weak_imports -mmacosx-version-min=10.8 -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.12.sdk test2.c
ld: weak import of symbol '_clock_gettime' not supported because of option: -no_weak_imports for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
$


And it will work fine if you target a newer OS X:

$ clang -Wl,-no_weak_imports -mmacosx-version-min=10.12 -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.12.sdk test2.c
$

BUT, this option relies on availability information in the prototype (time.h for clock_gettime). So if you don't use the prototype in the header provided by Apple, the symbol will always be "available" to the linker. This is why the Autoconf check is broken no matter what we do and we have to work around it manually -- it always uses its own broken prototype.
Comment 18 Tim-Philipp Müller 2016-10-24 21:46:29 UTC
> Apple has shipped a completely broken SDK.

Don't suppose the 10.12.1 update fixes anything here?
Comment 19 Nirbheek Chauhan 2016-10-24 21:56:17 UTC
> > Apple has shipped a completely broken SDK.

> Don't suppose the 10.12.1 update fixes anything here?

No, because the change would need to come as an XCode 8 update (it was last updated on Sept 13). I also don't think Apple will change this behaviour just to make Autoconf happy; not exactly their target demographic. ;)
Comment 20 Nirbheek Chauhan 2016-10-25 03:58:24 UTC
Meson was doing something similar to Autoconf, and was broken in the same way because of that. I've raised a PR for Meson that fixes this issue there: https://github.com/mesonbuild/meson/pull/949

We should be able to drop the extra code that we have in the Meson build files for this once the PR is merged and after 1.10 is released.
Comment 21 Andoni Morales 2016-10-25 08:46:30 UTC
(In reply to Nirbheek Chauhan from comment #17)
> I forgot to add another thing (I wish you could edit bugzilla comments): 
> 
> Apple wants you to use the new symbols as weak imports so that you can
> compile a single binary that can run on multiple platforms. You are supposed
> to check if the symbol is available at runtime and use fallback code if it
> does not.
> 
> I think this is a really cool feature, but they added it in a terrible way.
> They made it the default for all symbols. This means that people will
> compile their app with a new SDK and it will barf when they run it on an
> older OS X. This is most prominently visible on OS X 10.11.6 where if you
> build with the default XCode 8 SDK, your program will use symbols not
> available on your OS at all and fail at runtime.
> 
> To mitigate this, they added a new option that forces the compiler to not
> assume that your symbol usage is proper, and error out if you have any
> references to symbols not available in your targetted OS X version:
> `-Wl,-no_weak_imports`.
> 
> If you use that to compile your basic testcase, it will fail as follows:
> 
> $ clang -Wl,-no_weak_imports -mmacosx-version-min=10.8 -isysroot
> /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/
> Developer/SDKs/MacOSX10.12.sdk test2.c
> ld: weak import of symbol '_clock_gettime' not supported because of option:
> -no_weak_imports for architecture x86_64
> clang: error: linker command failed with exit code 1 (use -v to see
> invocation)
> $
> 
> 
> And it will work fine if you target a newer OS X:
> 
> $ clang -Wl,-no_weak_imports -mmacosx-version-min=10.12 -isysroot
> /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/
> Developer/SDKs/MacOSX10.12.sdk test2.c
> $
> 
> BUT, this option relies on availability information in the prototype (time.h
> for clock_gettime). So if you don't use the prototype in the header provided
> by Apple, the symbol will always be "available" to the linker. This is why
> the Autoconf check is broken no matter what we do and we have to work around
> it manually -- it always uses its own broken prototype.

Thanks for the explanation, it was a crazy weekend trying to fix our builds :) You are right, I think -Wl,-no_weak_imports is right now the best alternative to, at least,  make sure binaries are compatible with older versions.