[PATCH] D42645: New simple Checker for mmap calls

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

> What I propose is "two" separated checkers one using same but the value of PROT_EXEC will depend on the checker name e.g. security.MmapWriteExecGen vs security.MmapWriteExecGlibc ... or by default we keep PROT_EXEC as is and we would allow to override the value via analyser options. I would go with the second option.

This doesn't sound great; most of the users wouldn't ever go that far or read that much documentation to work correctly set these values. Analyzer config, like alpha checkers, is mostly for disabling stuff that is still in development, or for playing with values during development and debugging.

I'd rather try to either cover all cases, or look for the actual macro value. Like, if you have your `PROT_WRITE | PROT_EXEC` expression, it would look like `0x02 | 0x04` in the AST, but source locations of `0x02` and `0x04` would know that they came from macros with a certain name, which you could match. That, however, would defeat the purpose of path-sensitive analysis (flags would no longer be able to be located anywhere in the AST, i.e. passed through variables; tracking the value through assignments is possible but feels horribly wrong). I keep saying that in fact not using the path-sensitive engine is not bad and might be even good for this checker because these flags are hopefully rarely passed through variables, so you'd lose less positives due to not supporting pass through variables than you'd gain by not relying on the path sensitive engine's imperfect code coverage (large chunks of the code are not analyzed path-sensitively at all because the code before it was too complex to make any sense out of it). It should also be possible (but also unpleasant) to scan the whole AST to find `PROT_WRITE`-wrapped or `PROT_EXEC`-wrapped integer literals and see their values. Or you could probably pass some preprocessor info to the analyzer and see if you could find your answers there. So this is a long-standing problem without good solutions, unfortunately.


Repository:
  rC Clang

https://reviews.llvm.org/D42645