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

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

I haven't checked the implementation, but fundamentally patching the TaintBugVisitor is not how we should improve the diagnostic for taint issues.
I saw that this patch is not about NoteTags, so I didn't go any further that point.

What we should do instead, to add a fancy NoteTags to each of the Post transitions to propagate interestingness to the taint sources.
Where each NoteTag does:

- checks if any of the taint destinations are actually 'interesting', if none then just return an empty note.
- take the taint source arguments and mark their pre-call values as interesting
- construct a descriptive message explaining what happened:
  - If the transition had no taint sources, then it must be a "taint source"
  - If we had tainted sources, tell the user that X', Y', and Z' arguments were tainted, hence we propagated taint
  - take all the "interesting" taint destinations and tell the user that X, Y and Z arguments become tainted due to the propagation rule.

I'm attaching my proposed version for improving the diagnostics where I demonstrate all what I said. F26595921: proposed.patch <https://reviews.llvm.org/F26595921>
Note that my patch is really crude, and I just finished hacking it to get all tests pass in a couple hours.

Let me know if it would be a good way to refine your patch or I should review your current implementation.


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

https://reviews.llvm.org/D144269