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

ld64.lld.darwinnew unwind info is incorrect #47733

Closed
nico opened this issue Dec 4, 2020 · 7 comments
Closed

ld64.lld.darwinnew unwind info is incorrect #47733

nico opened this issue Dec 4, 2020 · 7 comments
Assignees
Labels
bugzilla Issues migrated from bugzilla lld:MachO

Comments

@nico
Copy link
Contributor

nico commented Dec 4, 2020

Bugzilla Link 48389
Resolution FIXED
Resolved on Feb 23, 2021 19:19
Version unspecified
OS All
CC @gkmhub,@int3,@smeenai
Fixed by commit(s) rG4a5e111aea7ac78190211a2549f8d0d53ee2f01d

Extended Description

…or at least subtly different from ld64.

Repro:

  1. Download https://drive.google.com/file/d/1Gjrg4bQCv3uD9Ucqt4ZtroHrz5v0iCJX/view?resourcekey=0-f53UMW57gtrPlSis2sTjKg and unzip it

  2. ~/src/llvm-project/out/gn/bin/ld64.lld.darwinnew @​response.txt

  3. ./base_unittests --gtest_filter='Exception'

Expected: All 3 tests pass

Actual: CallWithEHFrameTest.CatchExceptionHigher crashes.

If you remove --color-diagnostics from response.txt and link with regular ld, it passes.

The source of the test is here: https://source.chromium.org/chromium/chromium/src/+/master:base/mac/call_with_eh_frame_unittest.mm;l=25?q=CatchExceptionHigher&ss=chromium

The implementation of CallWithEHFrame is here: https://source.chromium.org/chromium/chromium/src/+/master:base/mac/call_with_eh_frame_asm.S;bpv=1;bpt=0 It's assembly with a custom personality routine, so it's possible things are wrong in the code -- but it's been working for 5 years and it works with ld64. https://codereview.chromium.org/1212093002 has the motivation for that code.

@nico
Copy link
Contributor Author

nico commented Dec 4, 2020

assigned to @int3

@nico
Copy link
Contributor Author

nico commented Jan 10, 2021

This is not a subtle bug after all. Exceptions just don't work at all:

% cat throw.cc
int main() {
try {
throw 0;
} catch (int i) {
return i;
}
}
% out/gn/bin/clang++ throw.cc -fuse-ld=lld.darwinnew
lld: warning: Option `-no_deduplicate' is not yet implemented. Stay tuned...
% ./a.out ; echo $?
libc++abi.dylib: terminating with uncaught exception of type int
zsh: abort ./a.out
134

Works fine with ld64.

@nico
Copy link
Contributor Author

nico commented Jan 10, 2021

ObjC exceptions also don't work:

% cat throw.m
int main() {
@​try {
@​throw (void*)0;
} @​catch (id i) {
return i;
}
}
% out/gn/bin/clang++ throw.m -framework Foundation -fuse-ld=lld.darwinnew
throw.m:5:12: warning: incompatible pointer to integer conversion returning 'id' from a function with result type 'int' [-Wint-conversion]
return i;
^
1 warning generated.
lld: warning: Option `-no_deduplicate' is not yet implemented. Stay tuned...
% ./a.out
2021-01-10 13:09:10.871 a.out[73775:15821192] *** Terminating app due to uncaught exception of class 'nil'
libc++abi.dylib: terminating with uncaught exception of type nil
zsh: abort ./a.out

(Also fine with ld64.)

@int3
Copy link
Contributor

int3 commented Jan 10, 2021

Yeah, the __eh_frame support is not done yet (I was just asking gkm about it yesterday).

@nico
Copy link
Contributor Author

nico commented Feb 9, 2021

I see some LSDA / personality stuff landed. Is this expected to work now, or not yet?

@int3
Copy link
Contributor

int3 commented Feb 9, 2021

The simple examples work, but the CallWithEHFrameTest still fails. To be precise, when linking, we hit the "too many personalities for compact unwind to encode" error. Just to see what would happen, I turned that error into a warning, and then ran the resulting binary -- CatchExceptionHigher passes, but CatchExceptionLower fails. So we're not quite there yet.

@int3
Copy link
Contributor

int3 commented Feb 23, 2021

Figured it out :) https://reviews.llvm.org/D97245

@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 lld:MachO
Projects
None yet
Development

No branches or pull requests

2 participants