Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

When _XOPEN_SOURCE is set,<cstdlib> and <ctime> started to fail on macOS, missing ::aligned_alloc and ::timespec_get #46552

Closed
stbergmann opened this issue Aug 17, 2020 · 8 comments
Assignees
Labels
bugzilla Issues migrated from bugzilla libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.

Comments

@stbergmann
Copy link
Collaborator

Bugzilla Link 47208
Resolution FIXED
Resolved on Sep 02, 2020 09:23
Version unspecified
OS Linux
CC @ldionne,@mclow
Fixed by commit(s) 5201b96

Extended Description

On macOS, with recent LLVM 12 trunk commit 67dfba9 " [libc++] Provide std::aligned_alloc and std::timespec_get on Apple platforms":

$ cat test.cc
#include
#include

$ .../llvm/inst/bin/clang++ -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.15.sdk -std=c++17 -fsyntax-only test.cc

$ .../llvm/inst/bin/clang++ -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.15.sdk -std=c++17 -fsyntax-only test.cc -D_XOPEN_SOURCE=600
In file included from test.cc:1:
.../llvm/inst/bin/../include/c++/v1/cstdlib:158:9: error: no member named 'aligned_alloc' in the global namespace
using ::aligned_alloc;
~~^
In file included from test.cc:2:
.../llvm/inst/bin/../include/c++/v1/ctime:76:9: error: no member named 'timespec_get' in the global namespace; did you mean 'timespec'?
using ::timespec_get;
~~^
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.15.sdk/usr/include/sys/_types/_timespec.h:33:1: note: 'timespec' declared here
_STRUCT_TIMESPEC
^
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.15.sdk/usr/include/sys/_types/_timespec.h:29:40: note: expanded from macro '_STRUCT_TIMESPEC'
#define _STRUCT_TIMESPEC struct timespec
^
2 errors generated.

...and similarly for other values of _XOPEN_SOURCE, like 700. I noticed that when building ICU 67.1 as part of building recent master LibreOffice, where some place in the ICU sources sets _XOPEN_SOURCE to 600.

  • One approach to fix this would be to make the condition in libcxx match the condition in Apple's SDK headers, with something like

diff --git a/libcxx/include/__config b/libcxx/include/__config
index d7b6a2acaef..fb5ca54493b 100644
--- a/libcxx/include/__config
+++ b/libcxx/include/__config
@@ -327,6 +327,10 @@

define _LIBCPP_USING_DEV_RANDOM

#endif

+#if defined(APPLE)
+# include <sys/cdefs.h>
+#endif
+
#if !defined(_LIBCPP_LITTLE_ENDIAN) && !defined(_LIBCPP_BIG_ENDIAN)

include <endian.h>

if __BYTE_ORDER == __LITTLE_ENDIAN

@@ -383,7 +387,8 @@

elif defined(APPLE)

  // timespec_get and aligned_alloc were introduced in macOS 10.15 and
  // aligned releases

-# if (ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED >= 101500 ||
+# if __DARWIN_C_LEVEL >= __DARWIN_C_FULL && \

  •    (__ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__ >= 101500 || \
        __ENVIRONMENT_IPHONE_OS_VERSION_MIN_REQUIRED__ >= 130000 || \
        __ENVIRONMENT_TV_OS_VERSION_MIN_REQUIRED__ >= 130000 || \
        __ENVIRONMENT_WATCH_OS_VERSION_MIN_REQUIRED__ >= 60000)
    
  • Another approach could be to change Apple's SDK headers, so that for C++ ::aligned_alloc and ::timespec_get would get declared regardless of any _XOPEN_SOURCE?

  • And yet another approach could be to declare that _XOPEN_SOURCE must not be defined when compiling C++ source (after all, at least technically it is an identifier that is reserved to the implementation), and instead fix ICU.

Thoughts how to proceed?

@stbergmann
Copy link
Collaborator Author

assigned to @ldionne

@ldionne
Copy link
Member

ldionne commented Sep 1, 2020

Thanks a lot for the heads up and the analysis, Stephan.

