🍒 Cherry-pick "Remove -funique-traps and replace it with a better implementation (rdar://158088410)" #11548
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The implementation was a bit hacky and interacted badly with the hot-cold splitting pass. So it is going to be removed in favor of a different implementation that's simpler. I also investated why `clang/test/BoundsSafety/CodeGen/unique-trap-blocks-O2.c` broke (rdar://150559639). It turns outthat the "trap blocks" started being unintentionally split by the hot-cold splitting pass because in upstream we started adding branch profiling weights (llvm#134310) which the hot-cold splitting pass used as a signal to split the "trap blocks" into their own function (by-passing the logic I originally added to prevent the splitting). While we could fix this it would be very fragile and given that we are removing the implementation anyway it doesn't matter anymore. rdar://158088410 (cherry picked from commit 8c4e017) Conflicts: clang/test/BoundsSafety-legacy-checks/CodeGen/unique-trap-blocks-O0-O2-opt-disabled.c clang/test/BoundsSafety/CodeGen/unique-trap-blocks-O0-O2-opt-disabled.c
This patch adds a driver/cc1 flag that prevents `-fbounds-safety` traps from being merged. This improves debuggability (because now it is possible to see which check lead to the trap) at the cost of increased code size. It is currently off by default. This implementation adds the `nomerge` attribute on call instructions to `-fbounds-safety` traps at the LLVM IR level and prevents re-using trap blocks during Clang's IRGen when the compiler flag is passed. This was basically already implemented upstream for trapping UBSan which meant the only thing we needed to do was set the `NoMerge` parameter to `true` when calling `EmitTrapCheck`. This is the replacement for the recently removed `-funique-traps` which was less than ideal for several reasons: * The use of `asm` instructions in the trap blocks made analyzing LLVM IR more challenging than it needed to be. * The trap blocks interacted poorly with the hot-cold splitting pass and required fragile workarounds to prevent the trap blocks from being split into their own function. The implementation of `-fbound-safety-unique-traps` has none of these problems. rdar://158088410 (cherry picked from commit 9b1d55c)
delcypher
added
the
clang:bounds-safety
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Cherry-picks #11455