Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

Exception unwinding fails with shadow call stack #44845

Open Quuxplusone opened 4 years ago

Quuxplusone commented 4 years ago
Bugzilla Link PR45875
Status NEW
Importance P normal
Reported by Ambre Williams (ambre@google.com)
Reported on 2020-05-11 08:23:44 -0700
Last modified on 2020-05-12 16:13:03 -0700
Version trunk
Hardware Other other
CC compnerd@compnerd.org, jgorbe@google.com, leonardchan@google.com, llvm-bugs@lists.llvm.org, peter@pcc.me.uk, roland@hack.frob.com, saugustine@google.com
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also
With a version of libunwind built for aarch64-unknown-fuchsia (which enables
the shadow call stack by default), the unwinding information generated for
_Unwind_RaiseException does not adjust x18. This breaks exception handling: the
function that catches the exception returns to the wrong place.

Here's an example, using the version of llvm shipped in the fuchsia tree:
Source:
#include <iostream>

int main() {
  void* x18;
  try {
    asm volatile("mov %0, x18" : "=r"(x18) : :);
    std::cerr << "will throw, x18=" << x18 << std::endl;
    throw "something";
  } catch (...) {
    asm volatile("mov %0, x18" : "=r"(x18) : :);
    std::cerr << "caught, x18=" << x18 << std::endl;
  }
  return 0;
}

Actual output:
will throw, x18=0x779ed45010
caught, x18=0x779ed45018
<CRASH>

Expected output:
will throw, x18=0x779ed45010
caught, x18=0x779ed45010

Looking at the library, _Unwind_RaiseException starts with the shadow call
stack prolog:
0000000000000000 <_Unwind_RaiseException>:
       0: 5e 86 00 f8                   str     x30, [x18], #8
       4: fd 7b bd a9                   stp     x29, x30, [sp, #-48]!

but it is omitted by its unwind information. I'm not sure how to compile
libunwind for fuchsia myself, but the compiled version I used can be found
under lib/aarch64-unknown-fuchsia/c++/libc++.a in the archive here:
https://chrome-infra-packages.appspot.com/p/fuchsia/third_party/clang/linux-amd64/+/RJTCpB4rJ4IkRZuUWvaLtgjL6zz1HcitXQOIG47dvh8C

This seems to be fixed by compiling UnwindLevel1.c with -fexceptions. I'm not
sure of what the difference should be between using -fexceptions or just -
funwind-tables, but it seems that the dwarf instruction to update x18 is not
emitted when a function is marked nounwind (https://github.com/llvm/llvm-
project/blob/2481f26ac3f228cc085d4d68ee72dadc07afa48f/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp#L2147).
Quuxplusone commented 4 years ago

Based on the description of nounwind in LangRef.rst, it should always be trumped by -fasynchronous-unwind-tables, which should always be on for Fuchsia targets.

Quuxplusone commented 4 years ago

AFAICT no other uses of addFrameInst are gated on the function nounwind attribute.

Quuxplusone commented 4 years ago

https://reviews.llvm.org/D54988 seems to only add the nounwind check to avoid emitting cfi directives because it's not "currently required" to adjust the scs register. Would a proper fix just be to disable remove the check?

Quuxplusone commented 4 years ago

I put a patch up on https://reviews.llvm.org/D79822 which removes the nounwind check.