[NFC] Pre-commit UBSAN src:*=sanitize test #140602
Merged
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
Created using spr 1.3.6
|
@llvm/pr-subscribers-clang Author: Qinkun Bao (qinkunbao) ChangesFull diff: https://github.com/llvm/llvm-project/pull/140602.diff 1 Files Affected:
diff --git a/clang/test/CodeGen/ubsan-src-ignorelist-category.test b/clang/test/CodeGen/ubsan-src-ignorelist-category.test
new file mode 100644
index 0000000000000..e0efd65df8652
--- /dev/null
+++ b/clang/test/CodeGen/ubsan-src-ignorelist-category.test
@@ -0,0 +1,37 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsanitize=signed-integer-overflow -fsanitize-ignorelist=%t/src.ignorelist -emit-llvm %t/test1.c -o - | FileCheck %s -check-prefix=CHECK-ALLOWLIST
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsanitize=signed-integer-overflow -fsanitize-ignorelist=%t/src.ignorelist -emit-llvm %t/test2.c -o - | FileCheck %s -check-prefix=CHECK-IGNORELIST
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsanitize=signed-integer-overflow -fsanitize-ignorelist=%t/src.ignorelist.contradict1 -emit-llvm %t/test1.c -o - | FileCheck %s -check-prefix=CHECK-ALLOWLISTOVERIDE1
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsanitize=signed-integer-overflow -fsanitize-ignorelist=%t/src.ignorelist.contradict2 -emit-llvm %t/test1.c -o - | FileCheck %s -check-prefix=CHECK-ALLOWLISTOVERIDE2
+
+
+// Verify ubsan only emits checks for files in the allowlist
+
+//--- src.ignorelist
+src:*
+src:*/test1.c=sanitize
+
+//--- src.ignorelist.contradict1
+src:*
+src:*/test1.c=sanitize
+src:*/test1.c
+
+//--- src.ignorelist.contradict2
+src:*
+src:*/test1.c
+src:*/test1.c=sanitize
+
+//--- test1.c
+int add1(int a, int b) {
+// CHECK-ALLOWLIST: llvm.sadd.with.overflow.i32
+// CHECK-ALLOWLISTOVERIDE1-NOT: llvm.sadd.with.overflow.i32
+// CHECK-ALLOWLISTOVERIDE2: llvm.sadd.with.overflow.i32
+ return a+b;
+}
+
+//--- test2.c
+int add2(int a, int b) {
+// CHECK-IGNORELIST-NOT: llvm.sadd.with.overflow.i32
+ return a+b;
+}
|
|
This patch should be "[NFC] Precommit test for src:*=sanitize for UBSan" (or similar), have the test output "passing" for the current build of clang (i.e., probably "incorrect" output), reviewed and then merged. |
Thank you for your feedback. However, I recommend against merging this test. The linked pull request (#140529) introduces a new feature rather than correcting a bug. I believe it's inappropriate to add a test covering a new feature and then submit a separate pull request to "fix" it. |
I would argue that since the If it had been a new syntax that no user would expect to work ( |
vitalybuka
approved these changes
May 19, 2025
|
|
||
| //--- test2.c | ||
| int add2(int a, int b) { | ||
| // CHECK-IGNORELIST-NOT: llvm.sadd.with.overflow.i32 |
There was a problem hiding this comment.
Please use --implicit-check-not= when possible instead of not -NOT
-NOT is order dependent, it's easy to break code, but -NOT will be satisfied.
In this case you are looking at exact expression in the code, so we rather check precisely instead of NOT
| @@ -0,0 +1,37 @@ | ||
| // RUN: rm -rf %t | ||
| // RUN: split-file %s %t | ||
| // RUN: %clang_cc1 -triple x86_64-linux-gnu -fsanitize=signed-integer-overflow -fsanitize-ignorelist=%t/src.ignorelist -emit-llvm %t/test1.c -o - | FileCheck %s -check-prefix=CHECK-ALLOWLIST |
There was a problem hiding this comment.
It's not going to hurt to test all 2 files for 3 cases
vitalybuka
reviewed
May 19, 2025
| @@ -0,0 +1,43 @@ | ||
| // RUN: rm -rf %t | ||
| // RUN: split-file %s %t | ||
| // RUN: %clang_cc1 -triple x86_64-linux-gnu -fsanitize=signed-integer-overflow -fsanitize-ignorelist=%t/src.ignorelist -emit-llvm %t/test1.c -o - | FileCheck %s --check-prefixes=CHECK1,IGNORE |
There was a problem hiding this comment.
I assume when you land a fix, you just need to update --check-prefixes=
and remove FIXME
vitalybuka
reviewed
May 19, 2025
| } | ||
|
|
||
| //--- test2.c | ||
| // CHECK2-LABEL: define dso_local i32 @sub |
There was a problem hiding this comment.
sub just make them more distiguishable
Yep, feature or bug, but the current behavior is not desirable. |
thurstond
approved these changes
May 19, 2025
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/52/builds/8460 Here is the relevant piece of the build log for the reference |
sivan-shani
pushed a commit
to sivan-shani/llvm-project
that referenced
this pull request
Jun 3, 2025
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.
For #140529