Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

Cannot catch exceptions on Apple M1 #48661

Open Quuxplusone opened 3 years ago

Quuxplusone commented 3 years ago
Bugzilla Link PR49692
Status NEW
Importance P normal
Reported by Jonas Hahnfeld (hahnjo@hahnjo.de)
Reported on 2021-03-23 02:58:33 -0700
Last modified on 2021-06-30 04:10:06 -0700
Version trunk
Hardware PC MacOS X
CC 1101.debian@gmail.com, Axel.Naumann@cern.ch, lhames@gmail.com, llvm-bugs@lists.llvm.org, vvasilev@cern.ch
Fixed by commit(s)
Attachments libunwind-ptrauth.diff (1041 bytes, text/plain)
Blocks
Blocked by
See also
On arm64-apple-darwin20.3.0, consider the following code:
 % cat test.cc
int main() {
  try {
    throw 1;
  } catch (...) { }
}

When compiled into an executable (by either Apple clang or Clang trunk), the
program exits normally:
 % clang++ -O2 throw.cc -o throw
 % ./throw
 % echo $?
0

However, the same code crashes when compiling into LLVM IR and executing via
lli:
 % clang++ -O2 -S -emit-llvm throw.cc
 % lli throw.ll
libc++abi.dylib: terminating with uncaught exception of type int

I've tried all combinations of --jit-kind and --jit-link, none of them worked.
Quuxplusone commented 3 years ago
Part of the problem may be related to "T.isOSDarwin() && .getArch() ==
Triple::aarch64" defaulting to emit only compact unwinding info without
__eh_frame. I tried forcing LLVM to generate that section with
 % git diff
diff --git a/llvm/lib/MC/MCObjectFileInfo.cpp b/llvm/lib/MC/MCObjectFileInfo.cpp
index 155104cddda2..6b045bafdf59 100644
--- a/llvm/lib/MC/MCObjectFileInfo.cpp
+++ b/llvm/lib/MC/MCObjectFileInfo.cpp
@@ -56,9 +56,11 @@ void MCObjectFileInfo::initMachOMCObjectFileInfo(const
Triple &T) {
           MachO::S_ATTR_STRIP_STATIC_SYMS | MachO::S_ATTR_LIVE_SUPPORT,
       SectionKind::getReadOnly());

+#if 0
   if (T.isOSDarwin() &&
       (T.getArch() == Triple::aarch64 || T.getArch() == Triple::aarch64_32))
     SupportsCompactUnwindWithoutEHFrame = true;
+#endif

   if (T.isWatchABI())
     OmitDwarfIfHaveCompactUnwind = true;

and afterwards I see the section being handed over to
RTDyldMemoryManager::registerEHFramesInProcess, but it still fails to catch the
exception.

Unfortunately, I haven't succeeded in building libunwind with debug symbols and
trace through the unwinding to see what's going wrong: It didn't even work to
inject a custom libunwind (neither from LLVM trunk nor the patched version from
https://opensource.apple.com/source/libunwind/) into a compiled executable,
that results in no exceptions being caught either. It appears Apple did some
changes to the exception handling ABI compared to x86_64, we probably need some
of their engineers to help us out here...
Quuxplusone commented 3 years ago

@lhames any recommendation whom to contact here? The fact that providing a custom-built libunwind is a dead end and that lli doesn't get this right either means we really don't know what to do here... Do you know of EH ABI specialists for Apple's AArch64?

Quuxplusone commented 3 years ago

Hi Axel,

This one is on me -- I'll look into supporting compact-unwind, but won't have time to get to it for a couple of weeks.

How urgent is this for you, and are you using ORCv2? Ideally I'll just implement this in JITLink, but that won't help if you're on MCJIT or ORCv1.

Quuxplusone commented 3 years ago
Hey Lang,

See all of that is already a *wealth* of information! :-) Tells us that here we
don't need to hunt our own bug, for once.

We're currently on llvm9's ORCv1; I hope we will be on ORCv2 by the end of the
year.

