clang: Enable unwind tables on !amd64

[cem]

Summary

There doesn't seem to be much sense in defaulting "on" unwind tables on amd64
and not on other arch. It causes surprising differences between platforms,
such as the PR below.

Prior to this change, FreeBSD inherited the default implementation of the
method from the Gnu.h Generic_Elf => Generic_GCC parent class, which returned
true only for amd64 targets. Override that and opt on always, similar to,
e.g., NetBSD.

PR: 241562

Test Plan

I would probably also be fine with 'return false;' plus Make rules to add the
appropriate options by default, but that seems more convoluted.

I did not check if LLVM has any unit tests that this breaks nor attempt to run
the LLVM unit tests. (And of course, I did not add any.)

I'm running i386 tinderbox now and will run all when/if i386 is successful.

[cem]

FWIW, tinderbox is clean.

[emaste]

I have no objection and find it surprising that the default is different for amd64 vs. all other architectures. I am not sure if we should instead default to off everywhere though, and enable it via cmdline flags.

It has been this way in Clang for over a decade - https://github.com/llvm/llvm-project/commit/0a248bb5c2d3a4d286867f407f6a96b3e361c6bb

Does GCC behave this way, enabling only for amd64?

[emaste]

Also how does this interact with (32-bit) arm which uses its own unwind format?

[cem]

In D22252#486603, @emaste wrote:

I am not sure if we should instead default to off everywhere though, and enable it via cmdline flags.

The counterargument to that position is, I think, our compiler flags coverage isn't perfect, especially for ports, and we'd like unwinding to work universally without surprises (the failure mode is silent failure as soon as the DWARF unwinder reaches an uninstrumented binary). There might be a few -fno-unwind-tables hiding somewhere, but I expect it's rarer than hardcoded CFLAGS="-O3 -funroll-loops" (or that kind of thing).

One fringe benefit might be that we could enable -fno-frame-pointer on i386, which should be significant. Currently we tie up %ebp as a framepointer on that platform, and it is notoriously register-starved. (If Dtrace is a factor, either it knows about eh_frame unwinding already, or could learn.) That's obviously beyond the scope of this commit.

It has been this way in Clang for over a decade - https://github.com/llvm/llvm-project/commit/0a248bb5c2d3a4d286867f407f6a96b3e361c6bb

Yeah, I noticed it was quite old. At some point that comment is changed to one that says something to the tune of, "our EH generation is broken on !x86_64," so amd64-only kind of makes sense. But later in time, the comment was removed, and the default unchanged. I don't have a Clang git tree hanging around so I wasn't able to find the commit that removed the comment (I just poked around on Github for the other context with blame/etc), but that might be interesting (or not).

Does GCC behave this way, enabling only for amd64?

I don't know :-). I don't have the source tree handy and don't really want to look through it deeply. If -v is to believed, the answer is "no." On AMD64 I see:

$ gcc9 -O2 -c -v /dev/null
...
COLLECT_GCC_OPTIONS='-O2' '-c' '-v' '-mtune=generic' '-march=x86-64'

$ i386-unknown-freebsd13.0-gcc -O2 -c -v /dev/null
...
COLLECT_GCC_OPTIONS='-O2' '-c' '-v' '-mtune=generic' '-march=i486'

In contrast (clang from before this change):

$ clang90 -c -v -x c /dev/null
...
"/usr/local/llvm90/bin/clang-9" -cc1 -triple x86_64-portbld-freebsd13.0 ...  -mconstructor-aliases -munwind-tables -fuse-init-array -target-cpu x86-64 ...
...

$ clang90 -m32 -c -v -x c /dev/null
...
"/usr/local/llvm90/bin/clang-9" -cc1 -triple i386-portbld-freebsd13.0 ...  -mconstructor-aliases -fuse-init-array -target-cpu i586 ...

[emaste]

might be interesting (or not)

Maybe not, here's where the GNU one was removed: https://github.com/llvm/llvm-project/commit/e8bd4e50e4a30e1a046b4f6a8768dce8569a982f There's a comment in CrossWindows.cpp that "LLVM currently does not know how to emit" non-x86 unwind tables still.

If -v is to believed, the answer is "no."

So it seems GCC never enables it by default?

[cem]

In D22252#486604, @emaste wrote:

Also how does this interact with (32-bit) arm which uses its own unwind format?

