[PATCH] D51057: [analyzer][UninitializedObjectChecker] Fixed dereferencing

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.


================
Comment at: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp:126-127
   if (V.isUndef()) {
+    assert(!FR->getDecl()->getType()->isReferenceType() &&
+           "References must be initialized!");
     return addFieldToUninits(
----------------
Szelethus wrote:
> NoQ wrote:
> > Good catch.
> > 
> > It might still be possible to initialize a reference with an already-undefined pointer if core checkers are turned off, but we don't support turning them off, so i guess it's fine.
> I removed it, because it did crash couple times on LLVM. Note that the assert checked whether the reference for undefined, not uninitialized :/.
> 
> It's no longer in the code, but this was it:
> ```
> assert(!FR->getDecl()->getType()->isReferenceType() &&
>        "References must be initialized!");
> ```
Hmm, indeed, it seems that we don't have a checker for garbage references, only for null references(?).

I guess it's a good idea for a checker.


================
Comment at: test/Analysis/cxx-uninitialized-object.cpp:879-902
+struct LambdaWrapper {
+  void *func; // no-crash
+  int dontGetFilteredByNonPedanticMode = 0;
+
+  LambdaWrapper(void *ptr) : func(ptr) {} // expected-warning{{1 uninitialized field}}
+};
+
----------------
Szelethus wrote:
> I'm 99% sure this is a FP, but it doesn't originate from the checker. Shouldn't `*ptr` be undef after the end of the code block as `lambda`'s lifetime ends?
> 
> Nevertheless, it did cause a crash, so here's a quick fix for it.
I'm pretty sure that all sorts of contents of `lambda` aka `*ptr` are undefined once it goes out of scope. Moreover, `ptr` is now a dangling pointer, and reading from it would cause undefined behavior. I'm not sure if the analyzer actually models this though. 

But on the other hand, even if it didn't go out of scope, i don't really see where field `a` was initialized here.

Soo what makes you think it's a false positive?


https://reviews.llvm.org/D51057