The current state is pretty painful for M1 users... but it's not mission
critical, our particles will continue to collide. And I can totally relate if
you do this for ORCv2 only. If all fails on our side we might still be able to
learn from what you do for JITLink and try to hack this into llvm9's ORCv1.

Thank you so much for your reply! Looking forward to your fixes in a couple of
weeks, even if in JITLink!

Axel.
Quuxplusone commented 3 years ago
Hi Lang,

> This one is on me -- I'll look into supporting compact-unwind, but won't have
time to get to it for a couple of weeks.

So darwinarm64 is compact unwinding only? Or would there be a chance to get
__eh_frame working, even if not the default for the platform? Do you have
technical information with respect to the "new" unwinding ABI?

Thanks,
Jonas
Quuxplusone commented 3 years ago
Hi Jonas,

> So darwinarm64 is compact unwinding only? Or would there be a chance to get
__eh_frame working, even if not the default for the platform? Do you have
technical information with respect to the "new" unwinding ABI?

I haven't had time to dig in to the details yet. My understanding is that arm64
prefers to emit compact unwind records (they're smaller than eh_frame records),
but can not do so in all cases and falls back to eh_frame records where
necessary. There is probably an option to force the compiler to emit eh_frame
records for all functions, but I have not looked for it yet. It's also possible
that JITLink's current eh-frame support is not sufficient for arm64 yet -- I'm
not sure it uses the same pointer encodings as x86-64. In any case using
compact_unwind is the eventual goal, because we would like to be able to load
objects and archives that weren't compiled specifically for the JIT.

The best sources for compact_unwind that I'm aware of are the apple
compact_unwind_encoding.h header [1], the source for libunwind [2], the source
for ld64 [3].

-- Lang.

[1] https://opensource.apple.com/source/libunwind/libunwind-35.1/include/mach-
o/compact_unwind_encoding.h.auto.html
[2] https://github.com/llvm/llvm-project/tree/main/libunwind
[3] https://opensource.apple.com/source/ld64/
Quuxplusone commented 3 years ago
Ok, thanks for the pointers. As written in comment 1, forcing emission of
__eh_frame doesn't help, and I haven't succeeded in catching exceptions using a
custom version of libunwind either (even not the one from Apple). So there must
have been some changes, and neither JITlink nor RuntimeDyld are currently
prepared to handle them.
Of course, we need support for compact unwinding in the long run to load
arbitrary libraries, but being at least able to catch exceptions of our own
libraries (where we can control the compilation options) would be great in the
short term. I might have another look into the pages you linked, but I'm not
super confident.

Jonas
Quuxplusone commented 3 years ago

Attached libunwind-ptrauth.diff (1041 bytes, text/plain): hacky patch for libunwind

Quuxplusone commented 3 years ago
I poked some more and I think I see where the crash in the system libunwind
comes from: From the accesses I see it appears that unw_set_reg loads a flag or
something from the beginning of the MachO image that contains the exception
handler. This works fine for compiled executables, but is set to the NULL
pointer for dynamically registered FDEs: https://github.com/llvm/llvm-
project/blob/32bc9a9bc3147a86ed43827d29d8f7d7cf05eb4a/libunwind/src/UnwindCursor.hpp#L1981
(the last parameter is dso_base and written into the extra field, which is
documented as "mach_header of mach-o image containing func").

As this doesn't happen in LLVM's libunwind nor in the version available on
opensource.apple.com, I guess there are more modifications Apple did and
there's not much we can do as "users" of the system. Lang, could you help us
get this message to the right team at Apple so they can fix their library?

Thanks,
Jonas
Quuxplusone commented 3 years ago
Quick reminder about this issue: I can still reproduce the original problem
("libc++abi: terminating with uncaught exception of type int") with latest main
and some hacky changes make "lli --jit-kind=mcjit throw.ll" crash in libunwind
as described before. I didn't test my hacky patch to libunwind again, but I
would expect it to "work" as before.

diff --git a/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldMachO.cpp
b/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldMachO.cpp
index 9ca76602ea18..50a3add0c207 100644
--- a/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldMachO.cpp
+++ b/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldMachO.cpp
@@ -324,6 +324,7 @@ void RuntimeDyldMachOCRTPBase<Impl>::registerEHFrames() {
       continue;
     SectionEntry *Text = &Sections[SectionInfo.TextSID];
     SectionEntry *EHFrame = &Sections[SectionInfo.EHFrameSID];
+#if 0
     SectionEntry *ExceptTab = nullptr;
     if (SectionInfo.ExceptTabSID != RTDYLD_INVALID_SECTION_ID)
       ExceptTab = &Sections[SectionInfo.ExceptTabSID];
@@ -338,6 +339,7 @@ void RuntimeDyldMachOCRTPBase<Impl>::registerEHFrames() {
     while (P != End) {
       P = processFDE(P, DeltaForText, DeltaForEH);
     }
+#endif

     MemMgr.registerEHFrames(EHFrame->getAddress(), EHFrame->getLoadAddress(),
                             EHFrame->getSize());
diff --git a/llvm/lib/MC/MCObjectFileInfo.cpp b/llvm/lib/MC/MCObjectFileInfo.cpp
index 1a448f040b3b..60f27aae36c6 100644
--- a/llvm/lib/MC/MCObjectFileInfo.cpp
+++ b/llvm/lib/MC/MCObjectFileInfo.cpp
@@ -57,9 +57,11 @@ void MCObjectFileInfo::initMachOMCObjectFileInfo(const
Triple &T) {
           MachO::S_ATTR_STRIP_STATIC_SYMS | MachO::S_ATTR_LIVE_SUPPORT,
       SectionKind::getReadOnly());

+#if 0
   if (T.isOSDarwin() &&
       (T.getArch() == Triple::aarch64 || T.getArch() == Triple::aarch64_32))
     SupportsCompactUnwindWithoutEHFrame = true;
+#endif

   if (T.isWatchABI())
     OmitDwarfIfHaveCompactUnwind = true;
Quuxplusone commented 3 years ago

Hi Jonas,

I recently ok'd a patch that seems like it matches your suggested fix: https://reviews.llvm.org/D103052

It hasn't landed yet, but I expect to commit it in the next couple of days.

I'll be focusing more attention on arm64 support over the next few months, so I'm hoping we'll see significant improvements in exception handling on that architecture.

Quuxplusone commented 3 years ago
Hi Lang,

yes, that patch fixes one third of the problem, namely not processing FDEs on
AArch64. https://reviews.llvm.org/D103052#2778324 mentions the second problem
of forcing emission of .eh_frame sections that I hacked via
diff --git a/llvm/lib/MC/MCObjectFileInfo.cpp b/llvm/lib/MC/MCObjectFileInfo.cpp
index 1a448f040b3b..60f27aae36c6 100644
--- a/llvm/lib/MC/MCObjectFileInfo.cpp
+++ b/llvm/lib/MC/MCObjectFileInfo.cpp
@@ -57,9 +57,11 @@ void MCObjectFileInfo::initMachOMCObjectFileInfo(const
Triple &T) {
           MachO::S_ATTR_STRIP_STATIC_SYMS | MachO::S_ATTR_LIVE_SUPPORT,
       SectionKind::getReadOnly());

+#if 0
   if (T.isOSDarwin() &&
       (T.getArch() == Triple::aarch64 || T.getArch() == Triple::aarch64_32))
     SupportsCompactUnwindWithoutEHFrame = true;
+#endif

   if (T.isWatchABI())
     OmitDwarfIfHaveCompactUnwind = true;

However, the real blocker (as far as I can tell) is that libunwind distributed
with macOS keeps crashing once you have all that in place. See comments 8 and 9
for more details, and as mentioned in comment 10 I'm still able to reproduce
the issue. Unfortunately, as mentioned before, this is not something we can fix
since Apple didn't even open-source the modifications to libunwind that provoke
the crash...

Jonas