Szelethus created this revision.
Szelethus added reviewers: NoQ, martong, steakhal, balazske, isuckatcs.
Szelethus added a project: clang.
Herald added subscribers: manas, ASDenysPetrov, gamesh411, dkrupp, donat.nagy,
mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun.
Herald added a project: All.
Szelethus requested review of this revision.
Herald added a subscriber: cfe-commits.
Discourse mail:
https://discourse.llvm.org/t/analyzer-why-do-we-suck-at-modeling-c-dynamic-memory/65667
`malloc()` returns a piece of uninitialized dynamic memory. We were (almost)
always to model this behaviour. Its C++ counterpart, `operator new` is a lot
more complex, because it allows for initialization, the most complicated of
which the usage of constructors.
We gradually became better in modeling constructors, but for some reason, most
likely for reasons lost in history, we never actually modeled the case when the
memory returned by `operator new` was just simply uninitialized. This patch
(attempts) to fix this tiny little error.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D135375
Files:
clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
clang/test/Analysis/NewDelete-checker-test.cpp
clang/test/Analysis/cxx-member-initializer-const-field.cpp
clang/test/Analysis/new-ctor-conservative.cpp
clang/test/Analysis/new-ctor-recursive.cpp
clang/test/Analysis/new.cpp
clang/test/Analysis/placement-new.cpp
clang/test/Analysis/reinterpret-cast.cpp
clang/test/Analysis/uninit-const.cpp
Index: clang/test/Analysis/uninit-const.cpp
===================================================================
--- clang/test/Analysis/uninit-const.cpp
+++ clang/test/Analysis/uninit-const.cpp
@@ -21,9 +21,11 @@
int f10(void) {
int *ptr;
-
- ptr = new int; //
- if(*ptr) {
+ // FIXME: The message is misleading -- we should state that
+ // a pointer to an uninitialized value is stored.
+ ptr = new int; // expected-note{{Storing uninitialized value}}
+ if(*ptr) { // expected-warning{{Branch condition evaluates to a garbage value [core.uninitialized.Branch]}}
+ // expected-note@-1 {{Branch condition evaluates to a garbage value}}
doStuff4(*ptr);
}
delete ptr;
@@ -32,10 +34,12 @@
int f9(void) {
int *ptr;
-
- ptr = new int; //
-
- doStuff_uninit(ptr); // no warning
+ // FIXME: The message is misleading -- we should state that
+ // a pointer to an uninitialized value is stored.
+ ptr = new int; // expected-note{{Storing uninitialized value}}
+ // expected-note@-1{{Value assigned to 'ptr'}}
+ doStuff_uninit(ptr); // expected-warning{{1st function call argument is a pointer to uninitialized value [core.CallAndMessage]}}
+ // expected-note@-1{{1st function call argument is a pointer to uninitialized value}}
delete ptr;
return 0;
}
Index: clang/test/Analysis/reinterpret-cast.cpp
===================================================================
--- clang/test/Analysis/reinterpret-cast.cpp
+++ clang/test/Analysis/reinterpret-cast.cpp
@@ -77,15 +77,11 @@
class Derived : public Base {};
void test() {
- Derived* p;
- *(reinterpret_cast<void**>(&p)) = new C;
- p->f();
-
- // We should still be able to do some reasoning about bindings.
- p->x = 42;
- clang_analyzer_eval(p->x == 42); // expected-warning{{TRUE}}
+ Derived* p;
+ *(reinterpret_cast<void**>(&p)) = new C;
+ p->f(); // expected-warning{{Called function pointer is an uninitialized pointer value [core.CallAndMessage]}}
};
-}
+} // namespace PR15345
int trackpointer_std_addressof() {
int x;
Index: clang/test/Analysis/placement-new.cpp
===================================================================
--- clang/test/Analysis/placement-new.cpp
+++ clang/test/Analysis/placement-new.cpp
@@ -112,6 +112,10 @@
namespace testHeapAllocatedBuffer {
void g2() {
char *buf = new char[2]; // expected-note {{'buf' initialized here}}
+ // FIXME: The message is misleading -- we should
+ // state that a pointer to an uninitialized value
+ // is stored.
+ // expected-note@-4{{Storing uninitialized value}}
long *lp = ::new (buf) long; // expected-warning{{Storage provided to placement new is only 2 bytes, whereas the allocated type requires 8 bytes}} expected-note 1 {{}}
(void)lp;
}
Index: clang/test/Analysis/new.cpp
===================================================================
--- clang/test/Analysis/new.cpp
+++ clang/test/Analysis/new.cpp
@@ -184,8 +184,7 @@
int testNoInitialization() {
int *n = new int;
- // Should warn that *n is uninitialized.
- if (*n) { // no-warning
+ if (*n) { // expected-warning{{Branch condition evaluates to a garbage value [core.uninitialized.Branch]}}
delete n;
return 0;
}
Index: clang/test/Analysis/new-ctor-recursive.cpp
===================================================================
--- clang/test/Analysis/new-ctor-recursive.cpp
+++ clang/test/Analysis/new-ctor-recursive.cpp
@@ -51,11 +51,9 @@
void testThatCharConstructorIndeedYieldsGarbage() {
S *s = new S(ConstructionKind::Garbage);
- clang_analyzer_eval(s->x == 0); // expected-warning{{UNKNOWN}}
- clang_analyzer_eval(s->x == 1); // expected-warning{{UNKNOWN}}
- // FIXME: This should warn, but MallocChecker doesn't default-bind regions
- // returned by standard operator new to garbage.
- s->x += 1; // no-warning
+ clang_analyzer_eval(s->x == 0); // expected-warning{{The left operand of '==' is a garbage value [core.UndefinedBinaryOperatorResult]}}
+ clang_analyzer_eval(s->x == 1);
+ s->x += 1;
delete s;
}
Index: clang/test/Analysis/new-ctor-conservative.cpp
===================================================================
--- clang/test/Analysis/new-ctor-conservative.cpp
+++ clang/test/Analysis/new-ctor-conservative.cpp
@@ -14,9 +14,12 @@
clang_analyzer_eval(s->x == 1); // expected-warning{{TRUE}}
}
-void checkNewPOD() {
+void checkNewPODunit() {
int *i = new int;
- clang_analyzer_eval(*i == 0); // expected-warning{{UNKNOWN}}
+ clang_analyzer_eval(*i == 0); // expected-warning{{The left operand of '==' is a garbage value [core.UndefinedBinaryOperatorResult]}}
+}
+
+void checkNewPOD() {
int *j = new int();
clang_analyzer_eval(*j == 0); // expected-warning{{TRUE}}
int *k = new int(5);
Index: clang/test/Analysis/cxx-member-initializer-const-field.cpp
===================================================================
--- clang/test/Analysis/cxx-member-initializer-const-field.cpp
+++ clang/test/Analysis/cxx-member-initializer-const-field.cpp
@@ -10,7 +10,7 @@
WithConstructor(int *x) : ptr(x) {}
static auto compliant() {
- WithConstructor c(new int);
+ WithConstructor c(new int{});
return *(c.ptr); // no warning
}
@@ -28,7 +28,7 @@
int *const ptr = nullptr;
static int compliant() {
- RegularAggregate c{new int};
+ RegularAggregate c{new int{}};
return *(c.ptr); // no warning
}
Index: clang/test/Analysis/NewDelete-checker-test.cpp
===================================================================
--- clang/test/Analysis/NewDelete-checker-test.cpp
+++ clang/test/Analysis/NewDelete-checker-test.cpp
@@ -385,7 +385,11 @@
public:
int *x;
DerefClass() {}
- ~DerefClass() {*x = 1;}
+ ~DerefClass() {
+ int i = 0;
+ x = &i;
+ *x = 1;
+ }
};
void testDoubleDeleteClassInstance() {
Index: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -10,15 +10,16 @@
//
//===----------------------------------------------------------------------===//
-#include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
-#include "clang/Analysis/ConstructionContext.h"
#include "clang/AST/DeclCXX.h"
-#include "clang/AST/StmtCXX.h"
#include "clang/AST/ParentMap.h"
+#include "clang/AST/StmtCXX.h"
+#include "clang/Analysis/ConstructionContext.h"
#include "clang/Basic/PrettyStackTrace.h"
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
using namespace clang;
using namespace ento;
@@ -923,6 +924,7 @@
// skip it for now.
ProgramStateRef State = I->getState();
SVal RetVal = State->getSVal(CNE, LCtx);
+ State = State->bindDefaultInitial(RetVal, UndefinedVal{}, LCtx);
// If this allocation function is not declared as non-throwing, failures
// /must/ be signalled by exceptions, and thus the return value will never
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
