Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

[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

Merged
merged 30 commits into from
May 28, 2025

Conversation

qinkunbao
Copy link
Member

@qinkunbao qinkunbao commented May 19, 2025

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,

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.

Created using spr 1.3.6
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels May 19, 2025
@llvmbot
Copy link
Member

llvmbot commented May 19, 2025

@llvm/pr-subscribers-clang

Author: Qinkun Bao (qinkunbao)

Changes

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

4 Files Affected:

  • (modified) clang/include/clang/Basic/SanitizerSpecialCaseList.h (+6-1)
  • (modified) clang/lib/Basic/NoSanitizeList.cpp (+7)
  • (modified) clang/lib/Basic/SanitizerSpecialCaseList.cpp (+16)
  • (added) clang/test/CodeGen/ubsan-src-ignorelist-category.test (+37)
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
Copy link

github-actions bot commented May 19, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

Created using spr 1.3.6
@qinkunbao qinkunbao requested a review from vitalybuka May 19, 2025 15:15
@qinkunbao qinkunbao changed the title Implement src:*=sanitize for UBSan [UBSan] Implement src:*=sanitize for UBSan May 19, 2025
@qinkunbao qinkunbao requested a review from thurstond May 19, 2025 18:43
clang/lib/Basic/NoSanitizeList.cpp Outdated Show resolved Hide resolved
clang/test/CodeGen/ubsan-src-ignorelist-category.test Outdated Show resolved Hide resolved
Created using spr 1.3.6
qinkunbao added 2 commits May 19, 2025 14:55
Created using spr 1.3.6

[skip ci]
Created using spr 1.3.6
@vitalybuka vitalybuka changed the base branch from main to users/qinkunbao/spr/demo-test-for-httpsgithubcomllvmllvm-projectpull140529 May 19, 2025 21:55
@vitalybuka vitalybuka changed the base branch from users/qinkunbao/spr/demo-test-for-httpsgithubcomllvmllvm-projectpull140529 to main May 19, 2025 21:59
qinkunbao added 2 commits May 19, 2025 15:13
Created using spr 1.3.6

[skip ci]
Created using spr 1.3.6
@vitalybuka vitalybuka changed the base branch from main to users/qinkunbao/spr/demo-test-for-httpsgithubcomllvmllvm-projectpull140529 May 19, 2025 22:14
clang/lib/Basic/NoSanitizeList.cpp Show resolved Hide resolved
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
Created using spr 1.3.6
clang/lib/Basic/NoSanitizeList.cpp Show resolved Hide resolved
// 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
Copy link
Collaborator

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:

  1. category - seems like your fix has no issues with that, but it would be nice to have test coverage
  2. multiple files (or maybe llvm-project/llvm/unittests/Support/SpecialCaseListTest.cpp will be enough)

@vitalybuka
Copy link
Collaborator

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
@qinkunbao qinkunbao requested a review from vitalybuka May 24, 2025 02:23
clang/lib/Basic/SanitizerSpecialCaseList.cpp Outdated Show resolved Hide resolved
clang/lib/Basic/SanitizerSpecialCaseList.cpp Outdated Show resolved Hide resolved
llvm/unittests/Support/SpecialCaseListTest.cpp Outdated Show resolved Hide resolved
qinkunbao added 2 commits May 27, 2025 18:51
Created using spr 1.3.6
Created using spr 1.3.6
@vitalybuka vitalybuka merged commit 4f1291e into main May 28, 2025
7 of 11 checks passed
@vitalybuka vitalybuka deleted the users/qinkunbao/spr/implement-srcsanitize-for-ubsan branch May 28, 2025 02:19
qinkunbao added a commit that referenced this pull request May 28, 2025
…nt> (FileIdx, LineNo). (#141540)

Accoring to the discussion in #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 #139128.
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`.
sivan-shani pushed a commit to sivan-shani/llvm-project that referenced this pull request Jun 3, 2025
sivan-shani pushed a commit to sivan-shani/llvm-project that referenced this pull request Jun 3, 2025
sivan-shani pushed a commit to sivan-shani/llvm-project that referenced this pull request Jun 3, 2025
sivan-shani pushed a commit to sivan-shani/llvm-project that referenced this pull request Jun 3, 2025
…lvm#140964)

As discussed in llvm#139772 and
llvm#140529, Matcher::Globs can
keep the order when parsing the case list.
sivan-shani pushed a commit to sivan-shani/llvm-project that referenced this pull request Jun 3, 2025
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.
sivan-shani pushed a commit to sivan-shani/llvm-project that referenced this pull request Jun 3, 2025
…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.
sivan-shani pushed a commit to sivan-shani/llvm-project that referenced this pull request Jun 3, 2025
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`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
Morty Proxy This is a proxified and sanitized view of the page, visit original site.