-
Notifications
You must be signed in to change notification settings - Fork 15.2k
Make sanitizer special case list slash-agnostic #149886
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
base: main
Are you sure you want to change the base?
Changes from all commits
c613019
a81b550
7bfc6ad
a5eaf58
47236f1
8535a11
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -122,7 +122,7 @@ class SpecialCaseList { | |
class Matcher { | ||
public: | ||
LLVM_ABI Error insert(StringRef Pattern, unsigned LineNumber, | ||
bool UseRegex); | ||
bool UseGlobs); | ||
// Returns the line number in the source file that this query matches to. | ||
// Returns zero if no match is found. | ||
LLVM_ABI unsigned match(StringRef Query) const; | ||
|
@@ -154,6 +154,7 @@ class SpecialCaseList { | |
}; | ||
|
||
std::vector<Section> Sections; | ||
bool CanonicalizePaths = false; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sannitizers support multiple files |
||
|
||
LLVM_ABI Expected<Section *> addSection(StringRef SectionStr, | ||
unsigned FileIdx, unsigned LineNo, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -153,12 +153,17 @@ bool SpecialCaseList::parse(unsigned FileIdx, const MemoryBuffer *MB, | |
return false; | ||
} | ||
|
||
// Scan the start of the file for special comments. These don't appear when | ||
// iterating below because comment lines are automatically skipped. | ||
StringRef Buffer = MB->getBuffer(); | ||
// In https://reviews.llvm.org/D154014 we added glob support and planned to | ||
// remove regex support in patterns. We temporarily support the original | ||
// behavior using regexes if "#!special-case-list-v1" is the first line of the | ||
// file. For more details, see | ||
// behavior using regexes if "#!special-case-list-v1" is the first line of | ||
// the file. For more details, see | ||
// https://discourse.llvm.org/t/use-glob-instead-of-regex-for-specialcaselists/71666 | ||
bool UseGlobs = !MB->getBuffer().starts_with("#!special-case-list-v1\n"); | ||
bool UseGlobs = !Buffer.consume_front("#!special-case-list-v1\n"); | ||
// Specifies that patterns should be matched against canonicalized filepaths. | ||
CanonicalizePaths = Buffer.consume_front("#!canonical-paths\n"); | ||
|
||
for (line_iterator LineIt(*MB, /*SkipBlanks=*/true, /*CommentMarker=*/'#'); | ||
!LineIt.is_at_eof(); LineIt++) { | ||
|
@@ -237,7 +242,12 @@ unsigned SpecialCaseList::inSectionBlame(const SectionEntries &Entries, | |
if (II == I->second.end()) | ||
return 0; | ||
|
||
return II->getValue().match(Query); | ||
if (CanonicalizePaths && (Prefix == "src" || Prefix == "mainfile")) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually we don't need SpecialCaseList::NormalizePath if we do src/mainfile it should be enough |
||
return II->getValue().match(llvm::sys::path::convert_to_slash( | ||
llvm::sys::path::remove_leading_dotslash(Query))); | ||
} else { | ||
return II->getValue().match(Query); | ||
} | ||
} | ||
|
||
} // namespace llvm |
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.
Let's just do '#!special-case-list-v2' for current behavior,
and we will garbage collect v1, v2 in future.
#!special-case-list-v1 is just for transition, maybe it's already time to remove it.
If we do opt-in with #!canonical-paths, then we just delaying transition for later and it does not make it easier.
Uh oh!
There was an error while loading. Please reload this page.
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.
And I sudgest to not add
#!special-case-list-v1
and#!canonical-paths
into this doc at all.Users should update files ASAP
Good place for this is Beaking Changes in clang release notes. Something :
"Ingnore/suppression lists will need to use forward slashes from now, including Windows". Temporarily you can return to previous behavior with "#!special-case-list-v2"
And we should list in relnotest all flags which use special case list, and affected by the change