[cfe-dev] sizeof conditions and -Wunreachable-code

[David Blaikie]

[while the unreachable-code-on-templates analysis/discussion
continues, I thought I'd take the time to start on perhaps a less
contentious/more actionable related area. No rush, though - I'll leave
this here & maybe work on something for Lang to review in the mean
time]

One of the major remaining sources of false positives for
-Wunreachable-code in LLVM/Clang is use of sizeof, mostly in
BitVector.h:

if (sizeof(BitWord) == 4)
  ...
else if (sizeof(BitWord) == 8)
  ...

One of these cases will always produce an unreachable code warning
from Clang. That's not very helpful.

So in the interests of removing these false positives I've worked up a
first pass of modifications to the CFG (& related bits & pieces) to
work as follows:

1) Track any constant expression evaluation that depended on sizeof evaluation
2) Add support for an extra bit flag on CFG edges
3) Raise that flag for any edge that depends on a sizeof-dependent
expression (currently I'm only doing this for sizeof in a switch
expression - it was slightly less intrusive to plumb this through than
for the boolean conditions in if/while/etc)
4) Filter out these extra edges when iterating over successors and
predecessors normally
5) Include these edges when finding unreachable blocks
6) (exclude these edges when we're just doing reachability for things
like missing returns)

Ted - do you think this is at least on the right track? Did you have
some other ideas in mind about how this could be done? (for example -
I chose to put all these edges in the same list & filter at iteration
time because I assumed order of edges was important & this way order
of both edge traversals (with & without the extra edges) remains the
same as it was before - if order is not important we could simply
store the extra edges in a separate list & be slightly more efficient
at iteration rather than having to filter all the time)

Obviously it'll need a fair bit of massaging to get it to a more
usable state - I couldn't decide what to call the extra
flags/variables, some are named things like "MayVaryByBuild" (eg: this
edge may exist or not exist depending on the arch of the build) or
"Optimistic" (be optimistic about reachability - finding more things
that can be reached than is strictly true for this build (by
considering MayVaryByBuild edges)). Perhaps there are some more
canonical terms that should be applied?

Should this be generalized to handle many more flags for different
kinds of graph edges? I know you mentioned previously that there's
already one CFG parameter about whether certain edges are included
("AllAlwaysAdd"). Are there clients that would benefit from being able
to build both those graphs in one pass & filter on iteration instead
of requesting multiple builds of the CFG? (it doesn't look like
AnalysisBasedWarnings needs to build two CFGs just because it
sometimes needs the "AllAlwaysAdd" flag - so I guess both kinds of
clients can use the more expensive CFG, so this question is probably
irrelevant). But if extra flags are needed we could generalize this
better with some kind of flag mask for construction and then a flag
mask & value to filter on when iterating over edges (or an entire
filtered view of the CFG?).

A couple of asides:

* I found a bug in the FilteredCFGBlockIterator - it doesn't filter on
the first element (fix is included/necessary for my patch).
* FilteredCFGBlockIterator could probably be refactored out to a more
general device like a filtering iterator (
http://blogs.msdn.com/b/vcblog/archive/2010/08/09/the-filterator.aspx
) & just provide the (possibly stateful) predicate. This could be
reused for specific_*_iterators in Clang, too)
* Oh - another thing: why are there edges to null in the CFG? Do they
serve a purpose?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: unreachable_sizeof.diff
Type: text/x-patch
Size: 24010 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20120221/0f76a770/attachment.bin>