[PATCH] D15448: [analyzer] SVal Visitor.

[Artem Dergachev via cfe-commits]

NoQ created this revision.
NoQ added reviewers: zaks.anna, dcoughlin, xazax.hun.
NoQ added a subscriber: cfe-commits.

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.

http://reviews.llvm.org/D15448

Files:
  docs/analyzer/DebugChecks.rst
  include/clang/StaticAnalyzer/Checkers/SValExplainer.h
  include/clang/StaticAnalyzer/Core/PathSensitive/SValVisitor.h
  lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
  test/Analysis/explain-svals.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D15448.42516.patch
Type: text/x-patch
Size: 30579 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20151211/426abb72/attachment-0001.bin>