llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-llvm-support Author: Devon Loehr (DKLoehr) <details> <summary>Changes</summary> 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 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 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; `````````` </details> https://github.com/llvm/llvm-project/pull/149886 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
