Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

Wrong compact unwind information for functions using stack probes #45782

Open Quuxplusone opened 4 years ago

Quuxplusone commented 4 years ago
Bugzilla Link PR46813
Status NEW
Importance P enhancement
Reported by Alex Crichton (alex@crichton.co)
Reported on 2020-07-22 14:07:58 -0700
Last modified on 2020-07-24 21:47:20 -0700
Version trunk
Hardware PC MacOS X
CC aemerson@apple.com, craig.topper@gmail.com, llvm-bugs@lists.llvm.org, llvm-dev@redking.me.uk, rjmccall@apple.com, spatel+llvm@rotateright.com
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also

In rust-lang/rust we've for the longest time enabled frame pointers on our macOS targets, but we're looking at that again recently and trying to figure out why we're still doing this. An attempt to disable frame pointers, however, led to segfaults during unwinding. We investigated this a bit and came up on this issue.

The issue here we found is that the compact unwind information for a function with a large stack frame is incorrect if the "probe-stack" attribute is used. Namely this IR:

target triple = "x86_64-apple-macosx10.7.0"

define void @foo() #0 { start: %_huge_stack = alloca [1024 x i64], align 8 call void @bar() ret void }

declare void @bar()

attributes #0 = { uwtable "probe-stack"="__probestack" }

when compiled yields:

$ llc unwind.ll -o foo.o -filetype=obj
$ llvm-objdump foo.o -uS
foo.o:  file format mach-o 64-bit x86-64

Disassembly of section __TEXT,__text:

0000000000000000 <_foo>:
./build/bin/llvm-objdump: warning: 'foo.o': failed to parse debug information for foo.o
       0: b8 08 20 00 00                movl    $8200, %eax
       5: e8 00 00 00 00                callq   0xa <_foo+0xa>
       a: 48 29 c4                      subq    %rax, %rsp
       d: e8 00 00 00 00                callq   0x12 <_foo+0x12>
      12: 48 81 c4 08 20 00 00          addq    $8200, %rsp
      19: c3                            retq
Unwind info:

Contents of __compact_unwind section:
  Entry at offset 0x0:
    start:                0x0 _foo
    length:               0x1a
    compact encoding:     0x03032000

Here the compact unwind information for foo is 0x03032000, where the tag (0x3000000) indicates UNWIND_X86_64_MODE_STACK_IND, which according to online documentation says:

// UNWIND_X86_64_MODE_STACK_IND: // A "frameless" (RBP not used as frame pointer) function large constant // stack size. This case is like the previous, except the stack size is too // large to encode in the compact unwind encoding. Instead it requires that // the function contains "subq $nnnnnnnn,RSP" in its prolog. The compact // encoding contains the offset to the nnnnnnnn value in the function in // UNWIND_X86_64_FRAMELESS_STACK_SIZE.

For this function, however, the subq instruction doesn't exist with a constant, but rather %rax which comes from __probestack

Quuxplusone commented 4 years ago

Stack probing on macOS isn't upstream in LLVM. Are you reporting a bug against Apple clang?

Quuxplusone commented 4 years ago

Oh, it seems this actually works/reproduces on trunk too.

Quuxplusone commented 4 years ago
We've discussed this internally, and I don't think there's anything we can do
here except:
* fall back to using DWARF CFI
* force using a frame pointer if a function needs a stack check in the prolog

It seems we should just force using an FP here.
Quuxplusone commented 4 years ago

Ah yes this was indeed against upstream LLVM (Rust is using LLVM 10 at the moment, but as you've seen this reproduces on trunk). FWIW although this may not be super well tested in Clang we've been using the probestack feature for some time now to get stack protection in Rust programs.

I think either solution would be fine for us, but is it easy to switch to DWARF CFI for these functions? If stack probes are used they're for large stack frames which typically mean large functions that may be under enough register pressure that an extra one could possibly help. In any case though freeing it up for other functions regardless would still be great!

Also I think that there's a new "probe-stack"="inline-asm" in upstream LLVM 11 (and trunk) which we'll probably switch to in the future as well. I haven't tested but I suspect a similar fix may be needed for that as well (either switching to CFI or using a base pointer)