-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[UBSan] Implement src:*=sanitize for UBSan #140529
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
[UBSan] Implement src:*=sanitize for UBSan #140529
Conversation
Created using spr 1.3.6
@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
Created using spr 1.3.6
Created using spr 1.3.6 [skip ci]
Created using spr 1.3.6 [skip ci]
// 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-prefixes=CHECK2 | ||
|
||
// 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-prefixes=CHECK1,IGNORE | ||
// 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)
llvm-project/clang/docs/SanitizerSpecialCaseList.rst needs to be updated And nice to mention in llvm-project/clang/docs/ReleaseNotes.rst |
Created using spr 1.3.6
Created using spr 1.3.6
Created using spr 1.3.6
…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.
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`.
…#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`.
…lvm#140964) As discussed in llvm#139772 and llvm#140529, Matcher::Globs can keep the order when parsing the case list.
Background: llvm#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, ``` src:* src:*/test1.cc=sanitize ``` `test1.cc` will still have the UBSan instrumented. Conflicting entries are resolved by the latest entry, which takes precedence. ``` src:* src:*/mylib/*=sanitize src:*/mylib/test.cc ``` `test.cc` does not have the UBSan check (In this case, `src:*/mylib/test.cc` overrides `src:*/mylib/*=sanitize` for `test.cc`). ``` src:* src:*/mylib/test.cc src:*/mylib/*=sanitize ``` `test1.cc` has the UBSan instrumented (In this case, `src:*/mylib/*=sanitize` overrides `src:*/mylib/test.cc`). Documents update will be in a new PR.
…nt> (FileIdx, LineNo). (llvm#141540) Accoring to the discussion in llvm#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#139128.
See: llvm#139128 and llvm#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`.
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.cc
will still have the UBSan instrumented.Conflicting entries are resolved by the latest entry, which takes precedence.
test.cc
does not have the UBSan check (In this case,src:*/mylib/test.cc
overridessrc:*/mylib/*=sanitize
fortest.cc
).test1.cc
has the UBSan instrumented (In this case,src:*/mylib/*=sanitize
overridessrc:*/mylib/test.cc
).Documents update will be in a new PR.