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

[David Blaikie]

Ping.

On Tue, Feb 21, 2012 at 9:55 PM, David Blaikie <dblaikie at gmail.com> wrote:
> [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: application/octet-stream
Size: 24021 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20120309/1e5bb869/attachment.obj>