[PATCH] D144269: [Analyzer] Show "taint originated here" note of alpha.security.taint.TaintPropagation checker at the correct place

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

I completely agree with @steakhal, these should be note tags:

- The "visitor way" is to reverse-engineer the exploded graph after the fact.
- The "slightly more sophisticated visitor way" is have checker callbacks leave extra hints in the graph to assist reverse engineering, which is what you appear to be trying to do.
- The "note tag" way is to simply capture that information from inside checker callbacks in the form of lambda captures. It eliminates the need to think about how to store the information in the state (it's stored in the program point instead), or how to structure it.

I also completely agree with @steakhal that the intermediate notes are valuable. In the motivating example, ideally both `strtol` and `getenv` need a note ("taint propagated here" and "taint originated here" respectively).

The challenging part with note tags is how do you figure out whether your bug report is taint-related. The traditional solution is to check the `BugType` but in this case an indeterminate amount of checkers may emit taint-related reports. I think now's a good time to include a "generic data map"-like data structure in `PathSensitiveBugReport` objects, so that checkers could put some data there during `emitReport()`, which can be picked up by note tags and potentially mutated in the process. For example, you can introduce a set of tracked tainted symbols there, which will be pre-populated by the checker with the final tainted symbol, then every time a note tag discovers that a symbol in the set becomes a target of taint propagation, it removes the symbol from the set and replaces it with the symbols from which its taint originated, so that later note tags would react on these new symbols instead.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144269/new/

https://reviews.llvm.org/D144269