[PATCH] D42645: New simple Checker for mmap calls

[Ed Maste via Phabricator via cfe-commits]

emaste added inline comments.


================
Comment at: include/clang/StaticAnalyzer/Checkers/Checkers.td:417-419
+// Operating systems specific PROT_READ/PROT_WRITE values is not implemented,
+// thus ought to be overriden with the proper analyser-config variables
+// remain in alpha until the state changes
----------------
devnexen wrote:
> emaste wrote:
> > I'm a bit confused by this comment; this checker works as-is for most common operating system cases, correct?
> Most of them yes, at least Muslc linux most of glibc I tested too. Not to mention *BSD ... But might be safer to put it as alpha for a start.
OK - to me it implies that the checker only works (anywhere) if the user provides the flag values. Maybe something like "the defaults are correct for several common operating systems, but may need to be overridden ...."


================
Comment at: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp:64
+      if (!BT)
+        BT.reset(new BugType(this, "W^X check fails, Write Exec prot flags set", "Security"));
+
----------------
devnexen wrote:
> emaste wrote:
> > `Write & Exec` (or `Write and Exec`) perhaps (assuming it doesn't become over-long then)?
> I kept short intentionally indeed we can always change but the user in order to use it needs to enable it willingly so I assumed the user might know enough about the topic in question.
Understood. To me it just read as "Write Exec" as one entity.


https://reviews.llvm.org/D42645