Good question! This gets translated into the -munwind-tables option to the cc1 compiler phase, which is parsed as the CodeGenOpts.UnwindTables flag. That flag is then used in a single place in Clang to add the ::UWTable attribute to generated function LLVM IR. After that, it's up to the backend to do something reasonable with that attribute.

Here's the first bit:

$ cat d.c
int
foo(int bar)
{
        return (5);
}

$ clang -S -emit-llvm -o - -funwind-tables  ./d.c
...
; Function Attrs: noinline nounwind optnone uwtable
define dso_local i32 @foo(i32 %bar) #0 {
entry:
  %bar.addr = alloca i32, align 4
  store i32 %bar, i32* %bar.addr, align 4
  ret i32 5
}
...

$ clang -S -emit-llvm -o - -fno-unwind-tables  ./d.c
...
; Function Attrs: noinline nounwind optnone
define dso_local i32 @foo(i32 %bar) #0 {
entry:
  %bar.addr = alloca i32, align 4
  store i32 %bar, i32* %bar.addr, align 4
  ret i32 5
}

These attributes are defined here: https://llvm.org/docs/LangRef.html . (The confusingly named nounwind just indicates that the static analysis pass has concluded that this function doesn't raise an exception.) The uwtable attribute indicates that unwind information should be generated for this function even if exceptions don't pass through it (either because the function only calls nothrow-ish code, or because the language doesn't have exceptions, like C).

So, broadly speaking I think those are the correct IR attributes. On x86, the uwtable attribute, when enabled, translates into .cfi_startproc, etc, directives in the assembler:

$ clang -S -o - -funwind-tables  ./d.c
...
foo:                                    # @foo
        .cfi_startproc
        pushq   %rbp
        .cfi_def_cfa_offset 16
        .cfi_offset %rbp, -16
...
        retq
        .cfi_endproc

I'm not sure how to find which target triple ARM uses by default. That should determine which ABI is used. There is a ARM::computeDefaultTargetABI function in LLVM that seems to select aapcs-linux on some Linux-y platforms and OpenBSD, apcs-gnu on NetBSD, and aapcs by default or if the target triple has EABI or EABIHF in it. So... let's assume aapcs I guess?

$ clang -target armv7-unknown-freebsd13.0 -mabi=aapcs -S -emit-llvm -o - -funwind-tables  ./d.c
...
; Function Attrs: noinline nounwind optnone uwtable
define dso_local arm_aapcscc i32 @foo(i32 %bar) #0 {
entry:

So far so good; asm:

$ clang -target armv7-unknown-freebsd13.0 -mabi=aapcs -S -o - -funwind-tables  ./d.c
        .text
        .syntax unified
        .eabi_attribute 67, "2.09"      @ Tag_conformance
        .eabi_attribute 6, 10   @ Tag_CPU_arch
        .eabi_attribute 7, 65   @ Tag_CPU_arch_profile
...

It doesn't emit DWARF CFI directives, so, I think that's at least not *wrong*... I don't see any additional unwind metadata with -funwind-tables vs -fno-unwind-tables, though. It might just be this function is too simple to need sophisticated unwinding. After making the function significantly more complicated (0x40 bytes of stack allocated in -O0), I still don't get any additional metadata. It's possible EABI unwinding works in the absense of per-function metadata; I don't know anything about it.

Anyway, I think the short takeaway is that there doesn't seem to be any regression for 32-bit ARM, as far as I can tell.

[cem]

In D22252#486649, @emaste wrote:

Maybe not, here's where the GNU one was removed: https://github.com/llvm/llvm-project/commit/e8bd4e50e4a30e1a046b4f6a8768dce8569a982f There's a comment in CrossWindows.cpp that "LLVM currently does not know how to emit" non-x86 unwind tables still.

Ah, I think I confused myself looking at f561abab56801665997217099e5652e625e81cb2, which contains the CrossWindows.cpp line. Maybe it's only on Windows that LLVM doesn't know how to emit non-x86 unwind? That seems plausible, given Windows' weird SEH thing.

So it seems GCC never enables it by default?

I wouldn't say I've exhaustively researched it by any means, but as far as I can tell, yep — it doesn't.

[emaste]

Thanks for digging into this further. I have no objection to this and think it's probably the right thing to do, but would like to hear @dim's opinion.

We should at least also make sure this goes upstream (for FreeBSD); there's probably a broader discussion to be had on LLVM's defaults across archs and operating systems too (but I don't expect you to pursue that).

[dim]

Yeah, I'm fine with this too. And indeed, we need to upstream this (with tests). :)