In D128387#3603205, @vitalybuka wrote:

Could you please move refactoring (like extraction emitPrologue) into a separate CL?

We also need one test for runtime and one test for generated IR.

LGTM. Should probably add [llvm] to the subject also. Generally it's helpful for immediately knowing which subrepo you're touching.

LGTM. Should also probably add [NFC] to the start of the subject

Added tests and this patch doesn't contain refactoring.

Feedback needed on:
this namespace approach will result in tooling compression methods being set at compile time, however we may want to aim for a runtime configurable approach in the future, likely a new revision of the compressed data formats that llvm interfaces with with outside tools IE:

• clang ast serialization code could be changed so compressed data could have a flag indicating a compression type.
• profile data code could be changed so compressed data could have a flag indicating a compression type as well.

I think this might be the cause of the many libc++ static_assert failures we're seeing on our builders: https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-linux-x64/b8810079977358640657/overview

In D128567#3609635, @leonardchan wrote:

Won't be landing this until I'm sure any new tests that will be running from this won't fail locally.

An alternative approach to this is update this bit of cmake in compiler-rt/CMakeLists.txt:

Won't be landing this until I'm sure any new tests that will be running from this won't fail locally.

*ping*

*ping*

*ping* any more comments?

Hi, I suspect that either this patch or 529a0570f7e8c5144bd3ad057e43f00e3af58d1b led to the test failures we're seeing on our clang windows builders (https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-windows-x64/b8818245900241242433/test-results):

In D114053#3296128, @jhenderson wrote:

Can this be abandoned now?

In D119201#3303275, @phosek wrote:

Have you checked if all the tests are still passing?

Hi, I suspect that this patch (or D117815) led to the build failure we're seeing on our builders (https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-linux-x64/b8824480583333895809):

It looks like this (expectedly) broke our downstream builder (https://ci.chromium.org/ui/p/fuchsia/builders/ci/clang_toolchain.ci.core.x64-debug/b8824668235249206913/overview). We can do a soft transition by checking against the clang git revision in our build, then setting a macro if the clang includes this change, then toggling between types based on the macro, but this seems pretty messy. For future commits, would this perhaps constitute an update to INSTR_PROF_RAW_VERSION? If that's mainly reserved for binary format changes, would it perhaps be a good idea to introduce a macro for source-level API changes? While the binary format hasn't changed, downstream projects that use InstrProfData.inc could break from stuff like this.

Will file a bug also once the bugzilla migration finishes.

I meant to commandeer https://reviews.llvm.org/D114818 rather than make a new patch.

Hi. I think the build issue we're seeing (https://ci.chromium.org/ui/p/fuchsia/builders/ci/clang_toolchain.ci.core.x64-debug-subbuild/b8829019800818975281/overview) is caused by this patch:

In D114818#3163688, @mstorsjo wrote:

In D114818#3162717, @leonardchan wrote:

Then the test passes, but it looks like unwinder skips past func (it jumps from _Z18crashing_leaf_funcv to main). Perhaps all this hints at an underlying issue in libunwind?

Is this a case where a function ends with a call instruction, and the return address points at the address after the call instruction, which would be the first instruction of the next function?

Can you provide disassembly of the linked binary (of the relevant functions) plus corresponding unwind info dumps (from e.g. llvm-dwarfdump)?

Interesting it works on Ubuntu20.04/Aarch64, I compile the code with ToT clang or gcc-10.
could you share an objdump of main?

In D114818#3162689, @ldionne wrote:

In D114818#3162675, @vvereschaka wrote:

Hi @ldionne, @leonardchan

I ran these changes on Linux (Ubuntu 18.04)/Aarch64 board. The test gets failed because of missed 'main':

ubuntu@jetson8:/tmp$ ./t.tmp.exe 
info.dli_sname: _Z14signal_handleri
info.dli_sname: __kernel_rt_sigreturn
info.dli_sname: _Z18crashing_leaf_funcv
info.dli_sname: __libc_start_main

I haven't yet responded on D114385, but it looks like using __builtin_trap() still breaks the test for us because the unwinder doesn't iterate over the frame for main. Instead it goes from crashing_leaf_func to __libc_start_main, then runs out of frames. I'm still investigating why it does this. I don't think that should be a blocker for landing this though since this approach seems more correct than the raise, so LGTM.

Ok I did more digging and it looks like the issue is that my host libc doesn't have an FDE for raise, but it does have one for kill (which I don't have an explanation for). I suppose that would mean D114743 isn't a bad alternative, but I guess this means it could also fail again for any user with a libc that doesn't happen to have unwind info for kill (could be a wait-and-see scenario). If we want to avoid the potential missing unwind info altogether, we could perhaps having some inline assembly that does a bad memory reference, but that might not be very portable. Thoughts?

No, that doesn't ring a bell. I think this is a question for whoever provides the C Standard Library on your aarch64 bot. Perhaps libunwind is somehow unable to understand the stack frames below raise before it gets to the kernel? Honestly I'm kind of shooting in the dark.

I'd be fine with switching to kill(getpid(), SIGSEGV) temporarily to unbreak your bot, but I'd like to avoid doing a blind fix and get the ball rolling on this issue with the C library folks. Do you think you can provide a patch to switch to kill?

The unwind_leaffunction.pass.cpp test was failing with ubsan because it was
relying on undefined behavior to "do the right thing" and result in a SIGSEGV.
IMO it makes more sense to simply raise a SIGSEGV directly, since that is guaranteed
to work.