[analyzer] Track the right hand side of the last store regardless of its value

[Szelethus]

Summary

The following code snippet taken from D64271#1572188 has an issue: namely, because flag's value isn't undef or a concrete int, it isn't being tracked.

int flag;
bool coin();

void foo() {
  flag = coin();
}

void test() {
  int *x = 0;
  int local_flag;
  flag = 1;

  foo();
  local_flag = flag;
  if (local_flag)
    x = new int;

  foo();
  local_flag = flag;
  if (local_flag)
    *x = 5;
}

This, in my opinion, makes no sense, other values may be interesting too. Originally added by rC185608.

[NoQ]

Whoa, the new notes are amazing. Looks like you've found a pretty bad omission.

clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp

1436–1437

You're removing this visitor, does it cause any removal of notes?

1989–1991

Fair enough. Did we already have a test case for this? I wonder if function pointers might also cause problems here.

Ugh, this code is so underdebugged.

[Szelethus]

Would you say it's good to go? :)

clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp

1436–1437

Nope, trackExpressionValue will add it back, if appropriate.

1989–1991

Yup, test/Analysis/exercise-ps.c flopped. And I agree, this looks kinda nasty.

[NoQ]

Yeah, i think this is good to go :)

[xazax.hun]

LGTM!

Since we allow new kinds of SVals to be tracked it would be great to test this first on a larger corpus of projects just to see if there is a crash (due to an unhandled SVal type).

[Szelethus]

In D64287#1589941, @xazax.hun wrote:

LGTM!

Since we allow new kinds of SVals to be tracked it would be great to test this first on a larger corpus of projects just to see if there is a crash (due to an unhandled SVal type).

It doesn't! Though that analysis was run with plenty of other patches as well, but I saw no crashes regardless.

clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp

