george.karpenkov added a comment.

Checking for asserts makes sense, but as a rough estimate will suppress roughly 
zero false positives I have seen.
In general, LLVM codebase is exceptional in its use of asserts, and most 
projects, unfortunately, don't really know how to use them.



================
Comment at: 
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp:519
+
+    if (FirstAccess->getBeginLoc() < FirstGuard->getBeginLoc())
+      return true;
----------------
Szelethus wrote:
> xazax.hun wrote:
> > I am not sure if this is a reliable way to check if the access is before 
> > the guard.
> > 
> > Consider:
> > ```
> > switch(x): {
> >    case 2: guard; access; break;
> >    case 1: access break;
> > }
> > ```
> > 
> > Here, we have no particular ordering between the access in case 1 and the 
> > guard in case 2 at runtime. But relying on the source locations we might 
> > come to the false conclusion that there is. Loops, gotos can cause similar 
> > problems.
> > I do understand that this might not be too easy to solve without traversing 
> > the cfg and we might not want to do that but I think we should at least add 
> > a test/todo. 
> > I am not sure if this is a reliable way to check if the access is before 
> > the guard.
> I'm 100% sure it isn't. Using the CFG instead of matchers sounds like a great 
> and difficult to implement (at least to me, as I never touched them) idea. It 
> should get rid of the false negatives, at least in part.
> > [...]  I think we should at least add a test/todo.
> There are some :)
> Using the CFG instead of matchers sounds like a great and difficult to 
> implement

That would also require building a dataflow framework, which we do not have 
(yet)


https://reviews.llvm.org/D51866



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to