No, sorry. It's not that it's particularly destabilizing so much as it's just well past the deadline for features and enhancements.
Jordan On Dec 16, 2013, at 13:07 , Gabor Kozar <[email protected]> wrote: > Is there any chance of this making it into 3.4? If so, I can work (and think) > on it tomorrow, otherwise it can wait, obviously. > > (Sorry for the short answer, had a long day.) > > -- > Gábor 'ShdNx' Kozár > [email protected] > > > > On Mon, Dec 16, 2013, at 18:50, Jordan Rose wrote: >> This does look fairly straightforward, and fairly good! But I'm concerned >> about the FunctionDecl parameter: the analyzer also allows >> ObjCMethodDecls and BlockDecls to be analyzed as top-level entry points. >> Perhaps a PointerUnion3 would be the best thing to do here, at least for >> now? Or alternately, we allow people to specify which initial state >> they're checking, either by having several callbacks or using a template >> parameter. What do you think? >> >> Also, one nice thing about checkInitialState is that it can be non-const, >> since it is guaranteed to run 0 or 1 times before the checker is used >> elsewhere. :-) >> >> >> - Handling NULL ProgramStateRef-s. I do not think it is reasonable to have >> NULL as the initial state. Probably assert when a checkInitialState call >> returns NULL? >> >> >> I think a NULL initial state means a checker has decided not to analyze >> this function. Weird and unlikely, but possible. (For example, it allows >> us to move the -analyze-function option into a checker.) >> >> - Whether to call the checkInitialState callback in >> ExprEngine::getInitialState, or where getInitialState is used. I found one >> such method: CoreEngine::ExecuteWorkList. Logically - knowing nothing of the >> codebase -, I think it makes sense to do this in getInitialState. It does >> not appear to belong to CoreEngine, especially since I cannot find any >> references to getCheckerManager() in CoreEngine. >> >> >> I agree. Can you explain why you needed to move the getInitialState call >> earlier? It seems dangerous to run checkers before we have a block >> counter in place, since conjured symbols use the block count. >> >> Do we have any checkers that can use this? Perhaps as proof-of-concept >> you can move the check for main's argc and argv into a checker. >> >> >> Once checkInitialState is working, I also plan to do a void >> checkBeginFunction(const FunctionDecl*, CheckerContext&) const, i.e. a >> callback that is called whenever a function is entered, either through a >> function call (this would be triggered right after checkPreCall) or when it >> is the entry point of the analysis. >> >> >> I'm not sold on this one yet. I mean, I get the idea, but it again seems >> like something where we want a nicer interface than just "FunctionDecl" >> (and at the very least need to handle ObjCMethodDecl as well). But we can >> discuss this later. >> >> I think you'd implement this by calling checkBeginFunction right after >> checkInitialState and also from ExprEngine::inlineCall (right after >> "State->enterStackFrame(...)") >> >> >> Finally, style notes (guessing this comes from bouncing between >> projects): >> >> + if(!State) return NULL; // exit early in case of unfeasible state >> >> >> Space after if, consequent statement on a separate line, comments also on >> a separate line (and in the form of a complete sentence, with >> capitalization and a period). >> >> Thank you for putting this together! >> Jordan >> >> >> On Dec 13, 2013, at 17:03 , Gabor Kozar <[email protected]> wrote: >> >> Hi Jordan, >> >> I had an issue recently with the Static Analyzer that I wouldn't be able to >> detect when a function was entered - either through a call or serving as the >> entry point of the analysis. (I ended up using checkPreStmt and figuring out >> if we've been magically transported inside a function body and check the >> program state whether the necessary info was already recorded by >> checkPreCall. Not something I'd call nice by any means.) >> >> So I wrote to cfg-dev and you mentioned that you had no such callbacks >> currently implemented, although they have been needed before, and that >> patches were welcome. Since this is a fairly trivial project, I was >> comfortable with doing it in my very limited free time (especially because I >> still intend to work on the Static Analyzer open projects at some point when >> I'm able). >> >> Find the diff file attached. Since you've pointed me to >> ExprEngine::getInitialState, I just modified it to call any checkers >> registered for check::InitialState at the end. So the signature for the >> callback is: >> ProgramStateRef checkInitialState(const FunctionDecl*, ProgramStateRef) const >> >> There are a few things I'm not sure about: >> - Handling NULL ProgramStateRef-s. I do not think it is reasonable to have >> NULL as the initial state. Probably assert when a checkInitialState call >> returns NULL? >> - Whether to call the checkInitialState callback in >> ExprEngine::getInitialState, or where getInitialState is used. I found one >> such method: CoreEngine::ExecuteWorkList. Logically - knowing nothing of the >> codebase -, I think it makes sense to do this in getInitialState. It does >> not appear to belong to CoreEngine, especially since I cannot find any >> references to getCheckerManager() in CoreEngine. >> >> Please let me know what you think! >> >> Once checkInitialState is working, I also plan to do a void >> checkBeginFunction(const FunctionDecl*, CheckerContext&) const, i.e. a >> callback that is called whenever a function is entered, either through a >> function call (this would be triggered right after checkPreCall) or when it >> is the entry point of the analysis. >> >> I'm not sure about where can I notify the checkers of this though. When is >> the point when we can say "okay so let's start analyzing this function"? If >> you could point me to the method, that would help a lot. (Ironically, we >> also have a code comprehension project running where I work [at Ericsson], >> but I do not have access to server where we have the Clang trunk parsed, >> since I'm doing this from home in my free time.) >> >> Thanks! >> >> -- >> Gábor 'ShdNx' Kozár >> [email protected] >> >> >> Email had 1 attachment: >> >> patch-checkInitialState.diff >> 6k (text/x-patch)
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
