[PATCH] D59555: [analyzer] Add yaml parser to GenericTaintChecker

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

This new approach is clearly useful to other checkers as well, not only the Taint checker. I believe we should strongly consider generalizing it somehow, it's just too awesome to restrict to a single checker.

There's also a closely related technology called "API Notes" that allows you to inject annotations into system headers by shipping .yaml files with the compiler. It was actively pushed for for the Analyzer in particular but ended up never getting upstreamed:

- http://lists.llvm.org/pipermail/cfe-dev/2015-December/046335.html
- http://lists.llvm.org/pipermail/cfe-dev/2017-May/053860.html

I suspect that it'll make perfect sense for your use case to define a clang attribute for taint sources and sinks and then define an API notes definition that'll inject this attribute into headers. If only API notes were available, this would have been a perfect use case for them. I think it might be a good idea to ping the mailing lists about API notes one more time, announce that you *are* going for a similar technology anyway, and see if anybody suggests something.

Pros/cons:

- The attributes/API notes solution is very comfy because it both allows users to address their false positives / false negatives by adding annotations in their source *and* allows us to annotate system headers via yaml files, and (if i understand correctly) both sorts of annotations are accessible uniformly via `Decl->getAttr<...>()`.
  - If we decide to go for our own yaml format that doesn't work on top of attributes, we'll either get only one of these, or double up our implementation burden in checkers.
    - Implementation burden should not be that high, but it will be annoying.
  - If we don't want users to annotate their headers with source-level annotations, it sounds easier to go for a custom yaml parser, because defining attributes is annoying.
    - But i believe we do want them to in this case.

Does any of this make any sense within the bigger plans that you have for this checker?


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

https://reviews.llvm.org/D59555