[analyzer] Bifurcate on getenv() calls

[steakhal]

Summary

The getenv() function might return NULL just like any other function.
However, in case of getenv() a state-split seems justified since the
programmer should expect the failure of this function.

secure_getenv(const char *name) behaves the same way but is not handled right now.
Note that std::getenv() is also not handled.

[steakhal]

clang/test/Analysis/std-c-library-functions.c

260

typo

[NoQ]

a state-split seems justified since the programmer should expect the failure of this function

The programmer can often make an argument that his program is always run "in a controlled environment". It could be a system daemon that's only invoked by the system in a specific manner or it could be a custom server that's run only on one server machine fully controlled by the developer. So most of the time, yeah, it's probably a useful behavior, but sometimes people may want to disable it. Maybe we should make a single global setting, say, -analyzer-config assume-controlled-environment=... or something like that, that'd make the static analyzer produce less such bifurcations.

[steakhal]

In D111245#3047214, @NoQ wrote:

a state-split seems justified since the programmer should expect the failure of this function

The programmer can often make an argument that his program is always run "in a controlled environment". It could be a system daemon that's only invoked by the system in a specific manner or it could be a custom server that's run only on one server machine fully controlled by the developer. So most of the time, yeah, it's probably a useful behavior, but sometimes people may want to disable it. Maybe we should make a single global setting, say, -analyzer-config assume-controlled-environment=... or something like that, that'd make the static analyzer produce less such bifurcations.

Hmm, that seems reasonable. Check the child revision: D111296.

[martong]

LGTM!

clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

void StdLibraryFunctionsChecker::initFunctionSummaries(
   //                  int delimiter, FILE *restrict stream);
   addToFunctionSummaryMap(
       "getdelim",
       Signature(ArgTypes{CharPtrPtrRestrictTy, SizePtrRestrictTy, IntTy,
                          FilePtrRestrictTy},
                 RetType{Ssize_tTy}),
       GetLineSummary);
 
+  // char *getenv(const char *name);
+  addToFunctionSummaryMap(
+      "getenv", Signature(ArgTypes{ConstCharPtrTy}, RetType{CharPtrTy}),
+      Summary(NoEvalCall)
+          .Case({NotNull(Ret)})
+          .Case({NotNull(Ret)->negate()})
+          .ArgConstraint(NotNull(ArgNo(0))));
+
   if (ModelPOSIX) {
 
     // long a64l(const char *str64);
     addToFunctionSummaryMap(
         "a64l", Signature(ArgTypes{ConstCharPtrTy}, RetType{LongTy}),
         Summary(NoEvalCall).ArgConstraint(NotNull(ArgNo(0))));
 
     // char *l64a(long value);

clang/test/Analysis/std-c-library-functions.c

 
 void test_call_by_pointer() {
   typedef int (*func)(int);
   func f = isascii;
   clang_analyzer_eval(f('A')); // expected-warning{{TRUE}}
   f = ispunct;
   clang_analyzer_eval(f('A')); // expected-warning{{FALSE}}
 }
+
+char *getenv(const char *name);
+void test_getenv() {
+  // getenv() bifurcates here.
+  clang_analyzer_eval(getenv("FOO") == 0);
+  // expected-warning@-1 {{TRUE}}
+  // expected-warning@-2 {{FALSE}}
+}