- User Since
- Apr 25 2018, 1:47 PM (218 w, 1 d)
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
Wed, Jun 29
Tue, Jun 28
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
Fri, Jun 24
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.
Wed, Jun 22
Wed, Jun 8
Tue, Jun 7
Mon, Jun 6
Thu, Jun 2
Wed, Jun 1
May 24 2022
*ping* any more comments?
May 23 2022
May 6 2022
Apr 21 2022
Apr 14 2022
Mar 30 2022
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):
Feb 11 2022
Feb 8 2022
Feb 7 2022
Jan 20 2022
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):
Jan 18 2022
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.
Dec 3 2021
Dec 2 2021
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.
Dec 1 2021
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:
Interesting it works on Ubuntu20.04/Aarch64, I compile the code with ToT clang or gcc-10.
could you share an objdump of main?
Nov 30 2021
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.
Nov 29 2021
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