[PATCH] D62978: [analyzer] trackExpressionValue(): Handle unknown values better

[Csaba Dabis via Phabricator via cfe-commits]

Charusso marked 8 inline comments as done.
Charusso added inline comments.


================
Comment at: clang/test/Analysis/diagnostics/deref-track-symbolic-region.c:14
   int m = 0;
-  syz.x = foo(); // expected-note{{Value assigned to 'syz.x'}}
 
----------------
Here.


================
Comment at: clang/test/Analysis/diagnostics/find_last_store.c:9
 void no_find_last_store() {
-  c *e = d(); // expected-note{{Calling 'd'}}
-              // expected-note at -1{{Returning from 'd'}}
-              // expected-note at -2{{'e' initialized here}}
+  c *e = d(); // expected-note{{'e' initialized here}}
 
----------------
NoQ wrote:
> This remaining note is also unnecessary. You can safely stop tracking the value at `e || ...`. In particular, `ReturnVisitor` is not at fault.
> 
> That said, when we renamed `trackNullOrUndefValue` to `trackExpressionValue`, we kinda assumed that it's not really important whether the value is null/undef or not - the function simply tracks the value. This change would break this invariant, causing null values to be handled in a special manner. I recommend adding another flag argument (similar to `EnableNullFPSuppression`) that would allow the caller to tell whether it's interested in the null or in the "whole" value (defaulting to the latter).
That is a great idea! I tried my best but after a while I give up because it is already too complex, so I just removed them. It turns out it makes sense to remove those notes, see inline.


================
Comment at: clang/test/Analysis/diagnostics/macro-null-return-suppression.cpp:53
   int *x = RETURN_NULL();
-  x = returnFreshPointer();  // expected-note{{Value assigned to 'x'}}
   if (x) {} // expected-note{{Taking false branch}}
----------------
Here.


================
Comment at: clang/test/Analysis/loop-widening-notes.cpp:12
                    // expected-note at -1 {{Loop condition is false. Execution continues on line 16}}
-       ++i,        // expected-note {{Value assigned to 'p_a'}} 
        i < flag_a;
----------------
Here.


================
Comment at: clang/test/Analysis/uninit-const.cpp:59
   int p = f6_1_sub(t); //expected-warning {{Assigned value is garbage or undefined}}
-                       //expected-note at -1 {{Passing value via 1st parameter 'p'}}
-                       //expected-note at -2 {{Calling 'f6_1_sub'}}
----------------
Here we mismatch the parameter.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62978/new/

https://reviews.llvm.org/D62978