Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BoundsSan] Update BoundsChecking.cpp to use no-merge attribute where applicable #120620

Merged
merged 1 commit into from
Dec 19, 2024

Conversation

thurstond
Copy link
Contributor

@thurstond thurstond commented Dec 19, 2024

#65972 introduced -ubsan-unique-traps and -bounds-checking-unique-traps, which attach the function size to the ubsantrap intrinsic.

#117651 changed ubsan-unique-traps to use nomerge instead of the function size, but did not update -bounds-checking-unique-traps. This patch adds nomerge to bounds-checking-unique-traps.

@llvmbot llvmbot added clang Clang issues not falling into any other category llvm:transforms labels Dec 19, 2024
@thurstond thurstond changed the title [BoundsSan] Update BoundsChecking.cpp to use no-merge attribute where… [BoundsSan] Update BoundsChecking.cpp to use no-merge attribute where applicable Dec 19, 2024
@llvmbot
Copy link
Member

llvmbot commented Dec 19, 2024

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-llvm-transforms

Author: Thurston Dang (thurstond)

Changes

… applicable

#65972 introduced -ubsan-unique-traps and -bounds-checking-unique-traps, which attach the function size to the ubsantrap intrinsic.

#117651 changed ubsan-unique-traps to use nomerge instead of the function size, but did not update -bounds-checking-unique-traps. This patch adds nomerge to bounds-checking-unique-traps.


Full diff: https://github.com/llvm/llvm-project/pull/120620.diff

2 Files Affected:

  • (modified) clang/test/CodeGen/bounds-checking.c (+3-2)
  • (modified) llvm/lib/Transforms/Instrumentation/BoundsChecking.cpp (+3-1)
diff --git a/clang/test/CodeGen/bounds-checking.c b/clang/test/CodeGen/bounds-checking.c
index f6c4880e70a150..2e0f6fb491e2c8 100644
--- a/clang/test/CodeGen/bounds-checking.c
+++ b/clang/test/CodeGen/bounds-checking.c
@@ -73,11 +73,11 @@ char B[10];
 char B2[10];
 // CHECK-LABEL: @f8
 void f8(int i, int k) {
-  // NOOPTLOCAL: call void @llvm.ubsantrap(i8 3)
+  // NOOPTLOCAL: call void @llvm.ubsantrap(i8 3) #[[ATTR1:[0-9]+]]
   // NOOPTARRAY: call void @llvm.ubsantrap(i8 18)
   B[i] = '\0';
 
-  // NOOPTLOCAL: call void @llvm.ubsantrap(i8 5)
+  // NOOPTLOCAL: call void @llvm.ubsantrap(i8 5) #[[ATTR1:[0-9]+]]
   // NOOPTARRAY: call void @llvm.ubsantrap(i8 18)
   B2[k] = '\0';
 }
@@ -90,3 +90,4 @@ struct S {
 struct S *f9(int i) {
   return &s[i];
 }
+// NOOPTLOCAL: attributes #[[ATTR1]] = { nomerge noreturn nounwind }
diff --git a/llvm/lib/Transforms/Instrumentation/BoundsChecking.cpp b/llvm/lib/Transforms/Instrumentation/BoundsChecking.cpp
index c86d967716a5a0..1a2dc5984523ec 100644
--- a/llvm/lib/Transforms/Instrumentation/BoundsChecking.cpp
+++ b/llvm/lib/Transforms/Instrumentation/BoundsChecking.cpp
@@ -196,8 +196,10 @@ static bool addBoundsChecking(Function &F, TargetLibraryInfo &TLI,
 
     CallInst *TrapCall;
     if (DebugTrapBB) {
+      // Ideally we would use the SanitizerHandler::OutOfBounds constant
       TrapCall = IRB.CreateIntrinsic(
           IntrID, {}, ConstantInt::get(IRB.getInt8Ty(), Fn->size()));
+      TrapCall->addFnAttr(llvm::Attribute::NoMerge);
     } else {
       TrapCall = IRB.CreateIntrinsic(IntrID, {}, {});
     }
@@ -251,4 +253,4 @@ void BoundsCheckingPass::printPipeline(
     OS << "<rt-abort>";
     break;
   }
-}
\ No newline at end of file
+}

… applicable

llvm#65972 introduced
-ubsan-unique-traps and -bounds-checking-unique-traps, which attach the function size to the ubsantrap intrinsic.

llvm#117651 changed
ubsan-unique-traps to use nomerge instead of the function size, but did
not update -bounds-checking-unique-traps. This patch adds nomerge to
bounds-checking-unique-traps.
@thurstond thurstond merged commit d33a2c5 into llvm:main Dec 19, 2024
5 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang Clang issues not falling into any other category llvm:transforms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants