[UBSan] Implement src:*=sanitize for UBSan#140529
Merged
vitalybuka merged 30 commits intoMay 28, 2025
Merged
Conversation
Created using spr 1.3.6
Member
|
@llvm/pr-subscribers-clang Author: Qinkun Bao (qinkunbao) ChangesFull diff: https://github.com/llvm/llvm-project/pull/140529.diff 4 Files Affected:
diff --git a/clang/include/clang/Basic/SanitizerSpecialCaseList.h b/clang/include/clang/Basic/SanitizerSpecialCaseList.h
index d024b7dfc2e85..25d518e7128cf 100644
--- a/clang/include/clang/Basic/SanitizerSpecialCaseList.h
+++ b/clang/include/clang/Basic/SanitizerSpecialCaseList.h
@@ -43,13 +43,18 @@ class SanitizerSpecialCaseList : public llvm::SpecialCaseList {
bool inSection(SanitizerMask Mask, StringRef Prefix, StringRef Query,
StringRef Category = StringRef()) const;
+ // Query ignorelisted entries if any bit in Mask matches the entry's section.
+ // Return 0 if not found. If found, return the line number (starts with 1).
+ unsigned inSectionBlame(SanitizerMask Mask, StringRef Prefix, StringRef Query,
+ StringRef Category = StringRef()) const;
+
protected:
// Initialize SanitizerSections.
void createSanitizerSections();
struct SanitizerSection {
SanitizerSection(SanitizerMask SM, SectionEntries &E)
- : Mask(SM), Entries(E){};
+ : Mask(SM), Entries(E) {};
SanitizerMask Mask;
SectionEntries &Entries;
diff --git a/clang/lib/Basic/NoSanitizeList.cpp b/clang/lib/Basic/NoSanitizeList.cpp
index e7e63c1f419e6..811480f914ec5 100644
--- a/clang/lib/Basic/NoSanitizeList.cpp
+++ b/clang/lib/Basic/NoSanitizeList.cpp
@@ -44,6 +44,13 @@ bool NoSanitizeList::containsFunction(SanitizerMask Mask,
bool NoSanitizeList::containsFile(SanitizerMask Mask, StringRef FileName,
StringRef Category) const {
+ unsigned nosanline = SSCL->inSectionBlame(Mask, "src", FileName, Category);
+ unsigned sanline = SSCL->inSectionBlame(Mask, "src", FileName, "sanitize");
+ // If we have two cases such as `src:a.cpp=sanitize` and `src:a.cpp`, the
+ // current entry override the previous entry.
+ if (nosanline > 0 && sanline > 0) {
+ return nosanline > sanline;
+ }
return SSCL->inSection(Mask, "src", FileName, Category);
}
diff --git a/clang/lib/Basic/SanitizerSpecialCaseList.cpp b/clang/lib/Basic/SanitizerSpecialCaseList.cpp
index 2dbf04c6ede97..7da36f3801453 100644
--- a/clang/lib/Basic/SanitizerSpecialCaseList.cpp
+++ b/clang/lib/Basic/SanitizerSpecialCaseList.cpp
@@ -63,3 +63,19 @@ bool SanitizerSpecialCaseList::inSection(SanitizerMask Mask, StringRef Prefix,
return false;
}
+
+unsigned SanitizerSpecialCaseList::inSectionBlame(SanitizerMask Mask,
+ StringRef Prefix,
+ StringRef Query,
+ StringRef Category) const {
+ for (auto &S : SanitizerSections) {
+ if (S.Mask & Mask) {
+ unsigned lineNum =
+ SpecialCaseList::inSectionBlame(S.Entries, Prefix, Query, Category);
+ if (lineNum > 0) {
+ return lineNum;
+ }
+ }
+ }
+ return 0;
+}
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;
+}
|
Created using spr 1.3.6
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
Created using spr 1.3.6
thurstond
reviewed
May 19, 2025
Created using spr 1.3.6
Created using spr 1.3.6 [skip ci]
Created using spr 1.3.6 [skip ci]
vitalybuka
reviewed
May 19, 2025
vitalybuka
pushed a commit
that referenced
this pull request
May 19, 2025
Base automatically changed from
users/qinkunbao/spr/demo-test-for-httpsgithubcomllvmllvm-projectpull140529
to
main
May 19, 2025 22:22
vitalybuka
reviewed
May 19, 2025
vitalybuka
reviewed
May 19, 2025
| // RUN: %clang_cc1 -triple x86_64-linux-gnu -fsanitize=signed-integer-overflow -fsanitize-ignorelist=%t/src.ignorelist.contradict1 -emit-llvm %t/test2.c -o - | FileCheck %s --check-prefixes=CHECK2 | ||
|
|
||
| // 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-prefixes=CHECK1,IGNORE | ||
| // 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-prefixes=CHECK1,SANITIZE |
Contributor
There was a problem hiding this comment.
Please in separate PR extend this test for:
- category - seems like your fix has no issues with that, but it would be nice to have test coverage
- multiple files (or maybe llvm-project/llvm/unittests/Support/SpecialCaseListTest.cpp will be enough)
Contributor
|
llvm-project/clang/docs/SanitizerSpecialCaseList.rst needs to be updated And nice to mention in llvm-project/clang/docs/ReleaseNotes.rst |
vitalybuka
added a commit
that referenced
this pull request
May 19, 2025
vitalybuka
added a commit
that referenced
this pull request
May 19, 2025
Created using spr 1.3.6
Created using spr 1.3.6
vitalybuka
reviewed
May 28, 2025
vitalybuka
reviewed
May 28, 2025
vitalybuka
approved these changes
May 28, 2025
llvm-sync Bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
May 28, 2025
…ir<uint, uint> (FileIdx, LineNo). (#141540) Accoring to the discussion in llvm/llvm-project#140529, we need to SSCL can be created from multiple ignore list files, so we can repeat-fsanitize-ignorelist=. The change is necessary to achieve the feature described in llvm/llvm-project#139128.
qinkunbao
added a commit
that referenced
this pull request
May 28, 2025
See: #139128 and #140529 for the background. The introduction of these new tests (ubsan-src-ignorelist-category.test) `-fsanitize-ignorelist=%t/src.ignorelist -fsanitize-ignorelist=%t/src.ignorelist.contradict9` in this PR will not lead to failures in the previous implementation (without this PR). This is because the existing logic distinguishes between Sections in different ignorelists, even if their names are identical. The order of these Sections is preserved using a `vector`.
llvm-sync Bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
May 28, 2025
…#141640) See: llvm/llvm-project#139128 and llvm/llvm-project#140529 for the background. The introduction of these new tests (ubsan-src-ignorelist-category.test) `-fsanitize-ignorelist=%t/src.ignorelist -fsanitize-ignorelist=%t/src.ignorelist.contradict9` in this PR will not lead to failures in the previous implementation (without this PR). This is because the existing logic distinguishes between Sections in different ignorelists, even if their names are identical. The order of these Sections is preserved using a `vector`.
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
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.
Background: #139128
It is a draft implementation for "src:*=sanitize". It should be applied to all sanitizers.
Any srcs assigned to the sanitize category will have their sanitizer instrumentation remained ignored by "src:". For example,
test1.ccwill still have the UBSan instrumented.Conflicting entries are resolved by the latest entry, which takes precedence.
test.ccdoes not have the UBSan check (In this case,src:*/mylib/test.ccoverridessrc:*/mylib/*=sanitizefortest.cc).test1.cchas the UBSan instrumented (In this case,src:*/mylib/*=sanitizeoverridessrc:*/mylib/test.cc).Documents update will be in a new PR.