Browse Source

[analyzer] Fix false negative on post-increment of uninitialized variable.

Summary:
Currently clang static analyzer does warn on:
```
int x;
x+=1;
x-=1;
x=x+1;
x=x-1;
```
But does warn on:
```
int x;
x++;
x--;
--x;
++x;
```

This differential should fix that.
Fixes https://bugs.llvm.org/show_bug.cgi?id=35419

Reviewers: dcoughlin, NoQ

Reviewed By: dcoughlin

Subscribers: NoQ, xazax.hun, szepet, cfe-commits, a.sidorin

Tags: #clang

Differential Revision: https://reviews.llvm.org/D40463

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@319411 91177308-0d34-0410-b5e6-96231b3b80d8
Roman Lebedev 7 years ago
parent
commit
d9b383bb5c

+ 3 - 0
docs/ReleaseNotes.rst

@@ -257,6 +257,9 @@ libclang
 Static Analyzer
 Static Analyzer
 ---------------
 ---------------
 
 
+- Static Analyzer can now properly detect and diagnose unary pre-/post-
+  increment/decrement on an uninitialized value.
+
 ...
 ...
 
 
 Undefined Behavior Sanitizer (UBSan)
 Undefined Behavior Sanitizer (UBSan)

+ 8 - 0
lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp

@@ -60,6 +60,14 @@ void UndefinedAssignmentChecker::checkBind(SVal location, SVal val,
   const Expr *ex = nullptr;
   const Expr *ex = nullptr;
 
 
   while (StoreE) {
   while (StoreE) {
+    if (const UnaryOperator *U = dyn_cast<UnaryOperator>(StoreE)) {
+      str = "The expression is an uninitialized value. "
+            "The computed value will also be garbage";
+
+      ex = U->getSubExpr();
+      break;
+    }
+
     if (const BinaryOperator *B = dyn_cast<BinaryOperator>(StoreE)) {
     if (const BinaryOperator *B = dyn_cast<BinaryOperator>(StoreE)) {
       if (B->isCompoundAssignmentOp()) {
       if (B->isCompoundAssignmentOp()) {
         ProgramStateRef state = C.getState();
         ProgramStateRef state = C.getState();

+ 8 - 1
lib/StaticAnalyzer/Core/ExprEngineC.cpp

@@ -1043,7 +1043,14 @@ void ExprEngine::VisitIncrementDecrementOperator(const UnaryOperator* U,
 
 
     // Propagate unknown and undefined values.
     // Propagate unknown and undefined values.
     if (V2_untested.isUnknownOrUndef()) {
     if (V2_untested.isUnknownOrUndef()) {
-      Bldr.generateNode(U, *I, state->BindExpr(U, LCtx, V2_untested));
+      state = state->BindExpr(U, LCtx, V2_untested);
+
+      // Perform the store, so that the uninitialized value detection happens.
+      Bldr.takeNodes(*I);
+      ExplodedNodeSet Dst3;
+      evalStore(Dst3, U, U, *I, state, loc, V2_untested);
+      Bldr.addNodes(Dst3);
+
       continue;
       continue;
     }
     }
     DefinedSVal V2 = V2_untested.castAs<DefinedSVal>();
     DefinedSVal V2 = V2_untested.castAs<DefinedSVal>();

File diff suppressed because it is too large
+ 131 - 129
test/Analysis/malloc-plist.c


+ 3 - 3
test/Analysis/objc-for.m

@@ -128,7 +128,7 @@ int collectionIsNotEmptyNSArray(NSArray *A) {
   int count = [A count];
   int count = [A count];
   if (count > 0) {
   if (count > 0) {
     int i;
     int i;
-    int j;
+    int j = 0;
     for (NSString *a in A) {
     for (NSString *a in A) {
       i = 1;
       i = 1;
       j++;
       j++;
@@ -141,7 +141,7 @@ int collectionIsNotEmptyNSArray(NSArray *A) {
 void onlySuppressExitAfterZeroIterations(NSMutableDictionary *D) {
 void onlySuppressExitAfterZeroIterations(NSMutableDictionary *D) {
   if (D.count > 0) {
   if (D.count > 0) {
     int *x;
     int *x;
-    int i;
+    int i = 0;
     for (NSString *key in D) {
     for (NSString *key in D) {
       x = 0;
       x = 0;
       i++;
       i++;
@@ -155,7 +155,7 @@ void onlySuppressExitAfterZeroIterations(NSMutableDictionary *D) {
 void onlySuppressLoopExitAfterZeroIterations_WithContinue(NSMutableDictionary *D) {
 void onlySuppressLoopExitAfterZeroIterations_WithContinue(NSMutableDictionary *D) {
   if (D.count > 0) {
   if (D.count > 0) {
     int *x;
     int *x;
-    int i;
+    int i = 0;
     for (NSString *key in D) {
     for (NSString *key in D) {
       x = 0;
       x = 0;
       i++;
       i++;

+ 29 - 1
test/Analysis/uninit-const.c

@@ -1,4 +1,6 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=unix.Malloc,core,alpha.core.CallAndMessageUnInitRefArg -analyzer-output=text -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=unix.Malloc,core,alpha.core.CallAndMessageUnInitRefArg,debug.ExprInspection -analyzer-output=text -verify %s
+
+void clang_analyzer_warnIfReached();
 
 
 // Passing uninitialized const data to function
 // Passing uninitialized const data to function
 #include "Inputs/system-header-simulator.h"
 #include "Inputs/system-header-simulator.h"
@@ -121,6 +123,32 @@ void f_12(void) {
 
 
 }
 }
 
 
+// https://bugs.llvm.org/show_bug.cgi?id=35419
+void f11_0(void) {
+  int x; // expected-note {{'x' declared without an initial value}}
+  x++; // expected-warning {{The expression is an uninitialized value. The computed value will also be garbage}}
+       // expected-note@-1 {{The expression is an uninitialized value. The computed value will also be garbage}}
+  clang_analyzer_warnIfReached(); // no-warning
+}
+void f11_1(void) {
+  int x; // expected-note {{'x' declared without an initial value}}
+  ++x; // expected-warning {{The expression is an uninitialized value. The computed value will also be garbage}}
+       // expected-note@-1 {{The expression is an uninitialized value. The computed value will also be garbage}}
+  clang_analyzer_warnIfReached(); // no-warning
+}
+void f11_2(void) {
+  int x; // expected-note {{'x' declared without an initial value}}
+  x--; // expected-warning {{The expression is an uninitialized value. The computed value will also be garbage}}
+       // expected-note@-1 {{The expression is an uninitialized value. The computed value will also be garbage}}
+  clang_analyzer_warnIfReached(); // no-warning
+}
+void f11_3(void) {
+  int x; // expected-note {{'x' declared without an initial value}}
+  --x; // expected-warning {{The expression is an uninitialized value. The computed value will also be garbage}}
+       // expected-note@-1 {{The expression is an uninitialized value. The computed value will also be garbage}}
+  clang_analyzer_warnIfReached(); // no-warning
+}
+
 int f_malloc_1(void) {
 int f_malloc_1(void) {
   int *ptr;
   int *ptr;
 
 

Some files were not shown because too many files changed in this diff