NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a_sidorin, rnkovacs, Szelethus,
baloghadamsoftware, Charusso.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, mikhail.ramalho,
a.sidorin, JDevlieghere, szepet.
Herald added a project: clang.
This is inspired by https://bugs.llvm.org/show_bug.cgi?id=42780, even though it
doesn't fix the bug.
If the global variable has an initializer, we'll ignore it because we're
normally not analyzing the program from the beginning. This means that a 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 for breaking this logic. In C we can also have global constructors
as a GNU extension, but i'd rather not worry about that.
In this patch i don't try to evaluate global initializers; the patch only
covers the case when they're constants. This is the reason why we don't cover
https://bugs.llvm.org/show_bug.cgi?id=42780, however we could cover it if
`SValBuilder.getConstantVal()` was a bit more feature-rich (namely, we need to
be able to constant-evaluate expressions like `&var.field` where `var` is a
global variable; it's a concrete region (i.e., without symbolic base) value in
our model, so we can make `getConstantVal()` powerful enough to emit it).
For initializers of fields and elements i'm piggybacking on @r.stahl's work to
cover global constant initializers (D45774 <https://reviews.llvm.org/D45774>).
The main benefit of this patch is to make it harder for people to send us false
positive reproducers that don't actually reproduce the false positive that
they're having :)
Repository:
rC Clang
https://reviews.llvm.org/D65361
Files:
clang/lib/StaticAnalyzer/Core/RegionStore.cpp
clang/test/Analysis/main.c
Index: clang/test/Analysis/main.c
===================================================================
--- /dev/null
+++ clang/test/Analysis/main.c
@@ -0,0 +1,22 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection \
+// RUN: -verify %s
+
+int x = 1;
+
+struct {
+ int a, b;
+} s = {2, 3};
+
+int arr[] = {4, 5, 6};
+
+void clang_analyzer_eval(int);
+
+int main() {
+ 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;
+}
Index: clang/lib/StaticAnalyzer/Core/RegionStore.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ clang/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -154,6 +154,7 @@
class RegionBindingsRef : public llvm::ImmutableMapRef<const MemRegion *,
ClusterBindings> {
ClusterBindings::Factory *CBFactory;
+ bool IsMainAnalysis;
public:
typedef llvm::ImmutableMapRef<const MemRegion *, ClusterBindings>
@@ -161,22 +162,25 @@
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 +210,14 @@
/// Return the internal tree as a Store.
Store asStore() const {
- return asImmutableMap().getRootWithoutRetain();
+ static_assert(alignof(decltype(*asImmutableMap().getRootWithoutRetain())) > 1,
+ "No room for the flag!");
+ return (Store)((uintptr_t)(asImmutableMap().getRootWithoutRetain()) |
+ (uintptr_t)(IsMainAnalysis));
+ }
+
+ bool isMainAnalysis() const {
+ return IsMainAnalysis;
}
void printJson(raw_ostream &Out, const char *NL = "\n",
@@ -382,7 +393,12 @@
SVal ArrayToPointer(Loc Array, QualType ElementTy) override;
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();
+ return StoreRef(RegionBindingsRef(
+ RegionBindingsRef::ParentTy(RBFactory.getEmptyMap(), RBFactory),
+ CBFactory, IsMainAnalysis).asStore(), *this);
}
//===-------------------------------------------------------------------===//
@@ -608,9 +624,10 @@
//===------------------------------------------------------------------===//
RegionBindingsRef getRegionBindings(Store store) const {
- return RegionBindingsRef(CBFactory,
- static_cast<const RegionBindings::TreeTy*>(store),
- RBFactory.getTreeFactory());
+ return RegionBindingsRef(
+ CBFactory,
+ (const RegionBindings::TreeTy *)((uintptr_t)store & ~(uintptr_t)1),
+ RBFactory.getTreeFactory(), (bool)((uintptr_t)store & (uintptr_t)1));
}
void printJson(raw_ostream &Out, Store S, const char *NL = "\n",
@@ -1674,7 +1691,9 @@
// Check if the containing array is const and has an initialized value.
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.
@@ -1764,7 +1783,8 @@
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())
+ 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 +2002,11 @@
if (isa<GlobalsSpaceRegion>(MS)) {
QualType T = VD->getType();
+ 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().
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