My inclination would be that the SDK headers should provide those declarations regardless of _XOPEN_SOURCE, since otherwise it looks like the implementation is not a valid C99 implementation when _XOPEN_SOURCE is defined. For the sake of keeping things simple, I like to push towards libc++ requiring a valid C99 implementation under it in order to work.

I'll ask the libc folks here at Apple what they think -- I'm sure there's a reason for the _XOPEN_SOURCE dance in the SDK.

@ldionne
Copy link
Member

ldionne commented Sep 1, 2020

So, actually, in the latest SDK, the condition for aligned_alloc is:

#if (__DARWIN_C_LEVEL >= __DARWIN_C_FULL) || \
    (defined(__STDC_VERSION__) && __STDC_VERSION__ >= 201112L) || \
    (defined(__cplusplus) && __cplusplus >= 201703L)
void    *aligned_alloc(size_t __alignment, size_t __size);
#endif

But for timespec_get it's still:

#if (__DARWIN_C_LEVEL >= __DARWIN_C_FULL) && \
        ((defined(__STDC_VERSION__) && __STDC_VERSION__ >= 201112L) || \
        (defined(__cplusplus) && __cplusplus >= 201703L))
int timespec_get(struct timespec *ts, int base);
#endif

So it looks like it was meant to be fixed, since they fixed in for aligned_alloc. I'll report the issue for timespec_get.

In the meantime, it does look like we should add a #if __DARWIN_C_LEVEL >= __DARWIN_C_FULL guard to whether libc++ provides the declarations.

@ldionne
Copy link
Member

ldionne commented Sep 1, 2020

I just committed a fix:

commit 99f3b23
Author: Louis Dionne ldionne@apple.com
Date: Tue Sep 1 15:05:33 2020 -0400

[libc++] Workaround timespec_get not always being available in Apple SDKs

timespec_get is not available in Apple SDKs when (__DARWIN_C_LEVEL >= __DARWIN_C_FULL)
isn't true, which leads to libc++ trying to import ::timespec_get into
namespace std when it's not available. This issue has been reported to
Apple's libc, but we need a workaround in the meantime.

llvm/llvm-project#46552 
rdar://68157284

Can you please let me know if it fixes your issue? It does fix the reproducer I created locally (and checked into the test suite).

@ldionne
Copy link
Member

ldionne commented Sep 1, 2020

Note that my commit only tackles timespec_get, since aligned_alloc has been fixed already. What SDK are you using?

@stbergmann
Copy link
Collaborator Author

Note that my commit only tackles timespec_get, since aligned_alloc has been
fixed already. What SDK are you using?

I'm using /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.15.sdk from Xcode Version 11.6 (11E708), which appears to be the latest one publicly available?

@stbergmann
Copy link
Collaborator Author

Note that my commit only tackles timespec_get, since aligned_alloc has been
fixed already. What SDK are you using?

I'm using
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/
Developer/SDKs/MacOSX10.15.sdk from Xcode Version 11.6 (11E708), which
appears to be the latest one publicly available?

Found Xcode 12 beta 6 now, which has the updated MacOSX11.0.sdk/usr/include/malloc/_malloc.h, and with which my original issue is indeed fixed. Thanks!

@ldionne
Copy link
Member

ldionne commented Sep 2, 2020

Note that 99f3b23 was reverted because it broke the modules build, but I just checked-in this commit which should fix the bug without breaking modules:

commit 5201b96
Author: Louis Dionne ldionne@apple.com
Date: Wed Sep 2 10:36:48 2020 -0400

[libc++] Re-apply the workaround for timespec_get not always being available in Apple SDKs

This commit re-applies 99f3b231cb21, which was reverted in 814242572731
because it broke the modules build. The modules failure was a circular
dependency between the Darwin module and __config. Specifically, the
issue was that if <__config> includes a system header, the std_config
module depends on the Darwin module. However, the Darwin module already
depends on the std_config header because some of its headers include
libc++ headers like <ctype.h> (they mean to include the C <ctype.h>,
but libc++ headers are first in the header search path).

This is fixed by moving the workaround to <ctime> only.

llvm/llvm-project#46552 
rdar://68157284

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 10, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

No branches or pull requests

2 participants