This is an archive of the discontinued LLVM Phabricator instance.

Differential D141184

[libc++] Fix aligned_alloc usage for Android
ClosedPublic

Authored by smeenai on Jan 6 2023, 11:36 PM.

Details

Reviewers
arichardson
danalbert
rprichard
philnik
Group Reviewers
Restricted Project
Commits
rG49c8f903efea: [libc++] Fix aligned_alloc usage for Android
Summary

Android only provides this function on API 28+; fix libc++ builds when
targeting older API levels.

Diff Detail

Event Timeline

smeenai created this revision.Jan 6 2023, 11:36 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 6 2023, 11:36 PM
Herald added subscribers: danielkiss, krytarowski. · View Herald Transcript
smeenai requested review of this revision.Jan 6 2023, 11:36 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 6 2023, 11:36 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
Herald added a subscriber: libcxx-commits. · View Herald Transcript
smeenai mentioned this in D138196: [libc++] Use aligned_alloc instead of posix_memalign for C++17.Jan 6 2023, 11:36 PM
Harbormaster completed remote builds in B206253: Diff 487058.Jan 7 2023, 12:56 AM
philnik accepted this revision.Jan 7 2023, 1:06 AM
philnik added a subscriber: philnik.

LGTM assuming D139147 is being worked on.

This revision is now accepted and ready to land.Jan 7 2023, 1:06 AM
arichardson accepted this revision.Jan 7 2023, 3:00 AM
hans added a subscriber: hans.Jan 10 2023, 2:24 AM
smeenai added a comment.Jan 10 2023, 6:25 AM
In D141184#4033243, @philnik wrote:

LGTM assuming D139147 is being worked on.

I'm not involved with that effort directly (I work on Meta's Android compiler team, whereas @rprichard is on the Android NDK team at Google), but I understand wanting a buildbot to test this configuration. I can't speak on his behalf, but I trust that Ryan will work on that actively once holiday season is wrapped up.

Closed by commit rG49c8f903efea: [libc++] Fix aligned_alloc usage for Android (authored by smeenai). · Explain WhyJan 10 2023, 6:26 AM
This revision was automatically updated to reflect the committed changes.
smeenai added a commit: rG49c8f903efea: [libc++] Fix aligned_alloc usage for Android.
rprichard added a comment.Jan 10 2023, 11:26 AM
In D141184#4039656, @smeenai wrote:
In D141184#4033243, @philnik wrote:

LGTM assuming D139147 is being worked on.

I'm not involved with that effort directly (I work on Meta's Android compiler team, whereas @rprichard is on the Android NDK team at Google), but I understand wanting a buildbot to test this configuration. I can't speak on his behalf, but I trust that Ryan will work on that actively once holiday season is wrapped up.

Yeah I'll be resuming work on that soon.

The D139147 configuration currently only tests a configuration where libc++ is built against API21 and run on an API33 emulator. I think(?) that would be sufficient to catch this issue because the API21 headers and libc.so stub wouldn't have an aligned_alloc function? The emulator image would have one in its /system/lib64/libc.so though.

This change looks OK to me.

Revision Contents

PathSize
libcxx/
include/
3 lines

Diff 487058

libcxx/include/__config

Show First 20 Lines • Show All 721 Lines • ▼ Show 20 Lines
// It is not yet possible to use aligned_alloc() on all Apple platforms since // It is not yet possible to use aligned_alloc() on all Apple platforms since
// 10.15 was the first version to ship an implementation of aligned_alloc(). // 10.15 was the first version to ship an implementation of aligned_alloc().
# if defined(__APPLE__) # if defined(__APPLE__)
# if (defined(__ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__) && \ # if (defined(__ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__) && \
__ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__ < 101500) __ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__ < 101500)
# define _LIBCPP_HAS_NO_C11_ALIGNED_ALLOC # define _LIBCPP_HAS_NO_C11_ALIGNED_ALLOC
# endif # endif
# elif defined(__ANDROID__) && __ANDROID_API__ < 28
// Android only provides aligned_alloc when targeting API 28 or higher.
# define _LIBCPP_HAS_NO_C11_ALIGNED_ALLOC
# endif # endif
# if defined(__APPLE__) || defined(__FreeBSD__) # if defined(__APPLE__) || defined(__FreeBSD__)
# define _LIBCPP_HAS_DEFAULTRUNELOCALE # define _LIBCPP_HAS_DEFAULTRUNELOCALE
# endif # endif
# if defined(__APPLE__) || defined(__FreeBSD__) || defined(__sun__) # if defined(__APPLE__) || defined(__FreeBSD__) || defined(__sun__)
# define _LIBCPP_WCTYPE_IS_MASK # define _LIBCPP_WCTYPE_IS_MASK
▲ Show 20 Lines • Show All 485 Lines • Show Last 20 Lines