[PATCH] D48436: [analyzer][UninitializedObjectChecker] Fixed a false negative by no longer filtering out certain constructor calls

Artem Dergachev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jul 30 17:03:28 PDT 2018

NoQ added inline comments.

Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:681-689
+    Optional<nonloc::LazyCompoundVal> OtherObject =
+        getObjectVal(OtherCtor, Context);
+    if (!OtherObject)
+      continue;
+    // If the CurrentObject is a field of OtherObject, it will be analyzed
+    // during the analysis of OtherObject.
Szelethus wrote:
> NoQ wrote:
> > Szelethus wrote:
> > > NoQ wrote:
> > > > It seems safer to look at `CXXConstructExpr::getConstructionKind()`.
> > > > 
> > > > Taking a `LazyCompoundVal` and converting it back to a region is definitely a bad idea because the region within `LazyCompoundVal` is completely immaterial and carries no meaning for the value represented by this `SVal`; it's not necessarily the region you're looking for.
> > > Looking at the singleton test case, I think I need to get that region somehow, and I'm not too sure what you meant under using `CXXConstructExpr::getConstructionKind()`. One thing I could do, is similar to how `getObjectVal` works:
> > > ```
> > >   Loc Tmp = Context.getSValBuilder().getCXXThis(OtherCtor->getParent(),
> > >                                                 Context.getStackFrame());
> > > 
> > >   auto OtherThis = Context.getState()->getSVal(Tmp).castAs<loc::MemRegionVal>();
> > > 
> > >   OtherThis.getRegion(); // Hooray!
> > > ```
> > > 
> > > I have tested it, and it works wonders. Is this a "safe-to-use" region?
> > You can directly ask `CXXConstructExpr` if it's a field or a base constructor. I could describe `getConstructionKind()` as some sort of very limited `ConstructionContext` that's available in the AST: it explains to you what sort of object is being constructed. I suspect it's enough for your purposes.
> I think I'm aware of how it works, but my issue is that it's very limited:
> ```
> enum ConstructionKind { CK_Complete, CK_NonVirtualBase, CK_VirtualBase, CK_Delegating }
> ```
> Sadly, whether the `CXXConstructExpr` was a field construction or a "top level" (non-field) construction can't be retrieved from `getConstructionKind()`, as I understand it.
> If I were to return true (say that the constructed object will be analyzed later, thus ignoring it for now) if `getConstructionKind() == CK_Complete`, the singleton testcase will fail (I have tested it). I'm very confident that I need to obtain the constructed object's `MemRegion`.
Whoops, yeah, i was wrong. I mis-remembered that there's a `CK_` for field.

Then, yeah, the code looks good, let me recall where we were in terms of why do we need it><


More information about the cfe-commits mailing list