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

[Bug] potential upcoming ABI break in libc++ #1963

Closed
DanAlbert opened this issue Oct 30, 2023 · 3 comments
Closed

[Bug] potential upcoming ABI break in libc++ #1963

DanAlbert opened this issue Oct 30, 2023 · 3 comments
Labels

Comments

@DanAlbert
Copy link
Member

Description

tl;dr: do not use std::expected at ABI boundaries

https://discourse.llvm.org/t/abi-break-in-libc-for-a-17-x-guidance-requested/74483 warns about an impending ABI break required to fix UB in std::expected.

The good news is that atm the libc++ folks think the ABI break will only be rarely noticeable:

the ABI break actually only breaks some very specific cases. You have to have a type with more than one byte of tail padding and have the expected marked with [[no_unique_address]]

https://discord.com/channels/636084430946959380/636732894974312448/1168654171940065330

The discourse thread suggest we cherry-pick llvm/llvm-project#68733 to fix the UB, but they want to make a more complete (better performance?) fix going forward that will be an ABI break.

I don't think that's set in stone yet, but if you want to defend against incompatibility with future NDKs, don't use std::expected at ABI boundaries (that's mostly a problem for middleware vendors, if you build all your C++ code as part of your app, you should be safe) with r26 or older. We're considering potentially disabling std::expected in an r26 point release to avoid the bad ABI getting too far out into the wild.

Upstream bug

llvm/llvm-project#70708

Commit to cherry-pick

llvm/llvm-project#68733

Affected versions

r26

Canary version

No response

Host OS

Linux, Mac, Windows

Host OS version

any

Affected ABIs

armeabi-v7a, arm64-v8a, x86, x86_64

@DanAlbert DanAlbert added the bug label Oct 30, 2023
@DanAlbert DanAlbert changed the title [Bug]: potential upcoming ABI break in libc++ [Bug] potential upcoming ABI break in libc++ Oct 30, 2023
@github-project-automation github-project-automation bot moved this to Triaged in NDK r26c Oct 30, 2023
@DanAlbert DanAlbert removed this from NDK r26c Jan 16, 2024
@DanAlbert
Copy link
Member Author

Not a regression, so whether or not this is included in r26c will depend on whether it's a clean cherry-pick or not.

@github-project-automation github-project-automation bot moved this to Triaged in NDK r26c Jan 16, 2024
@pirama-arumuga-nainar
Copy link
Collaborator

Doesn't cherry-pick cleanly to r26's toolchain. Safer to punt to r27 (where this will apply cleanly: branch point: Oct 25, fix merge date: Oct 30).

@DanAlbert DanAlbert moved this from Triaged to Needs prebuilt update in NDK r26c Jan 16, 2024
@DanAlbert DanAlbert moved this to Needs prebuilt update in NDK r27 Jan 16, 2024
@DanAlbert DanAlbert removed this from NDK r26c Jan 17, 2024
@DanAlbert
Copy link
Member Author

SGTM, thanks for checking.

@github-project-automation github-project-automation bot moved this from Needs prebuilt update to Merged in NDK r27 Mar 12, 2024
@github-project-automation github-project-automation bot moved this to Prebuilts submitted in LLVM Aug 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Prebuilts submitted
Status: Merged
Development

No branches or pull requests

2 participants