Author: Fangyi Zhou Date: 2025-05-15T19:29:58+02:00 New Revision: 81d48e0f61f3e78cd6d6be9d3c8e48e7761a5ed5
URL: https://github.com/llvm/llvm-project/commit/81d48e0f61f3e78cd6d6be9d3c8e48e7761a5ed5 DIFF: https://github.com/llvm/llvm-project/commit/81d48e0f61f3e78cd6d6be9d3c8e48e7761a5ed5.diff LOG: [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. Added: clang/test/Analysis/ftime-trace-no-init.cpp Modified: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h clang/lib/StaticAnalyzer/Core/SymbolManager.cpp Removed: ################################################################################ diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h index 9e7c98fdded17..86774ad5043dd 100644 --- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h +++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h @@ -100,41 +100,7 @@ class SymbolConjured : public SymbolData { ConstCFGElementRef getCFGElementRef() const { return Elem; } // It might return null. - const Stmt *getStmt() const { - switch (Elem->getKind()) { - case CFGElement::Initializer: - return Elem->castAs<CFGInitializer>().getInitializer()->getInit(); - case CFGElement::ScopeBegin: - return Elem->castAs<CFGScopeBegin>().getTriggerStmt(); - case CFGElement::ScopeEnd: - return Elem->castAs<CFGScopeEnd>().getTriggerStmt(); - case CFGElement::NewAllocator: - return Elem->castAs<CFGNewAllocator>().getAllocatorExpr(); - case CFGElement::LifetimeEnds: - return Elem->castAs<CFGLifetimeEnds>().getTriggerStmt(); - case CFGElement::LoopExit: - return Elem->castAs<CFGLoopExit>().getLoopStmt(); - case CFGElement::Statement: - return Elem->castAs<CFGStmt>().getStmt(); - case CFGElement::Constructor: - return Elem->castAs<CFGConstructor>().getStmt(); - case CFGElement::CXXRecordTypedCall: - return Elem->castAs<CFGCXXRecordTypedCall>().getStmt(); - case CFGElement::AutomaticObjectDtor: - return Elem->castAs<CFGAutomaticObjDtor>().getTriggerStmt(); - case CFGElement::DeleteDtor: - return Elem->castAs<CFGDeleteDtor>().getDeleteExpr(); - case CFGElement::BaseDtor: - return nullptr; - case CFGElement::MemberDtor: - return nullptr; - case CFGElement::TemporaryDtor: - return Elem->castAs<CFGTemporaryDtor>().getBindTemporaryExpr(); - case CFGElement::CleanupFunction: - return nullptr; - } - return nullptr; - } + const Stmt *getStmt() const; unsigned getCount() const { return Count; } /// It might return null. diff --git a/clang/lib/StaticAnalyzer/Core/SymbolManager.cpp b/clang/lib/StaticAnalyzer/Core/SymbolManager.cpp index a6ade661d04a2..a469df4ca7160 100644 --- a/clang/lib/StaticAnalyzer/Core/SymbolManager.cpp +++ b/clang/lib/StaticAnalyzer/Core/SymbolManager.cpp @@ -80,6 +80,49 @@ void UnarySymExpr::dumpToStream(raw_ostream &os) const { os << ')'; } +const Stmt *SymbolConjured::getStmt() const { + // Sometimes the CFG element is invalid, avoid dereferencing it. + if (Elem.getParent() == nullptr || + Elem.getIndexInBlock() >= Elem.getParent()->size()) + return nullptr; + switch (Elem->getKind()) { + case CFGElement::Initializer: + if (const auto *Init = Elem->castAs<CFGInitializer>().getInitializer()) { + return Init->getInit(); + } + return nullptr; + case CFGElement::ScopeBegin: + return Elem->castAs<CFGScopeBegin>().getTriggerStmt(); + case CFGElement::ScopeEnd: + return Elem->castAs<CFGScopeEnd>().getTriggerStmt(); + case CFGElement::NewAllocator: + return Elem->castAs<CFGNewAllocator>().getAllocatorExpr(); + case CFGElement::LifetimeEnds: + return Elem->castAs<CFGLifetimeEnds>().getTriggerStmt(); + case CFGElement::LoopExit: + return Elem->castAs<CFGLoopExit>().getLoopStmt(); + case CFGElement::Statement: + return Elem->castAs<CFGStmt>().getStmt(); + case CFGElement::Constructor: + return Elem->castAs<CFGConstructor>().getStmt(); + case CFGElement::CXXRecordTypedCall: + return Elem->castAs<CFGCXXRecordTypedCall>().getStmt(); + case CFGElement::AutomaticObjectDtor: + return Elem->castAs<CFGAutomaticObjDtor>().getTriggerStmt(); + case CFGElement::DeleteDtor: + return Elem->castAs<CFGDeleteDtor>().getDeleteExpr(); + case CFGElement::BaseDtor: + return nullptr; + case CFGElement::MemberDtor: + return nullptr; + case CFGElement::TemporaryDtor: + return Elem->castAs<CFGTemporaryDtor>().getBindTemporaryExpr(); + case CFGElement::CleanupFunction: + return nullptr; + } + return nullptr; +} + void SymbolConjured::dumpToStream(raw_ostream &os) const { os << getKindStr() << getSymbolID() << '{' << T << ", LC" << LCtx->getID(); if (auto *S = getStmt()) diff --git a/clang/test/Analysis/ftime-trace-no-init.cpp b/clang/test/Analysis/ftime-trace-no-init.cpp new file mode 100644 index 0000000000000..7fb289b19da78 --- /dev/null +++ b/clang/test/Analysis/ftime-trace-no-init.cpp @@ -0,0 +1,5 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=core,apiModeling %s -ftime-trace=%t.raw.json -verify +// expected-no-diagnostics + +// GitHub issue 139779 +struct {} a; // no-crash _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
