Author: Vitaly Buka Date: 2026-06-27T23:32:02+01:00 New Revision: 91cfa5786716e4908af9a36104ac425728d5aa83
URL: https://github.com/llvm/llvm-project/commit/91cfa5786716e4908af9a36104ac425728d5aa83 DIFF: https://github.com/llvm/llvm-project/commit/91cfa5786716e4908af9a36104ac425728d5aa83.diff LOG: Reland "Make sanitizer special case list slash-agnostic" (#206250) 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](https://g-issues.chromium.org/issues/425364464). 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`. Note: this is not default behavior yet. To activate use `#!special-case-list-v4` in the first line of the special case list file. Reverts llvm/llvm-project#205399 Relands https://github.com/llvm/llvm-project/pull/149886 Reason for the revert was incorrect https://github.com/llvm/llvm-project/pull/202854#issuecomment-4813462549 Authored-by: Devon Loehr <[email protected]> Co-authored-by: Vitaly Buka <[email protected]> Added: Modified: clang/docs/ReleaseNotes.rst clang/docs/SanitizerSpecialCaseList.rst clang/unittests/Basic/DiagnosticTest.cpp llvm/lib/Support/SpecialCaseList.cpp llvm/unittests/Support/SpecialCaseListTest.cpp Removed: ################################################################################ diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 93f770c10afae..d6b978ec91659 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -1103,6 +1103,11 @@ Sanitizers warning for deprecated matches. Version 5 drops backward compatibility and requires rules to match canonicalized paths (without leading ``./``). +- Sanitizer Special Case Lists (``-fsanitize-ignorelist``) and warning + suppression mappings (``--warning-suppression-mappings``) now recognize version + 4 of the Special Case List format (indicated by ``#!special-case-list-v4``). + On Windows hosts, path matching is slash-agnostic (both forward slashes (``/``) + and backslashes (``\``) match either path separator in both patterns and paths). Python Binding Changes ---------------------- diff --git a/clang/docs/SanitizerSpecialCaseList.rst b/clang/docs/SanitizerSpecialCaseList.rst index 1de3555c5a8ce..918abf19f8f10 100644 --- a/clang/docs/SanitizerSpecialCaseList.rst +++ b/clang/docs/SanitizerSpecialCaseList.rst @@ -230,6 +230,18 @@ tool-specific docs. [{cfi-vcall,cfi-icall}] fun:*BadCfiCall + +.. note:: + + By default, ``src`` and ``mainfile`` are matched against the filename as seen + by LLVM. On Windows, this might involve a mix of forward and backslashes as + file separators, and writing patterns to match both variants can be + inconvenient. + + Starting with version 4 (indicated by ``#!special-case-list-v4``), path matching + on Windows hosts is slash-agnostic: both forward slashes (``/``) and backslashes + (``\``) match either path separator in both patterns and paths. + .. note:: By default, path matching (for ``src`` and ``mainfile``) matches the query diff --git a/clang/unittests/Basic/DiagnosticTest.cpp b/clang/unittests/Basic/DiagnosticTest.cpp index 4d310d3ece23f..4ced52c8f715f 100644 --- a/clang/unittests/Basic/DiagnosticTest.cpp +++ b/clang/unittests/Basic/DiagnosticTest.cpp @@ -414,4 +414,39 @@ TEST_F(SuppressionMappingTest, ParsingRespectsOtherWarningOpts) { clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS); EXPECT_THAT(diags(), IsEmpty()); } + +#ifdef _WIN32 +TEST_F(SuppressionMappingTest, CanonicalizesSlashesOnWindows) { + llvm::StringLiteral SuppressionMappingFile = R"(#!special-case-list-v4 + [unused] + src:*clang/* + src:*clang/lib/Sema/*=emit + src:*clang/lib\\Sema/foo* + fun:suppress/me)"; + 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/bar.h)"))); + EXPECT_TRUE(Diags.isSuppressedViaMapping( + diag::warn_unused_function, locForFile(R"(clang/lib/Basic\bar.h)"))); + EXPECT_TRUE(Diags.isSuppressedViaMapping( + diag::warn_unused_function, locForFile(R"(clang\lib/Basic/bar.h)"))); + EXPECT_FALSE(Diags.isSuppressedViaMapping( + diag::warn_unused_function, locForFile(R"(clang/lib/Sema/baz.h)"))); + EXPECT_FALSE(Diags.isSuppressedViaMapping( + diag::warn_unused_function, locForFile(R"(clang/lib/Sema\baz.h)"))); + + // Under slash-agnostic matching, backslashes and forward slashes match each + // other, so we match the third pattern. + EXPECT_TRUE(Diags.isSuppressedViaMapping( + diag::warn_unused_function, locForFile(R"(clang\lib\Sema/foo.h)"))); + EXPECT_TRUE(Diags.isSuppressedViaMapping( + diag::warn_unused_function, locForFile(R"(clang/lib/Sema/foo.h)"))); +} +#endif + } // namespace diff --git a/llvm/lib/Support/SpecialCaseList.cpp b/llvm/lib/Support/SpecialCaseList.cpp index d72f7e7fd1d81..fe12039719059 100644 --- a/llvm/lib/Support/SpecialCaseList.cpp +++ b/llvm/lib/Support/SpecialCaseList.cpp @@ -24,6 +24,7 @@ #include "llvm/Support/GlobPattern.h" #include "llvm/Support/LineIterator.h" #include "llvm/Support/MemoryBuffer.h" +#include "llvm/Support/Path.h" #include "llvm/Support/Regex.h" #include "llvm/Support/VirtualFileSystem.h" #include "llvm/Support/WithColor.h" @@ -44,7 +45,7 @@ namespace { // Lagacy v1 matcher. class RegexMatcher { public: - Error insert(StringRef Pattern, unsigned LineNumber); + Error insert(StringRef Pattern, unsigned LineNumber, bool SlashAgnostic); unsigned match(StringRef Query) const; StringRef findRule(unsigned LineNo) const; @@ -62,7 +63,7 @@ class RegexMatcher { class GlobMatcher { public: - Error insert(StringRef Pattern, unsigned LineNumber); + Error insert(StringRef Pattern, unsigned LineNumber, bool SlashAgnostic); unsigned match(StringRef Query) const; StringRef findRule(unsigned LineNo) const; @@ -95,6 +96,7 @@ struct QueryOptions { bool UseGlobs = true; bool RemoveDotSlash = false; bool WarnDotSlashMatch = false; + bool SlashAgnostic = false; }; /// Represents a set of patterns and their line numbers @@ -116,7 +118,8 @@ class Matcher { mutable std::once_flag Warned; }; -Error RegexMatcher::insert(StringRef Pattern, unsigned LineNumber) { +Error RegexMatcher::insert(StringRef Pattern, unsigned LineNumber, + bool SlashAgnostic) { if (Pattern.empty()) return createStringError(errc::invalid_argument, "Supplied regex was blank"); @@ -155,11 +158,13 @@ StringRef RegexMatcher::findRule(unsigned LineNo) const { return {}; } -Error GlobMatcher::insert(StringRef Pattern, unsigned LineNumber) { +Error GlobMatcher::insert(StringRef Pattern, unsigned LineNumber, + bool SlashAgnostic) { if (Pattern.empty()) return createStringError(errc::invalid_argument, "Supplied glob was blank"); - auto Res = GlobPattern::create(Pattern, /*MaxSubPatterns=*/1024); + auto Res = + GlobPattern::create(Pattern, /*MaxSubPatterns=*/1024, SlashAgnostic); if (auto Err = Res.takeError()) return Err; Globs.emplace_back(Pattern, LineNumber, std::move(Res.get())); @@ -257,7 +262,11 @@ Matcher::Matcher(QueryOptions QOpts) : Options(QOpts) { } Error Matcher::insert(StringRef Pattern, unsigned LineNumber) { - return std::visit([&](auto &V) { return V.insert(Pattern, LineNumber); }, M); + return std::visit( + [&](auto &V) { + return V.insert(Pattern, LineNumber, Options.SlashAgnostic); + }, + M); } /// Matches Query against the patterns. The behavior is controlled by @@ -406,6 +415,15 @@ bool SpecialCaseList::parse(unsigned FileIdx, const MemoryBuffer *MB, bool UseGlobs = MinVersion(2); bool RemoveDotSlash = MinVersion(3); bool WarnDotSlash = MinVersion(4) && !MinVersion(5); + // TODO: Improve efficiency on Windows. + // `SlashAgnostic` makes `GlobMatcher` lookup inefficient by reducing the part + // of the pattern handled by the RadixTree. This was already the case even + // before `SlashAgnostic` because `GlobMatcher` pessimizes on escape sequences + // needed to represent Windows backslashes. A possible, but not unique, + // solution is to assume (or convert Windows query) backslashes, and + // preprocess the Glob pattern to use diff erent escape sequences. + bool SlashAgnostic = MinVersion(4) && llvm::sys::path::is_style_windows( + llvm::sys::path::Style::native); auto ErrOrSection = addSection("*", FileIdx, 1, true); if (auto Err = ErrOrSection.takeError()) { @@ -457,6 +475,7 @@ bool SpecialCaseList::parse(unsigned FileIdx, const MemoryBuffer *MB, if (llvm::is_contained(PathPrefixes, Prefix)) { QOpts.RemoveDotSlash = RemoveDotSlash; QOpts.WarnDotSlashMatch = WarnDotSlash; + QOpts.SlashAgnostic = SlashAgnostic; } auto [Pattern, Category] = Postfix.split("="); diff --git a/llvm/unittests/Support/SpecialCaseListTest.cpp b/llvm/unittests/Support/SpecialCaseListTest.cpp index 5bcd111f53059..d4d47c37993f2 100644 --- a/llvm/unittests/Support/SpecialCaseListTest.cpp +++ b/llvm/unittests/Support/SpecialCaseListTest.cpp @@ -464,4 +464,24 @@ TEST_F(SpecialCaseListTest, FileIdx) { sys::fs::remove(Path); } +#ifdef _WIN32 +TEST_F(SpecialCaseListTest, SlashAgnosticPathsOnWindows) { + std::unique_ptr<SpecialCaseList> SCL = + makeSpecialCaseList("#!special-case-list-v4\n" + "\n" + "src:*foo/bar*\n" + "src:*foo\\\\baz\n" + "fun:hi\\\\bye=category\n"); + EXPECT_TRUE(SCL->inSection("", "src", "foo/bar")); + EXPECT_TRUE(SCL->inSection("", "src", "foo\\bar")); + // The baz pattern matches because paths are matched slash-agnostically + EXPECT_TRUE(SCL->inSection("", "src", "foo/baz")); + EXPECT_TRUE(SCL->inSection("", "src", "foo\\baz")); + // Slash-agnostic matching only applies to files + EXPECT_TRUE(SCL->inSection("", "fun", "hi\\bye", "category")); + EXPECT_FALSE(SCL->inSection("", "fun", "hi/bye", "category")); +} + +#endif + } // namespace _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
