Skip to content

Commit abc8736

Browse files
author
Balazs Benics
committed
[analyzer] Restrict CallDescription fuzzy builtin matching
`CallDescriptions` for builtin functions relaxes the match rules somewhat, so that the `CallDescription` will match for calls that have some prefix or suffix. This was achieved by doing a `StringRef::contains()`. However, this is somewhat problematic for builtins that are substrings of each other. Consider the following: `CallDescription{ builtin, "memcpy"}` will match for `__builtin_wmemcpy()` calls, which is unfortunate. This patch addresses/works around the issue by checking if the characters around the function's name are not part of the 'name' semantically. In other words, to accept a match for `"memcpy"` the call should not have alphanumeric (`[a-zA-Z]`) characters around the 'match'. So, `CallDescription{ builtin, "memcpy"}` will not match on: - `__builtin_wmemcpy: there is a `w` alphanumeric character before the match. - `__builtin_memcpyFOoBar_inline`: there is a `F` character after the match. - `__builtin_memcpyX_inline`: there is an `X` character after the match. But it will still match for: - `memcpy`: exact match - `__builtin_memcpy`: there is an _ before the match - `__builtin_memcpy_inline`: there is an _ after the match - `memcpy_inline_builtinFooBar`: there is an _ after the match Reviewed By: NoQ Differential Revision: https://reviews.llvm.org/D118388
1 parent 0694353 commit abc8736

File tree

2 files changed

+77
-2
lines changed

2 files changed

+77
-2
lines changed

clang/lib/StaticAnalyzer/Core/CheckerContext.cpp

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,29 @@ bool CheckerContext::isCLibraryFunction(const FunctionDecl *FD,
5555
if (Name.empty())
5656
return true;
5757
StringRef BName = FD->getASTContext().BuiltinInfo.getName(BId);
58-
if (BName.contains(Name))
59-
return true;
58+
size_t start = BName.find(Name);
59+
if (start != StringRef::npos) {
60+
// Accept exact match.
61+
if (BName.size() == Name.size())
62+
return true;
63+
64+
// v-- match starts here
65+
// ...xxxxx...
66+
// _xxxxx_
67+
// ^ ^ lookbehind and lookahead characters
68+
69+
const auto MatchPredecessor = [=]() -> bool {
70+
return start <= 0 || !llvm::isAlpha(BName[start - 1]);
71+
};
72+
const auto MatchSuccessor = [=]() -> bool {
73+
std::size_t LookbehindPlace = start + Name.size();
74+
return LookbehindPlace >= BName.size() ||
75+
!llvm::isAlpha(BName[LookbehindPlace]);
76+
};
77+
78+
if (MatchPredecessor() && MatchSuccessor())
79+
return true;
80+
}
6081
}
6182

6283
const IdentifierInfo *II = FD->getIdentifier();

clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -487,6 +487,60 @@ TEST(CallDescription, MatchBuiltins) {
487487
" __builtin___memset_chk(&x, 0, sizeof(x),"
488488
" __builtin_object_size(&x, 0));"
489489
"}"));
490+
491+
{
492+
SCOPED_TRACE("multiple similar builtins");
493+
EXPECT_TRUE(tooling::runToolOnCode(
494+
std::unique_ptr<FrontendAction>(new CallDescriptionAction<>(
495+
{{{CDF_MaybeBuiltin, "memcpy", 3}, false},
496+
{{CDF_MaybeBuiltin, "wmemcpy", 3}, true}})),
497+
R"(void foo(wchar_t *x, wchar_t *y) {
498+
__builtin_wmemcpy(x, y, sizeof(wchar_t));
499+
})"));
500+
}
501+
{
502+
SCOPED_TRACE("multiple similar builtins reversed order");
503+
EXPECT_TRUE(tooling::runToolOnCode(
504+
std::unique_ptr<FrontendAction>(new CallDescriptionAction<>(
505+
{{{CDF_MaybeBuiltin, "wmemcpy", 3}, true},
506+
{{CDF_MaybeBuiltin, "memcpy", 3}, false}})),
507+
R"(void foo(wchar_t *x, wchar_t *y) {
508+
__builtin_wmemcpy(x, y, sizeof(wchar_t));
509+
})"));
510+
}
511+
{
512+
SCOPED_TRACE("lookbehind and lookahead mismatches");
513+
EXPECT_TRUE(tooling::runToolOnCode(
514+
std::unique_ptr<FrontendAction>(
515+
new CallDescriptionAction<>({{{CDF_MaybeBuiltin, "func"}, false}})),
516+
R"(
517+
void funcXXX();
518+
void XXXfunc();
519+
void XXXfuncXXX();
520+
void test() {
521+
funcXXX();
522+
XXXfunc();
523+
XXXfuncXXX();
524+
})"));
525+
}
526+
{
527+
SCOPED_TRACE("lookbehind and lookahead matches");
528+
EXPECT_TRUE(tooling::runToolOnCode(
529+
std::unique_ptr<FrontendAction>(
530+
new CallDescriptionAction<>({{{CDF_MaybeBuiltin, "func"}, true}})),
531+
R"(
532+
void func();
533+
void func_XXX();
534+
void XXX_func();
535+
void XXX_func_XXX();
536+
537+
void test() {
538+
func(); // exact match
539+
func_XXX();
540+
XXX_func();
541+
XXX_func_XXX();
542+
})"));
543+
}
490544
}
491545

492546
} // namespace

0 commit comments

Comments
 (0)