On Apr 30, 2014, at 3:54 , Anders Rönnholm <anders.ronnh...@evidente.se> wrote:
> < Clearing the map at the end of a function is a start, but you ought to make
> sure you don't clear it for the function that called this one. I suppose
> that's a refinement that can be made later, since it only produces false
> negatives. (I'm not super happy about not being able to merge branches within
> the <function, though.)
>
> No I don't like it either and would like to fix it but I haven't figured out
> how. Can I find out the current function somehow when the division occurs?
I would just put the current StackFrameContext in as part of the key for the
map, or make a two-tiered map. Then when you hit the end of the function, you
can just drop things that match the current StackFrameContext.
>
> < +unsigned DivZeroChecker2::getBlockID(const Expr *B, CheckerContext &C)
> const {
> < + const CFG &cfg = C.getPredecessor()->getCFG();
> < + for (CFG::const_iterator I = cfg.begin(); I != cfg.end(); ++I) {
> < + for (CFGBlock::const_iterator BI = (*I)->begin(), BE = (*I)->end();
> < + BI != BE; ++BI) {
> < + Optional<CFGStmt> stmt = BI->getAs<CFGStmt>();
> < + if (stmt && stmt->getStmt() == B)
> < + return (*I)->getBlockID();
> < + }
> < + }
> < + return 0;
> < +}
>
> < This is much too expensive. The CheckerContext has a NodeBuilder, which has
> a NodeBuilderContext, which has a CFGBlock. We should just expose that
> somewhere (probably on CheckerContext).
>
> I have made a get function in CheckerContext to retrieve the NodeBuilder. Is
> there something better than blockid? Scope id would have been nice. Blockid
> creates false negatives like below when an if-statement that has nothing to
> do with the division comes in between.
>
> void err_not(int x, int y) {
> var = 77 / x;
>
> if (y)
> int v = 0;
>
> if (!x){}// should warn here but don't since this is a new blockid.
> }
We don't currently track scopes, but that still wouldn't eliminate all the
false positives. I guess it would be closer, though—some day if we start
tracking scopes again, we can switch to that.
I don't think it makes sense to expose the whole NodeBuilder through the
CheckerContext, just the block ID.
> < Finally, please include some test cases with inlining so that the
> clear-at-end-of-function logic is exercised.
>
> What's the difference with inline and normal functions in this case? As I
> have understood it inline functions are still treated like normal functions
> when walking the ast.
When doing path-sensitive checks, the analyzer can choose to inline any
function call that it can see the definition for. I want to make sure that we
still detect test-after-uses when the test and use are both within the inlined
function and when there is an inlined function call between the test and use in
the top-most function. (The latter would hit the first issue I noted, that
clearing the entire state at the end of a function is a bit too destructive.)
Jordan_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
