Skip to content

Commit 60b4854

Browse files
authored
[SSAF] Let UnsafeBufferUsageExtractor & PointerFlowExtractor ignore templates (#198927)
Templates are ignored for two reasons: - Template instantiations are still handled. Template facts can be inferred from their instantiations. - Templates are inherently difficult to reason about. Their ASTs can contain dependent expression types (such as ParenListExpr) that complicate analysis.
1 parent 8f249a7 commit 60b4854

6 files changed

Lines changed: 158 additions & 2 deletions

File tree

clang/lib/ScalableStaticAnalysisFramework/Analyses/PointerFlow/PointerFlowExtractor.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -336,6 +336,12 @@ class PointerFlowTUSummaryExtractor : public TUSummaryExtractor {
336336

337337
findContributors(Ctx, Contributors);
338338
for (auto *CD : Contributors) {
339+
// Templates are skipped, but their instantiations are handled. The idea
340+
// is that we can conclude facts about a template through all of its
341+
// instantiations.
342+
if (CD->isTemplated())
343+
continue;
344+
339345
auto EntitySummary = extractEntitySummary(CD, Ctx, *this);
340346

341347
assert(EntitySummary);

clang/lib/ScalableStaticAnalysisFramework/Analyses/SSAFAnalysesCommon.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,11 @@ class ContributorFinder : public DynamicRecursiveASTVisitor {
3434
public:
3535
std::set<const NamedDecl *> Contributors;
3636

37+
ContributorFinder() {
38+
ShouldVisitTemplateInstantiations = true;
39+
ShouldVisitImplicitCode = false;
40+
}
41+
3742
bool VisitFunctionDecl(FunctionDecl *D) override {
3843
Contributors.insert(D);
3944
return true;

clang/lib/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsageExtractor.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,12 @@ void clang::ssaf::UnsafeBufferUsageTUSummaryExtractor::HandleTranslationUnit(
7474

7575
findContributors(Ctx, Contributors);
7676
for (auto *CD : Contributors) {
77+
// Templates are skipped, but their instantiations are handled. The idea
78+
// is that we can conclude facts about a template through all of its
79+
// instantiations.
80+
if (CD->isTemplated())
81+
continue;
82+
7783
auto EntitySummary = extractEntitySummary(CD, Ctx);
7884

7985
assert(EntitySummary);

clang/unittests/ScalableStaticAnalysisFramework/Analyses/PointerFlow/PointerFlowTest.cpp

Lines changed: 71 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,9 +104,13 @@ const SomeDecl *findEntityByName(FindEntityByName Name, ASTContext &Ctx) {
104104
const SomeDecl *FoundDecl = nullptr;
105105

106106
NamedDeclFinder(FindEntityByName SearchingName)
107-
: SearchingName(SearchingName) {}
107+
: SearchingName(SearchingName) {
108+
ShouldVisitTemplateInstantiations = true;
109+
}
108110

109111
bool VisitDecl(Decl *D) override {
112+
if (D->isTemplated())
113+
return true;
110114
if (auto *ND = dyn_cast<NamedDecl>(D)) {
111115
FoundDecl = llvm::dyn_cast_or_null<SomeDecl>(
112116
matchNamedDeclByFindEntityByName(SearchingName, ND));
@@ -1265,6 +1269,72 @@ TEST_F(PointerFlowTest, CXXConstructExprArrayInit) {
12651269
EXPECT_EQ(*Sum, makeEdges(__LINE__, {{{"q", 1U}, {"arr", 1U}}}));
12661270
}
12671271

1272+
//////////////////////////////////////////////////////////////
1273+
// Template is ignored but instantiations are visited. //
1274+
//////////////////////////////////////////////////////////////
1275+
TEST_F(PointerFlowTest, FunctionTemplate) {
1276+
ASSERT_TRUE(setUpTest(R"cpp(
1277+
template <typename T>
1278+
T* f(T *p) {
1279+
int *q = p;
1280+
return q;
1281+
}
1282+
)cpp"));
1283+
ASSERT_FALSE(findEntityByName("f", AST->getASTContext()));
1284+
}
1285+
1286+
TEST_F(PointerFlowTest, MethodInClassTemplate) {
1287+
ASSERT_TRUE(setUpTest(R"cpp(
1288+
template <typename T>
1289+
struct Wrapper {
1290+
T *ptr;
1291+
void set(T *p) { ptr = p; }
1292+
};
1293+
)cpp"));
1294+
ASSERT_FALSE(findEntityByName("set", AST->getASTContext()));
1295+
}
1296+
1297+
TEST_F(PointerFlowTest, FunctionTemplateInstantiation) {
1298+
ASSERT_TRUE(setUpTest(R"cpp(
1299+
template <typename T>
1300+
T* f(T *p) {
1301+
int *q = p;
1302+
return q;
1303+
}
1304+
1305+
void test(int *p) {
1306+
f(p);
1307+
}
1308+
)cpp"));
1309+
1310+
auto *Sum = getEntitySummary<FunctionDecl>("f");
1311+
1312+
ASSERT_TRUE(Sum);
1313+
ASSERT_EQ(*Sum, makeEdges(__LINE__, {{{"q", 1U}, {"p", 1U}},
1314+
{{"f", 1U, 1}, {"q", 1U}}}));
1315+
}
1316+
1317+
TEST_F(PointerFlowTest, MethodInClassTemplateInstantiation) {
1318+
ASSERT_TRUE(setUpTest(R"cpp(
1319+
template <typename T>
1320+
struct Wrapper {
1321+
T *ptr;
1322+
void set(T *p) { ptr = p; }
1323+
};
1324+
1325+
void test(int *p) {
1326+
Wrapper<int> W;
1327+
1328+
W.set(p);
1329+
}
1330+
)cpp"));
1331+
1332+
auto *Sum = getEntitySummary<FunctionDecl>("set");
1333+
1334+
ASSERT_TRUE(Sum);
1335+
ASSERT_EQ(*Sum, makeEdges(__LINE__, {{{"ptr", 1U}, {"p", 1U}}}));
1336+
}
1337+
12681338
//////////////////////////////////////////////////////////////
12691339
// Robustness Tests (No Crash Tests) //
12701340
//////////////////////////////////////////////////////////////

clang/unittests/ScalableStaticAnalysisFramework/Analyses/UnsafeBufferUsage/UnsafeBufferUsageTest.cpp

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -688,6 +688,71 @@ TEST_F(UnsafeBufferUsageTest, CXXScalarValueInitExpr) {
688688
EXPECT_EQ(*Sum, makeSet(__LINE__, {{"q", 1U}}));
689689
}
690690

691+
//////////////////////////////////////////////////////////////
692+
// Template is ignored but instantiations are visited. //
693+
//////////////////////////////////////////////////////////////
694+
695+
TEST_F(UnsafeBufferUsageTest, FunctionTemplate) {
696+
ASSERT_TRUE(setUpTest(R"cpp(
697+
template <typename T>
698+
T* f(T *p) {
699+
return &p[5];
700+
}
701+
)cpp"));
702+
ASSERT_FALSE(findDeclByName("f", AST->getASTContext()));
703+
}
704+
705+
TEST_F(UnsafeBufferUsageTest, MethodInClassTemplate) {
706+
ASSERT_TRUE(setUpTest(R"cpp(
707+
template <typename T>
708+
struct Wrapper {
709+
T *ptr;
710+
void set(T *p) { ptr = p[5]; }
711+
};
712+
)cpp"));
713+
ASSERT_FALSE(findDeclByName("set", AST->getASTContext()));
714+
}
715+
716+
TEST_F(UnsafeBufferUsageTest, FunctionTemplateInstantiation) {
717+
ASSERT_TRUE(setUpTest(R"cpp(
718+
template<typename T>
719+
void unsafe(T p) {
720+
p[1] = p[2] + p[3];
721+
}
722+
723+
void f(int *p) {
724+
unsafe(p);
725+
}
726+
)cpp"));
727+
728+
auto *Sum = getEntitySummary("unsafe");
729+
730+
ASSERT_TRUE(Sum);
731+
ASSERT_EQ(*Sum, makeSet(__LINE__, {{"p", 1U}}));
732+
}
733+
734+
TEST_F(UnsafeBufferUsageTest, MethodInClassTemplateInstantiation) {
735+
ASSERT_TRUE(setUpTest(R"cpp(
736+
template<typename T>
737+
struct UnsafeClass {
738+
T p;
739+
void unsafe_method() {
740+
p[1] = p[2] + p[3];
741+
}
742+
};
743+
744+
void f(int *p) {
745+
UnsafeClass<int *> UC;
746+
747+
UC.unsafe_method();
748+
}
749+
)cpp"));
750+
751+
auto *Sum = getEntitySummary("unsafe_method");
752+
753+
ASSERT_TRUE(Sum);
754+
EXPECT_EQ(*Sum, makeSet(__LINE__, {{"p", 1U}}));
755+
}
691756
// Robustness test: unsupported constructs will not cause crash
692757
#ifndef NDEBUG
693758
TEST_F(UnsafeBufferUsageTest, StmtExprArrayAccess) {

clang/unittests/ScalableStaticAnalysisFramework/FindDecl.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,13 @@ const SomeDecl *findDeclByName(StringRef Name, ASTContext &Ctx) {
2121
StringRef SearchingName;
2222
const NamedDecl *FoundDecl = nullptr;
2323

24-
NamedDeclFinder(StringRef SearchingName) : SearchingName(SearchingName) {}
24+
NamedDeclFinder(StringRef SearchingName) : SearchingName(SearchingName) {
25+
ShouldVisitTemplateInstantiations = true;
26+
}
2527

2628
bool VisitDecl(Decl *D) override {
29+
if (D->isTemplated())
30+
return true;
2731
if (const auto *ND = dyn_cast<SomeDecl>(D)) {
2832
if (ND->getNameAsString() == SearchingName) {
2933
FoundDecl = ND;

0 commit comments

Comments
 (0)