r370244 - [analyzer] Trust global initializers when analyzing main().
Artem Dergachev via cfe-commits
cfe-commits at lists.llvm.org
Wed Aug 28 11:44:32 PDT 2019
Author: dergachev
Date: Wed Aug 28 11:44:32 2019
New Revision: 370244
URL: http://llvm.org/viewvc/llvm-project?rev=370244&view=rev
Log:
[analyzer] Trust global initializers when analyzing main().
If the global variable has an initializer, we'll ignore it because we're usually
not analyzing the program from the beginning, which means that the global
variable may have changed before we start our analysis.
However when we're analyzing main() as the top-level function, we can rely
on global initializers to still be valid. At least in C; in C++ we have global
constructors that can still break this logic.
This patch allows the Static Analyzer to load constant initializers from
global variables if the top-level function of the current analysis is main().
Differential Revision: https://reviews.llvm.org/D65361
Added:
cfe/trunk/test/Analysis/main.c
cfe/trunk/test/Analysis/main.cpp
Modified:
cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
Modified: cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp?rev=370244&r1=370243&r2=370244&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp Wed Aug 28 11:44:32 2019
@@ -155,28 +155,42 @@ class RegionBindingsRef : public llvm::I
ClusterBindings> {
ClusterBindings::Factory *CBFactory;
+ // This flag indicates whether the current bindings are within the analysis
+ // that has started from main(). It affects how we perform loads from
+ // global variables that have initializers: if we have observed the
+ // program execution from the start and we know that these variables
+ // have not been overwritten yet, we can be sure that their initializers
+ // are still relevant. This flag never gets changed when the bindings are
+ // updated, so it could potentially be moved into RegionStoreManager
+ // (as if it's the same bindings but a different loading procedure)
+ // however that would have made the manager needlessly stateful.
+ bool IsMainAnalysis;
+
public:
typedef llvm::ImmutableMapRef<const MemRegion *, ClusterBindings>
ParentTy;
RegionBindingsRef(ClusterBindings::Factory &CBFactory,
const RegionBindings::TreeTy *T,
- RegionBindings::TreeTy::Factory *F)
+ RegionBindings::TreeTy::Factory *F,
+ bool IsMainAnalysis)
: llvm::ImmutableMapRef<const MemRegion *, ClusterBindings>(T, F),
- CBFactory(&CBFactory) {}
+ CBFactory(&CBFactory), IsMainAnalysis(IsMainAnalysis) {}
- RegionBindingsRef(const ParentTy &P, ClusterBindings::Factory &CBFactory)
+ RegionBindingsRef(const ParentTy &P,
+ ClusterBindings::Factory &CBFactory,
+ bool IsMainAnalysis)
: llvm::ImmutableMapRef<const MemRegion *, ClusterBindings>(P),
- CBFactory(&CBFactory) {}
+ CBFactory(&CBFactory), IsMainAnalysis(IsMainAnalysis) {}
RegionBindingsRef add(key_type_ref K, data_type_ref D) const {
return RegionBindingsRef(static_cast<const ParentTy *>(this)->add(K, D),
- *CBFactory);
+ *CBFactory, IsMainAnalysis);
}
RegionBindingsRef remove(key_type_ref K) const {
return RegionBindingsRef(static_cast<const ParentTy *>(this)->remove(K),
- *CBFactory);
+ *CBFactory, IsMainAnalysis);
}
RegionBindingsRef addBinding(BindingKey K, SVal V) const;
@@ -206,7 +220,13 @@ public:
/// Return the internal tree as a Store.
Store asStore() const {
- return asImmutableMap().getRootWithoutRetain();
+ llvm::PointerIntPair<Store, 1, bool> Ptr = {
+ asImmutableMap().getRootWithoutRetain(), IsMainAnalysis};
+ return reinterpret_cast<Store>(Ptr.getOpaqueValue());
+ }
+
+ bool isMainAnalysis() const {
+ return IsMainAnalysis;
}
void printJson(raw_ostream &Out, const char *NL = "\n",
@@ -381,8 +401,15 @@ public:
/// casts from arrays to pointers.
SVal ArrayToPointer(Loc Array, QualType ElementTy) override;
+ /// Creates the Store that correctly represents memory contents before
+ /// the beginning of the analysis of the given top-level stack frame.
StoreRef getInitialStore(const LocationContext *InitLoc) override {
- return StoreRef(RBFactory.getEmptyMap().getRootWithoutRetain(), *this);
+ bool IsMainAnalysis = false;
+ if (const auto *FD = dyn_cast<FunctionDecl>(InitLoc->getDecl()))
+ IsMainAnalysis = FD->isMain() && !Ctx.getLangOpts().CPlusPlus;
+ return StoreRef(RegionBindingsRef(
+ RegionBindingsRef::ParentTy(RBFactory.getEmptyMap(), RBFactory),
+ CBFactory, IsMainAnalysis).asStore(), *this);
}
//===-------------------------------------------------------------------===//
@@ -608,9 +635,13 @@ public: // Part of public interface to c
//===------------------------------------------------------------------===//
RegionBindingsRef getRegionBindings(Store store) const {
- return RegionBindingsRef(CBFactory,
- static_cast<const RegionBindings::TreeTy*>(store),
- RBFactory.getTreeFactory());
+ llvm::PointerIntPair<Store, 1, bool> Ptr;
+ Ptr.setFromOpaqueValue(const_cast<void *>(store));
+ return RegionBindingsRef(
+ CBFactory,
+ static_cast<const RegionBindings::TreeTy *>(Ptr.getPointer()),
+ RBFactory.getTreeFactory(),
+ Ptr.getInt());
}
void printJson(raw_ostream &Out, Store S, const char *NL = "\n",
@@ -1671,10 +1702,12 @@ SVal RegionStoreManager::getBindingForEl
return svalBuilder.makeIntVal(c, T);
}
} else if (const VarRegion *VR = dyn_cast<VarRegion>(superR)) {
- // Check if the containing array is const and has an initialized value.
+ // Check if the containing array has an initialized value that we can trust.
+ // We can trust a const value or a value of a global initializer in main().
const VarDecl *VD = VR->getDecl();
- // Either the array or the array element has to be const.
- if (VD->getType().isConstQualified() || R->getElementType().isConstQualified()) {
+ if (VD->getType().isConstQualified() ||
+ R->getElementType().isConstQualified() ||
+ (B.isMainAnalysis() && VD->hasGlobalStorage())) {
if (const Expr *Init = VD->getAnyInitializer()) {
if (const auto *InitList = dyn_cast<InitListExpr>(Init)) {
// The array index has to be known.
@@ -1763,8 +1796,11 @@ SVal RegionStoreManager::getBindingForFi
const VarDecl *VD = VR->getDecl();
QualType RecordVarTy = VD->getType();
unsigned Index = FD->getFieldIndex();
- // Either the record variable or the field has to be const qualified.
- if (RecordVarTy.isConstQualified() || Ty.isConstQualified())
+ // Either the record variable or the field has an initializer that we can
+ // trust. We trust initializers of constants and, additionally, respect
+ // initializers of globals when analyzing main().
+ if (RecordVarTy.isConstQualified() || Ty.isConstQualified() ||
+ (B.isMainAnalysis() && VD->hasGlobalStorage()))
if (const Expr *Init = VD->getAnyInitializer())
if (const auto *InitList = dyn_cast<InitListExpr>(Init)) {
if (Index < InitList->getNumInits()) {
@@ -1982,6 +2018,12 @@ SVal RegionStoreManager::getBindingForVa
if (isa<GlobalsSpaceRegion>(MS)) {
QualType T = VD->getType();
+ // If we're in main(), then global initializers have not become stale yet.
+ if (B.isMainAnalysis())
+ if (const Expr *Init = VD->getAnyInitializer())
+ if (Optional<SVal> V = svalBuilder.getConstantVal(Init))
+ return *V;
+
// Function-scoped static variables are default-initialized to 0; if they
// have an initializer, it would have been processed by now.
// FIXME: This is only true when we're starting analysis from main().
Added: cfe/trunk/test/Analysis/main.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/main.c?rev=370244&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/main.c (added)
+++ cfe/trunk/test/Analysis/main.c Wed Aug 28 11:44:32 2019
@@ -0,0 +1,32 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify %s
+
+int x = 1;
+
+struct {
+ int a, b;
+} s = {2, 3};
+
+int arr[] = {4, 5, 6};
+
+void clang_analyzer_eval(int);
+
+int main() {
+ // In main() we know that the initial values are still valid.
+ clang_analyzer_eval(x == 1); // expected-warning{{TRUE}}
+ clang_analyzer_eval(s.a == 2); // expected-warning{{TRUE}}
+ clang_analyzer_eval(s.b == 3); // expected-warning{{TRUE}}
+ clang_analyzer_eval(arr[0] == 4); // expected-warning{{TRUE}}
+ clang_analyzer_eval(arr[1] == 5); // expected-warning{{TRUE}}
+ clang_analyzer_eval(arr[2] == 6); // expected-warning{{TRUE}}
+ return 0;
+}
+
+void foo() {
+ // In other functions these values may already be overwritten.
+ clang_analyzer_eval(x == 1); // expected-warning{{TRUE}} // expected-warning{{FALSE}}
+ clang_analyzer_eval(s.a == 2); // expected-warning{{TRUE}} // expected-warning{{FALSE}}
+ clang_analyzer_eval(s.b == 3); // expected-warning{{TRUE}} // expected-warning{{FALSE}}
+ clang_analyzer_eval(arr[0] == 4); // expected-warning{{TRUE}} // expected-warning{{FALSE}}
+ clang_analyzer_eval(arr[1] == 5); // expected-warning{{TRUE}} // expected-warning{{FALSE}}
+ clang_analyzer_eval(arr[2] == 6); // expected-warning{{TRUE}} // expected-warning{{FALSE}}
+}
Added: cfe/trunk/test/Analysis/main.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/main.cpp?rev=370244&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/main.cpp (added)
+++ cfe/trunk/test/Analysis/main.cpp Wed Aug 28 11:44:32 2019
@@ -0,0 +1,22 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify %s
+
+int x = 1;
+
+struct {
+ int a, b;
+} s = {2, 3};
+
+int arr[] = {4, 5, 6};
+
+void clang_analyzer_eval(int);
+
+int main() {
+ // Do not trust global initializers in C++.
+ clang_analyzer_eval(x == 1); // expected-warning{{TRUE}} // expected-warning{{FALSE}}
+ clang_analyzer_eval(s.a == 2); // expected-warning{{TRUE}} // expected-warning{{FALSE}}
+ clang_analyzer_eval(s.b == 3); // expected-warning{{TRUE}} // expected-warning{{FALSE}}
+ clang_analyzer_eval(arr[0] == 4); // expected-warning{{TRUE}} // expected-warning{{FALSE}}
+ clang_analyzer_eval(arr[1] == 5); // expected-warning{{TRUE}} // expected-warning{{FALSE}}
+ clang_analyzer_eval(arr[2] == 6); // expected-warning{{TRUE}} // expected-warning{{FALSE}}
+ return 0;
+}
More information about the cfe-commits
mailing list