[PATCH] D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call

Umann Kristóf via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Apr 12 10:11:21 PDT 2018


Szelethus added inline comments.


================
Comment at: lib/StaticAnalyzer/Checkers/CtorUninitializedMemberChecker.cpp:187
+
+  if (isCalledByConstructor(Context))
+    return;
----------------
xazax.hun wrote:
> Szelethus wrote:
> > whisperity wrote:
> > > I think somewhere there should be a bit of reasoning why exactly these cases are ignored by the checker.
> > At this function's declaration I have some comments describing what this does and why it does it. Did you find it insufficient?
> I am a bit surprised why these errors are not deduplicated by the analyzer. Maybe it would worth some investigation?
Copied from `BugReport`s constructor documentation  for uniqueing:
>The reports that have the same report location, description, bug type, and ranges are uniqued - only one of the equivalent reports will be presented to the user. [...]
For this code snippet:
```
struct A {
   struct B {
      int x, y;
      
      B() {}
   } b;
   int w;

   A() {
      b = B();
   }
};
```
the warning message after a call to `A::A()` would be "3 uninitialized fields[...]", and for `B::B()` inside `A`s constructor would be "2 uninitialized fields[...]", so it wouldn't be filtered out.


================
Comment at: lib/StaticAnalyzer/Checkers/CtorUninitializedMemberChecker.cpp:292
+      if (LocalChain)
+        LocalChain->push_back(FR);
+      if (isNonUnionUninit(FR, LocalChain))
----------------
xazax.hun wrote:
> Don't you need to pop somewhere?
Nice catch! This isn't covered by tests, and would most probably cause an incorrect note message. I'll be sure to fix it.


https://reviews.llvm.org/D45532





More information about the cfe-commits mailing list