FindLastStoreBRVisitor::VisitNode(const ExplodedNode *Succ,
 
   if (!StoreSite)
     return nullptr;
   Satisfied = true;
 
   // If we have an expression that provided the value, try to track where it
   // came from.
   if (InitE) {
-    if (V.isUndef() ||
-        V.getAs<loc::ConcreteInt>() || V.getAs<nonloc::ConcreteInt>()) {
-      if (!IsParam)
-        InitE = InitE->IgnoreParenCasts();
-      bugreporter::trackExpressionValue(StoreSite, InitE, BR,
+
                                       EnableNullFPSuppression);
-    }
-    ReturnVisitor::addVisitorIfNecessary(StoreSite, InitE->IgnoreParenCasts(),
-                                         BR, EnableNullFPSuppression);
-  }
 
   // Okay, we've found the binding. Emit an appropriate message.
   SmallString<256> sbuf;
   llvm::raw_svector_ostream os(sbuf);
 
   if (Optional<PostStmt> PS = StoreSite->getLocationAs<PostStmt>()) {
     const Stmt *S = PS->getStmt();
     const char *action = nullptr;
static bool trackExpressionValue(
   if (auto L = V.getAs<loc::MemRegionVal>()) {
     report.addVisitor(llvm::make_unique<UndefOrNullArgVisitor>(L->getRegion()));
 
     // FIXME: this is a hack for fixing a later crash when attempting to
     // dereference a void* pointer.
     // We should not try to dereference pointers at all when we don't care
     // what is written inside the pointer.
     bool CanDereference = true;
-    if (const auto *SR = dyn_cast<SymbolicRegion>(L->getRegion()))
+    if (const auto *SR = L->getRegionAs<SymbolicRegion>()) {
       if (SR->getSymbol()->getType()->getPointeeType()->isVoidType())
         CanDereference = false;
+    }
+    else if (const auto *AR = L->getRegionAs<AllocaRegion>())
+      CanDereference = false;
 
     // At this point we are dealing with the region's LValue.
     // However, if the rvalue is a symbolic region, we should track it as well.
     // Try to use the correct type when looking up the value.
     SVal RVal;
-    if (ExplodedGraph::isInterestingLValueExpr(Inner)) {
+    if (ExplodedGraph::isInterestingLValueExpr(Inner))
       RVal = LVState->getRawSVal(L.getValue(), Inner->getType());
-    } else if (CanDereference) {
+    else if (CanDereference)
       RVal = LVState->getSVal(L->getRegion());
-    }
 
     if (CanDereference)
       if (auto KV = RVal.getAs<KnownSVal>())
         report.addVisitor(llvm::make_unique<FindLastStoreBRVisitor>(
             *KV, L->getRegion(), EnableNullFPSuppression, TKind));
 
     const MemRegion *RegionRVal = RVal.getAsRegion();
     if (RegionRVal && isa<SymbolicRegion>(RegionRVal)) {

clang/test/Analysis/track-control-dependency-conditions.cpp

void f(int y) {
   if (flag) // expected-note{{'flag' is 1}}
             // expected-note@-1{{Taking true branch}}
             // debug-note@-2{{Tracking condition 'flag'}}
     *x = 5; // expected-warning{{Dereference of null pointer}}
             // expected-note@-1{{Dereference of null pointer}}
 }
 } // end of namespace tracked_condition_written_in_nested_stackframe
 
+namespace condition_written_in_nested_stackframe_before_assignment {
+int flag = 0;
+int getInt();
+
+void foo() {
+  flag = getInt(); // tracking-note{{Value assigned to 'flag'}}
+}
+
+void f() {
+  int *x = 0; // expected-note{{'x' initialized to a null pointer value}}
+  int y = 0;
+
+  foo(); // tracking-note{{Calling 'foo'}}
+         // tracking-note@-1{{Returning from 'foo'}}
+  y = flag; // tracking-note{{Value assigned to 'y'}}
+
+  if (y) // expected-note{{Assuming 'y' is not equal to 0}}
+         // expected-note@-1{{Taking true branch}}
+         // debug-note@-2{{Tracking condition 'y'}}
+    *x = 5; // expected-warning{{Dereference of null pointer}}
+            // expected-note@-1{{Dereference of null pointer}}
+}
+} // end of namespace condition_written_in_nested_stackframe_before_assignment
+
 namespace collapse_point_not_in_condition {
 
 [[noreturn]] void halt();
 
 void assert(int b) {
   if (!b) // tracking-note{{Assuming 'b' is not equal to 0}}
           // tracking-note@-1{{Taking false branch}}
     halt();

clang/test/Analysis/uninit-const.c

 void doStuff_constPointerToConstInt              (int const * const u){};
 void doStuff_constPointerToConstPointerToConstInt(int const * const * const u){};
 void doStuff_pointerToConstPointerToConstInt(int const * const * u){};
 void doStuff_pointerToPointerToConstInt       (int const **u){};
 void doStuff_constStaticSizedArray(const int a[static 10]) {}
 void doStuff_variadic(const int *u, ...){};
 
 void f_1(void) {
-  int t;
+  int t; // expected-note {{'t' declared without an initial value}}
   int* tp = &t;        // expected-note {{'tp' initialized here}}
   doStuff_pointerToConstInt(tp);  // expected-warning {{1st function call argument is a pointer to uninitialized value}}
                        // expected-note@-1 {{1st function call argument is a pointer to uninitialized value}}
 }
 
 void f_1_1(void) {
-  int t;
-  int* tp1 = &t;
+  int t; // expected-note {{'t' declared without an initial value}}
+  int* tp1 = &t; // expected-note {{'tp1' initialized here}}
   int* tp2 = tp1;        // expected-note {{'tp2' initialized here}}
   doStuff_pointerToConstInt(tp2);  // expected-warning {{1st function call argument is a pointer to uninitialized value}}
                        // expected-note@-1 {{1st function call argument is a pointer to uninitialized value}}
 }
 
 
 int *f_2_sub(int *p) {
-  return p;
+  return p; // expected-note {{Returning pointer (loaded from 'p')}}
 }
 
 void f_2(void) {
-  int t;
-  int* p = f_2_sub(&t);
+  int t; // expected-note {{'t' declared without an initial value}}
+  int* p = f_2_sub(&t); // expected-note {{Passing value via 1st parameter 'p'}}
+                        // expected-note@-1{{Calling 'f_2_sub'}}
+                        // expected-note@-2{{Returning from 'f_2_sub'}}
+                        // expected-note@-3{{'p' initialized here}}
   int* tp = p; // expected-note {{'tp' initialized here}}
   doStuff_pointerToConstInt(tp); // expected-warning {{1st function call argument is a pointer to uninitialized value}}
                       // expected-note@-1 {{1st function call argument is a pointer to uninitialized value}}
 }
 
 int z;
 void f_3(void) {
       doStuff_pointerToConstInt(&z);  // no warning
 }
 
 void f_4(void) {
       int x=5;
       doStuff_pointerToConstInt(&x);  // no warning
 }
 
 void f_5(void) {
-  int ta[5];
+  int ta[5]; // expected-note {{'ta' initialized here}}
   int* tp = ta;        // expected-note {{'tp' initialized here}}
   doStuff_pointerToConstInt(tp);  // expected-warning {{1st function call argument is a pointer to uninitialized value}}
                        // expected-note@-1 {{1st function call argument is a pointer to uninitialized value}}
 }
 
 void f_5_1(void) {
   int ta[5];        // expected-note {{'ta' initialized here}}
   doStuff_pointerToConstInt(ta);  // expected-warning {{1st function call argument is a pointer to uninitialized value}}

 
 void f_8(void) {
       int g;       // expected-note {{'g' declared without an initial value}}
       doStuff2(g); // expected-warning {{1st function call argument is an uninitialized value}}
                    // expected-note@-1 {{1st function call argument is an uninitialized value}}
 }
 
 void f_9(void) {
-  int  a[6];
+  int  a[6]; // expected-note {{'a' initialized here}}
   int const *ptau = a;             // expected-note {{'ptau' initialized here}}
   doStuff_arrayOfConstInt(ptau);    // expected-warning {{1st function call argument is a pointer to uninitialized value}}
                                    // expected-note@-1 {{1st function call argument is a pointer to uninitialized value}}
 }
 
 void f_10(void) {
   int  a[6];                     // expected-note {{'a' initialized here}}
   doStuff_arrayOfConstInt(a);    // expected-warning {{1st function call argument is a pointer to uninitialized value}}
int f_malloc_2(void) {
 
   doStuff_pointerToConstInt(ptr); // no warning
   free(ptr);
   return 0;
 }
 
 // uninit pointer, uninit val
 void f_variadic_unp_unv(void) {
-  int t;
+  int t; // expected-note {{'t' declared without an initial value}}
   int v;
   int* tp = &t;           // expected-note {{'tp' initialized here}}
   doStuff_variadic(tp,v);  // expected-warning {{1st function call argument is a pointer to uninitialized value}}
                           // expected-note@-1 {{1st function call argument is a pointer to uninitialized value}}
 }
 // uninit pointer, init val
 void f_variadic_unp_inv(void) {
-  int t;
+  int t; // expected-note {{'t' declared without an initial value}}
   int v = 3;
   int* tp = &t;           // expected-note {{'tp' initialized here}}
   doStuff_variadic(tp,v);  // expected-warning {{1st function call argument is a pointer to uninitialized value}}
                           // expected-note@-1 {{1st function call argument is a pointer to uninitialized value}}
 }
 
 // init pointer, uninit val
 void f_variadic_inp_unv(void) {
void f_variadic_inp_inp(void) {
   int u=3;
   int *vp = &u ;
   int *tp = &t;
   doStuff_variadic(tp,vp); // no-warning
 }
 
 //uninit pointer, init pointer
 void f_variadic_unp_inp(void) {
-  int t;
+  int t; // expected-note {{'t' declared without an initial value}}
   int u=3;
   int *vp = &u ;
   int *tp = &t;             // expected-note {{'tp' initialized here}}
   doStuff_variadic(tp,vp); // expected-warning {{1st function call argument is a pointer to uninitialized value}}
                             // expected-note@-1 {{1st function call argument is a pointer to uninitialized value}}
 }
 
 //init pointer, uninit pointer
 void f_variadic_inp_unp(void) {
   int t=5;
   int u;
   int *vp = &u ;
   int *tp = &t;
   doStuff_variadic(tp,vp); // no-warning
 }
 
 //uninit pointer, uninit pointer
 void f_variadic_unp_unp(void) {
-  int t;
+  int t; // expected-note {{'t' declared without an initial value}}
   int u;
   int *vp = &u ;
   int *tp = &t;             // expected-note {{'tp' initialized here}}
   doStuff_variadic(tp,vp); // expected-warning {{1st function call argument is a pointer to uninitialized value}}
                             // expected-note@-1 {{1st function call argument is a pointer to uninitialized value}}
 }

clang/test/Analysis/uninit-const.cpp

int p = f6_1_sub(t); //expected-warning {{Assigned value is garbage or undefined}}
                        //expected-note@-3 {{Returning from 'f6_1_sub'}}
                        //expected-note@-4 {{Assigned value is garbage or undefined}}
   int q = p;
   doStuff6(q);
 }
 
 void f6_2(void) {
   int t;       //expected-note {{'t' declared without an initial value}}
-  int &p = t;
-  int &s = p;
+  int &p = t;  //expected-note {{'p' initialized here}}
+  int &s = p;  //expected-note {{'s' initialized here}}
   int &q = s;  //expected-note {{'q' initialized here}}
   doStuff6(q); //expected-warning {{1st function call argument is an uninitialized value}}
                //expected-note@-1 {{1st function call argument is an uninitialized value}}
 }
 
 void doStuff6_3(int& q_, int *ptr_) {}
 
 void f6_3(void) {
void f6(void) {
   doStuff6(k); // expected-warning {{1st function call argument is an uninitialized value}}
                // expected-note@-1 {{1st function call argument is an uninitialized value}}
 
 }
 
 
 
 void f5(void) {
-  int t;
+  int t; // expected-note {{'t' declared without an initial value}}
   int* tp = &t;        // expected-note {{'tp' initialized here}}
   doStuff_uninit(tp);  // expected-warning {{1st function call argument is a pointer to uninitialized value}}
                        // expected-note@-1 {{1st function call argument is a pointer to uninitialized value}}
 }
 
 
 void f4(void) {
       int y;        // expected-note {{'y' declared without an initial value}}