It seems that in several places in the code Clang Static Analyzer tries to recursively traverse the SVal hierarchy, so i made a visitor for SVal, SymExpr, and MemRegion hierarchies. Actually, three separate visitors, but they're rarely useful on their own, so there's FullSValVisitor to merge the three for visiting the whole thing. The approach was literally copied from StmtVisitor etc in an obvious manner.
One thing that could make the visitor a lot more useful, which i'd probably love to implement, is a simple re-usable VisitChildren() method (in case the visitor's return type is void). Because we cannot write such method in every visitor as easily as we do it for, say, StmtVisitor (we don't have a full-featured iterator for child values/symbols/regions). This would allow a trivial implementation of methods like "find all ElementRegion's inside this SVal, and mark their indices"). To-think: how should such method handle lazy compound values?
This review is a bit green, in a sense that there's not much actually delivered yet, apart from the visitor header itself. Some further todo's would be:
- Refactor some pieces of the code to use the visitor. In fact, we already have the SymbolVisitor class somewhere. Probably SymbolReaper could be simplified.
- Some checkers, that rely on exploring the hierarchy, may be making use of the visitor. Even if existing checkers don't use it, developers of new checkers may like it.
- The object responsible for this alpha-renaming thing would most likely look like a FullSValVisitor that returns an SVal.
- Not sure, maybe split the three visitors into three different header files?
In order to make sure that the visitor header compiles, i started a simple example visitor - the SValExplainer. It explains symbolic values in a human-readable manner, returning an std::string. SValExplainer can be used:
- for pretty-printing values to the analyzer's end-user, eg. in checker warning messages, or even in "[assuming ...]" diagnostic pieces instead of pretty-printed expressions.
- for deep-testing analyzer internals, when the test needs to ensure that a particular kind of SVal is produced durign analysis. In fact, one of the tests a FIXME test that exposes a certain problem in the core.
- as a documentation for SVal kinds (because novice checker developers are often confused about the meaning of different SVal kinds). Users may also rely on it to understand how the analyzer works during debugging, eg. quickly explain what does this particular SVal they obtained in a certain callback actually means.
Todos for SValExplainer include:
- Explaining more values. In particular, i could use a bit of advice for Objective-C-specific values, because i know very little about this language. I might have also forgotten something. Memory spaces are worth it, most likely.
- Improving natural language. Probably some bugs would be exposed later. Not sure if the long "of"-chains the explainer produces sound naturally.
- Probably add various constructor-time flags if there are multiple users of the explainer having different expectations.
In order to test SValExplainer, a new callback was added to the debug.ExprInspectionChecker, namely clang_analyzer_explain(), that causes an explanation of its argument value to be printed as a warning message. I also added another callback - clang_analyzer_getExtent() in order to obtain SymbolExtent for testing. Testing how extents are modeled would probably be useful later as well. Regexps are used in the tests in order to match the start and the end of the warning message.
So, essentially, i'm humbly requesting a quick glance on this code, if this facility is useful, if some stuff is clearly useless, and whether any of the todos are actually wanted.
I'd probably make more updates in the process.