Make sanitizer special case list slash-agnostic

[DKLoehr]

This changes the glob matcher for the sanitizer special case format so that it treats / as matching both forward and back slashes.

When dealing with cross-compiles or build systems that don't normalize slashes, it's possible to run into file paths with inconsistent slashiness, e.g. ../..\v8/include\v8-internal.h when building chromium.

We can match this using the current syntax using this ugly kludge: src:*{/,\\}v8{/,\\}*. However, since the format is explicitly for listing file paths, it makes sense to treat / as denoting a path separator rather than a literal forward slash. This allows us to write the much more natural form src:*/v8/* and have it work on any platform.

This is technically a behavior change, but it seems very unlikely to come up in practice. It will only make a difference if a user has a system where they want to catch a/b but not a\b. Even in the worst case, they can still regain the previous behavior by escaping the slash character in their pattern: src:a\/b.

[llvmbot]

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-llvm-support

Author: Devon Loehr (DKLoehr)

Changes

This changes the glob matcher for the sanitizer special case format so that it treats / as matching both forward and back slashes.

When dealing with cross-compiles or build systems that don't normalize slashes, it's possible to run into file paths with inconsistent slashiness, e.g. ../..\v8/include\v8-internal.h when building chromium.

We can match this using the current syntax using this ugly kludge: src:*{/,\\}v8{/,\\}*. However, since the format is explicitly for listing file paths, it makes sense to treat / as denoting a path separator rather than a literal forward slash. This allows us to write the much more natural form src:*/v8/* and have it work on any platform.

This is technically a behavior change, but it seems very unlikely to come up in practice. It will only make a difference if a user has a system where both a/b and a\b exist, are different files, and they only want to capture the first of them. Even in the worst case, they can still regain the previous behavior by escaping the slash character in their pattern: src:a\/b.

---

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

5 Files Affected:

• (modified) clang/docs/SanitizerSpecialCaseList.rst (+1)
• (modified) clang/unittests/Basic/DiagnosticTest.cpp (+23)
• (modified) llvm/docs/ReleaseNotes.md (+4)
• (modified) llvm/include/llvm/Support/GlobPattern.h (+1)
• (modified) llvm/lib/Support/GlobPattern.cpp (+4)

diff --git a/clang/docs/SanitizerSpecialCaseList.rst b/clang/docs/SanitizerSpecialCaseList.rst
index 194f2fc5a7825..3aea40ce0715d 100644
--- a/clang/docs/SanitizerSpecialCaseList.rst
+++ b/clang/docs/SanitizerSpecialCaseList.rst
@@ -174,6 +174,7 @@ tool-specific docs.
     # Lines starting with # are ignored.
     # Turn off checks for the source file
     # Entries without sections are placed into [*] and apply to all sanitizers
+    # "/" matches both windows and unix path separators ("/" and "\")
     src:path/to/source/file.c
     src:*/source/file.c
     # Turn off checks for this main file, including files included by it.
diff --git a/clang/unittests/Basic/DiagnosticTest.cpp b/clang/unittests/Basic/DiagnosticTest.cpp
index b0a034e9af1cd..3ab3cc918f5dc 100644
--- a/clang/unittests/Basic/DiagnosticTest.cpp
+++ b/clang/unittests/Basic/DiagnosticTest.cpp
@@ -360,4 +360,27 @@ TEST_F(SuppressionMappingTest, ParsingRespectsOtherWarningOpts) {
   clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS);
   EXPECT_THAT(diags(), IsEmpty());
 }
+
+TEST_F(SuppressionMappingTest, ForwardSlashMatchesBothDirections) {
+  llvm::StringLiteral SuppressionMappingFile = R"(
+  [unused]
+  src:*clang/*
+  src:*clang/lib/Sema/*=emit
+  src:*clang/lib\\Sema/foo*)";
+  Diags.getDiagnosticOptions().DiagnosticSuppressionMappingsFile = "foo.txt";
+  FS->addFile("foo.txt", /*ModificationTime=*/{},
+              llvm::MemoryBuffer::getMemBuffer(SuppressionMappingFile));
+  clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS);
+  EXPECT_THAT(diags(), IsEmpty());
+
+  EXPECT_TRUE(Diags.isSuppressedViaMapping(
+      diag::warn_unused_function, locForFile(R"(clang/lib/Basic/foo.h)")));
+  EXPECT_FALSE(Diags.isSuppressedViaMapping(
+      diag::warn_unused_function, locForFile(R"(clang/lib/Sema\bar.h)")));
+  EXPECT_TRUE(Diags.isSuppressedViaMapping(
+      diag::warn_unused_function, locForFile(R"(clang\lib\Sema/foo.h)")));
+  // The third pattern requires a literal backslash before Sema
+  EXPECT_FALSE(Diags.isSuppressedViaMapping(
+      diag::warn_unused_function, locForFile(R"(clang/lib/Sema/foo.h)")));
+}
 } // namespace
diff --git a/llvm/docs/ReleaseNotes.md b/llvm/docs/ReleaseNotes.md
index bb1f88e8480f1..8a883b7329343 100644
--- a/llvm/docs/ReleaseNotes.md
+++ b/llvm/docs/ReleaseNotes.md
@@ -338,6 +338,10 @@ Changes to BOLT
 Changes to Sanitizers
 ---------------------
 
+* The [sanitizer special case list format](https://clang.llvm.org/docs/SanitizerSpecialCaseList.html#format)
+  now treats forward slashes as either a forward or a backslash, to handle
+  paths with mixed unix and window styles.
+
 Other Changes
 -------------
 
diff --git a/llvm/include/llvm/Support/GlobPattern.h b/llvm/include/llvm/Support/GlobPattern.h
index 62ed4a0f23fd9..af92c63331282 100644
--- a/llvm/include/llvm/Support/GlobPattern.h
+++ b/llvm/include/llvm/Support/GlobPattern.h
@@ -35,6 +35,7 @@ namespace llvm {
 ///   expansions are not supported. If \p MaxSubPatterns is empty then
 ///   brace expansions are not supported and characters `{,}` are treated as
 ///   literals.
+/// * `/` matches both unix and windows path separators: `/` and `\`.
 /// * `\` escapes the next character so it is treated as a literal.
 ///
 /// Some known edge cases are:
diff --git a/llvm/lib/Support/GlobPattern.cpp b/llvm/lib/Support/GlobPattern.cpp
index 7004adf461a0c..26b3724863ee8 100644
--- a/llvm/lib/Support/GlobPattern.cpp
+++ b/llvm/lib/Support/GlobPattern.cpp
@@ -231,6 +231,10 @@ bool GlobPattern::SubGlobPattern::match(StringRef Str) const {
         ++S;
         continue;
       }
+    } else if (*P == '/' && (*S == '/' || *S == '\\')) {
+      ++P;
+      ++S;
+      continue;
     } else if (*P == *S || *P == '?') {
       ++P;
       ++S;