[PATCH] D120310: [clang][analyzer] Add modeling of 'errno' (work-in-progress).

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

steakhal added inline comments.


================
Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:351-353
+def ErrnoChecker : Checker<"Errno">,
+  HelpText<"Make the special value 'errno' available to other checkers.">,
+  Documentation<HasDocumentation>;
----------------
I think `ErrnoModeling` would be more appropriate.


================
Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:1564-1567
+/// Make sure that ErrnoChecker is also enabled.
+def ErrnoTesterChecker : Checker<"ErrnoTest">,
+  HelpText<"Check modeling aspects of 'errno'.">,
+  Documentation<NotDocumented>;
----------------
If this is the case, it should have a `Dependency<Errno>`


================
Comment at: clang/lib/StaticAnalyzer/Checkers/Errno.h:22
+namespace ento {
+namespace errno_check {
+
----------------
balazske wrote:
> steakhal wrote:
> > I think we can settle on something better. What about calling it simply `errno`?
> Just `errno` may not work because it collides with the "real" errno (that is a macro).
Ah, I see. Ugly macros!
Then why not simply leave it in the `clang::ento` namespace?


================
Comment at: clang/lib/StaticAnalyzer/Core/MemRegion.cpp:1156
 
 /// getSymbolicRegion - Retrieve or create a "symbolic" memory region.
+const SymbolicRegion *
----------------
I would recommend removing this comment alltogether.
The header is already annotated, by the time this will get out-of-sync anyway.


================
Comment at: clang/test/Analysis/errno.c:15-21
+#ifdef ERRNO_VAR
+#include "Inputs/errno_var.h"
+#endif
+#ifdef ERRNO_FUNC
+#include "Inputs/errno_func.h"
+#endif
+#include "Inputs/system-header-simulator.h"
----------------
Fun fact, I think you can `#include ERRNO_HEADER`, if you pass the `-DERRNO_HEADER=\"Inputs/errno_var.h\"` :D


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120310