[cfe-dev] Check virtual calls in ctor or dtor
ioripolo at gmail.com
Tue Oct 25 23:50:35 PDT 2011
I re-implement this check as a static analyzer checker, and i make the
recursion a work list as you suggested.
Currently I'm not using dynamic programming but introduced a SmallPtrSet to
avoid analyzing the same methods again. It's a top-down analysis, how could
i introduce dynamic programming? By adjust the work list? It seems that if
we introduce DP we still need a set to record those method we have analyzed.
What you Say?
2011/10/4, Ted Kremenek <kremenek at apple.com>:
> Hi Lei,
> The part that scares me is the recursive walk of the callee's body.
> not likely to have good enough performance for a compile-time warning.
> a compile-time warning, I think it is fine to just check the body of the
> constructor, but if you want to analyze the bodies of called functions
> need to be extremely clever to do it efficiently enough for it to be a
> compile-time warning. We really like compiler warnings to have highly
> predictable performance, and recursively walking the entire classes's
> bodies is not really amendable to that.
> If you think walking through all the bodies is necessary, then I'd leave
> as a static analyzer checker. You'd certainly get better coverage. To
> handle the recursion, I'd probably make it a worklist (i.e., data
> to avoid stack blowup and to keep your memory footprint small. You should
> also employ dynamic programming to avoid analyzing the same methods again.
> The other nice thing about this approach is that it probably can be nicely
> generalized to global IPA once we have it.
> One caveat about doing a recursive analysis of method bodies without any
> path-based analysis is that you are introducing additional risk of a false
> positive. There could be unreachable paths where a constructor can't
> actually transitively trigger a call to a virtual function because of the
> context, but your check may report an issue. I don't think this will
> typically be a problem, but it is something to consider.
> On Sep 27, 2011, at 1:39 AM, 章磊 wrote:
>> Hi Ted,
>> My goal is to check whether there is a virtual call (directly or
>> indirectly) in construction or destruction.
>> In my former patch i do this check to all the constructors and the
>> destructor in checkASTDecl(const CXXRecordDecl ...). My way is walk
>> through the ast body of ctor/dtor: if there is virtual call, warn it; if
>> there is a call, walk through the callee's body recursively.
>> I think it's ok to do this check without the analysis engine and the
>> inline IPA.
>> What you say?
>> 在 2011年9月26日 下午9:37，Ted Kremenek <kremenek at apple.com>写道：
>> On Sep 25, 2011, at 2:32 AM, 章磊 wrote:
>>> Hi Ted,
>>> I tried to implement this as a compiler warning. But i had some
>>> problem in how to get the body of the ctor or dtor.
>>> I implemented this in Sema::CheckConstructor and
>>> Sema::CheckDestructor, but i can not get the body of them when there
>>> is no body with the declaration yet.
>> The idea how I thought this would be implemented was to do the analysis
>> when processing CallExprs as we are building them. We should know up
>> front when we are about to process a constructor body. Simply set/clear
>> Sema flag when processing the constructor body, and your check can
>> that flag when processing a CallExpr to see if it needs to do the check.
>> Alternatively, checking the DeclContext may be all that is needed to see
>> if we are currently in a constructor body.
>> The advantage of this approach is that it keeps all the checking local.
>> Walking the AST of a constructor body *after* we have constructed it is
>> slow, and not really a great approach for doing compiler warnings in the
>> frontend (if we can at all help it).
>>> And i also need the function body
>>> of the CallExpr in the ctor/dtor to do recursive analysis.
>> Interesting. If your goal is to do inter procedural analysis, then this
>> should remain a static analyzer check, or that should be a case caught
>> beyond what a compiler warning should do. That wasn't clear to me before
>> that this was something you wanted to do.
>> The tradeoffs between compiler and static analyzer warnings are fairly
>> 1) Compiler warnings have higher visibility because all users see them
>> the time when building their code.
>> 2) The logic behind compiler warnings must be as fast as possible.
>> Inter-procedural analysis is usually a show stopper, and ideally the
>> checks are highly local. Sema is already "walking" all of the code as it
>> builds the AST, so often the warning logic can be put in key places,
>> essentially where the warning would be issued.
>> Best regards!
>> Lei Zhang
-------------- next part --------------
An HTML attachment was scrubbed...
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 7545 bytes
Desc: not available
More information about the cfe-dev