[PATCH] D71433: [analyzer] CERT: POS34-C

[Zurab Tsinadze via Phabricator via cfe-commits]

zukatsinadze marked 4 inline comments as done.
zukatsinadze added inline comments.


================
Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:765
+  HelpText<"Finds calls to the `putenv` function which pass a pointer to "
+           "an automatic variable as the argument. (CERT POS 34C)">,
+  Documentation<HasDocumentation>;
----------------
Charusso wrote:
> I would write ##`putenv`## -> `'putenv'` and the CERT rule-number should be clear from that point so you could emit it.
Oops. Forgot this one. Will fix it later.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/cert/PutenvWithAutoChecker.cpp:45-52
+  if (const auto *DRE = dyn_cast<DeclRefExpr>(ArgExpr->IgnoreImpCasts()))
+    if (const auto *VD = dyn_cast<VarDecl>(DRE->getDecl()))
+      IsPossiblyAutoVar = isa<ParmVarDecl>(VD) && isa<UnknownSpaceRegion>(MSR);
+
+  if (!IsPossiblyAutoVar &&
+      (isa<HeapSpaceRegion>(MSR) || isa<StaticGlobalSpaceRegion>(MSR) ||
+       isa<GlobalSystemSpaceRegion>(MSR) ||
----------------
NoQ wrote:
> Simply check whether the memory space is a stack memory space. This should be a one-liner.
I could not get rid of `isPossiblyAutoVar` and `GlobalInternalSpaceRegion`. 


================
Comment at: clang/test/Analysis/cert/pos34-c-fp-suppression.cpp:15
+int volatile_memory1(char *a) {
+  return putenv(a);
+  // expected-warning at -1 {{'putenv' function should not be called with auto variables}}
----------------
I need `isPossiblyAutoVar` for this type. 


================
Comment at: clang/test/Analysis/cert/pos34-c-fp-suppression.cpp:20
+void volatile_memory2(char *a) {
+  char *buff = (char *)"hello";
+  putenv(buff);
----------------
And `GlobalInternalSpaceRegion` for this.


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

https://reviews.llvm.org/D71433