Skip to content

Commit 81d48e0

Browse files
authored
[clang][analyzer] Fix a nullptr dereference when -ftime-trace is used (Reland) (#139980)
Fixes #139779. The bug was introduced in #137355 in `SymbolConjured::getStmt`, when trying to obtain a statement for a CFG initializer without an initializer. This commit adds a null check before access. Previous PR #139820, Revert #139936 Additional notes since previous PR: When conjuring a symbol, sometimes there is no valid CFG element, e.g. in the file causing the crash, there is no element at all in the CFG. In these cases, the CFG element reference in the expression engine will be invalid. As a consequence, there needs to be extra checks to ensure the validity of the CFG element reference.
1 parent 1138983 commit 81d48e0

File tree

3 files changed

+49
-35
lines changed

3 files changed

+49
-35
lines changed

clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h

Lines changed: 1 addition & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -100,41 +100,7 @@ class SymbolConjured : public SymbolData {
100100
ConstCFGElementRef getCFGElementRef() const { return Elem; }
101101

102102
// It might return null.
103-
const Stmt *getStmt() const {
104-
switch (Elem->getKind()) {
105-
case CFGElement::Initializer:
106-
return Elem->castAs<CFGInitializer>().getInitializer()->getInit();
107-
case CFGElement::ScopeBegin:
108-
return Elem->castAs<CFGScopeBegin>().getTriggerStmt();
109-
case CFGElement::ScopeEnd:
110-
return Elem->castAs<CFGScopeEnd>().getTriggerStmt();
111-
case CFGElement::NewAllocator:
112-
return Elem->castAs<CFGNewAllocator>().getAllocatorExpr();
113-
case CFGElement::LifetimeEnds:
114-
return Elem->castAs<CFGLifetimeEnds>().getTriggerStmt();
115-
case CFGElement::LoopExit:
116-
return Elem->castAs<CFGLoopExit>().getLoopStmt();
117-
case CFGElement::Statement:
118-
return Elem->castAs<CFGStmt>().getStmt();
119-
case CFGElement::Constructor:
120-
return Elem->castAs<CFGConstructor>().getStmt();
121-
case CFGElement::CXXRecordTypedCall:
122-
return Elem->castAs<CFGCXXRecordTypedCall>().getStmt();
123-
case CFGElement::AutomaticObjectDtor:
124-
return Elem->castAs<CFGAutomaticObjDtor>().getTriggerStmt();
125-
case CFGElement::DeleteDtor:
126-
return Elem->castAs<CFGDeleteDtor>().getDeleteExpr();
127-
case CFGElement::BaseDtor:
128-
return nullptr;
129-
case CFGElement::MemberDtor:
130-
return nullptr;
131-
case CFGElement::TemporaryDtor:
132-
return Elem->castAs<CFGTemporaryDtor>().getBindTemporaryExpr();
133-
case CFGElement::CleanupFunction:
134-
return nullptr;
135-
}
136-
return nullptr;
137-
}
103+
const Stmt *getStmt() const;
138104

139105
unsigned getCount() const { return Count; }
140106
/// It might return null.

clang/lib/StaticAnalyzer/Core/SymbolManager.cpp

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,49 @@ void UnarySymExpr::dumpToStream(raw_ostream &os) const {
8080
os << ')';
8181
}
8282

83+
const Stmt *SymbolConjured::getStmt() const {
84+
// Sometimes the CFG element is invalid, avoid dereferencing it.
85+
if (Elem.getParent() == nullptr ||
86+
Elem.getIndexInBlock() >= Elem.getParent()->size())
87+
return nullptr;
88+
switch (Elem->getKind()) {
89+
case CFGElement::Initializer:
90+
if (const auto *Init = Elem->castAs<CFGInitializer>().getInitializer()) {
91+
return Init->getInit();
92+
}
93+
return nullptr;
94+
case CFGElement::ScopeBegin:
95+
return Elem->castAs<CFGScopeBegin>().getTriggerStmt();
96+
case CFGElement::ScopeEnd:
97+
return Elem->castAs<CFGScopeEnd>().getTriggerStmt();
98+
case CFGElement::NewAllocator:
99+
return Elem->castAs<CFGNewAllocator>().getAllocatorExpr();
100+
case CFGElement::LifetimeEnds:
101+
return Elem->castAs<CFGLifetimeEnds>().getTriggerStmt();
102+
case CFGElement::LoopExit:
103+
return Elem->castAs<CFGLoopExit>().getLoopStmt();
104+
case CFGElement::Statement:
105+
return Elem->castAs<CFGStmt>().getStmt();
106+
case CFGElement::Constructor:
107+
return Elem->castAs<CFGConstructor>().getStmt();
108+
case CFGElement::CXXRecordTypedCall:
109+
return Elem->castAs<CFGCXXRecordTypedCall>().getStmt();
110+
case CFGElement::AutomaticObjectDtor:
111+
return Elem->castAs<CFGAutomaticObjDtor>().getTriggerStmt();
112+
case CFGElement::DeleteDtor:
113+
return Elem->castAs<CFGDeleteDtor>().getDeleteExpr();
114+
case CFGElement::BaseDtor:
115+
return nullptr;
116+
case CFGElement::MemberDtor:
117+
return nullptr;
118+
case CFGElement::TemporaryDtor:
119+
return Elem->castAs<CFGTemporaryDtor>().getBindTemporaryExpr();
120+
case CFGElement::CleanupFunction:
121+
return nullptr;
122+
}
123+
return nullptr;
124+
}
125+
83126
void SymbolConjured::dumpToStream(raw_ostream &os) const {
84127
os << getKindStr() << getSymbolID() << '{' << T << ", LC" << LCtx->getID();
85128
if (auto *S = getStmt())
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
// RUN: %clang_analyze_cc1 -analyzer-checker=core,apiModeling %s -ftime-trace=%t.raw.json -verify
2+
// expected-no-diagnostics
3+
4+
// GitHub issue 139779
5+
struct {} a; // no-crash

0 commit comments

Comments
 (0)