[analyzer][StdLibraryFunctionsChecker] Have proper weak dependencies

[martong]

Summary

We want the generice StdLibraryFunctionsChecker to report only if there
are no specific checkers that would handle the argument constraint for a
function.

Note, the assumptions are still evaluated, even if the arguement
constraint checker is set to not report. This means that the assumptions
made in the generic StdLibraryFunctionsChecker should be an
over-approximation of the assumptions made in the specific checkers. But
most importantly, the assumptions should not contradict.

[Szelethus]

This is exactly how I imagines weak dependencies to work. LGTM on my end.

[martong]

clang/include/clang/StaticAnalyzer/Checkers/Checkers.td

352

Umm, we should not have removed this. The checker always asserts that the arguments are initialized and that is checked by the dependency.

This is causing an assertion in D87081. Unfortunately, we hadn't have any tests to catch this, but now we have one, exactly in D87081. So I am putting back this dependency there.

clang/include/clang/StaticAnalyzer/Checkers/Checkers.td

 //===----------------------------------------------------------------------===//
 // APIModeling.
 //===----------------------------------------------------------------------===//
 
 let ParentPackage = APIModeling in {
 
 def StdCLibraryFunctionsChecker : Checker<"StdCLibraryFunctions">,
   HelpText<"Improve modeling of the C standard library functions">,
-  Dependencies<[CallAndMessageModeling]>,
   CheckerOptions<[
     CmdLineOption<Boolean,
                   "DisplayLoadedSummaries",
                   "If set to true, the checker displays the found summaries "
                   "for the given translation unit.",
                   "false",
                   Released,
                   Hide>,
def BlockInCriticalSectionChecker : Checker<"BlockInCriticalSection">,
   HelpText<"Check for calls to blocking functions inside a critical section">,
   Documentation<HasAlphaDocumentation>;
 
 def StdCLibraryFunctionArgsChecker : Checker<"StdCLibraryFunctionArgs">,
   HelpText<"Check constraints of arguments of C standard library functions, "
            "such as whether the parameter of isalpha is in the range [0, 255] "
            "or is EOF.">,
   Dependencies<[StdCLibraryFunctionsChecker]>,
-  WeakDependencies<[NonNullParamChecker]>,
+  WeakDependencies<[CallAndMessageChecker, NonNullParamChecker, StreamChecker]>,
   Documentation<NotDocumented>;
 
 } // end "alpha.unix"
 
 //===----------------------------------------------------------------------===//
 // C++ checkers.
 //===----------------------------------------------------------------------===//
 

clang/test/Analysis/analyzer-enabled-checkers.c

 // RUN: %clang --analyze %s --target=x86_64-pc-linux-gnu \
 // RUN:   -Xclang -analyzer-list-enabled-checkers \
 // RUN:   -Xclang -analyzer-display-progress \
 // RUN:   2>&1 | FileCheck %s --implicit-check-not=ANALYZE \
 // RUN:                       --implicit-check-not=\.
 
 // CHECK:      OVERVIEW: Clang Static Analyzer Enabled Checkers List
 // CHECK-EMPTY:
-// CHECK-NEXT: core.CallAndMessageModeling
 // CHECK-NEXT: apiModeling.StdCLibraryFunctions
 // CHECK-NEXT: apiModeling.TrustNonnull
 // CHECK-NEXT: apiModeling.llvm.CastValue
 // CHECK-NEXT: apiModeling.llvm.ReturnValue
+// CHECK-NEXT: core.CallAndMessageModeling
 // CHECK-NEXT: core.CallAndMessage
 // CHECK-NEXT: core.DivideZero
 // CHECK-NEXT: core.DynamicTypePropagation
 // CHECK-NEXT: core.NonNullParamChecker
 // CHECK-NEXT: core.NonnilStringConstants
 // CHECK-NEXT: core.NullDereference
 // CHECK-NEXT: core.StackAddrEscapeBase
 // CHECK-NEXT: core.StackAddressEscape

clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c

+// Here we test the order of the Checkers when StdCLibraryFunctionArgs is
+// enabled.
+
 // RUN: %clang --analyze %s --target=x86_64-pc-linux-gnu \
+// RUN:   -Xclang -analyzer-checker=core \
+// RUN:   -Xclang -analyzer-checker=apiModeling.StdCLibraryFunctions \
+// RUN:   -Xclang -analyzer-config \
+// RUN:      -Xclang apiModeling.StdCLibraryFunctions:ModelPOSIX=true \
+// RUN:   -Xclang -analyzer-checker=alpha.unix.StdCLibraryFunctionArgs \
+// RUN:   -Xclang -analyzer-checker=alpha.unix.Stream \
 // RUN:   -Xclang -analyzer-list-enabled-checkers \
 // RUN:   -Xclang -analyzer-display-progress \
 // RUN:   2>&1 | FileCheck %s --implicit-check-not=ANALYZE \
 // RUN:                       --implicit-check-not=\.
 
 // CHECK:      OVERVIEW: Clang Static Analyzer Enabled Checkers List
 // CHECK-EMPTY:
 // CHECK-NEXT: core.CallAndMessageModeling
+// CHECK-NEXT: core.CallAndMessage
+// CHECK-NEXT: core.NonNullParamChecker
+// CHECK-NEXT: alpha.unix.Stream
 // CHECK-NEXT: apiModeling.StdCLibraryFunctions
+// CHECK-NEXT: alpha.unix.StdCLibraryFunctionArgs
 // CHECK-NEXT: apiModeling.TrustNonnull
 // CHECK-NEXT: apiModeling.llvm.CastValue
 // CHECK-NEXT: apiModeling.llvm.ReturnValue
-// CHECK-NEXT: core.CallAndMessage
 // CHECK-NEXT: core.DivideZero
 // CHECK-NEXT: core.DynamicTypePropagation
-// CHECK-NEXT: core.NonNullParamChecker
 // CHECK-NEXT: core.NonnilStringConstants
 // CHECK-NEXT: core.NullDereference
 // CHECK-NEXT: core.StackAddrEscapeBase
 // CHECK-NEXT: core.StackAddressEscape
 // CHECK-NEXT: core.UndefinedBinaryOperatorResult
 // CHECK-NEXT: core.VLASize
 // CHECK-NEXT: core.builtin.BuiltinFunctions
 // CHECK-NEXT: core.builtin.NoReturnFunctions

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

+// Check that the more specific checkers report and not the generic
+// StdCLibraryFunctionArgs checker.
+
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctions \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:ModelPOSIX=true \
+// RUN:   -analyzer-checker=alpha.unix.StdCLibraryFunctionArgs \
+// RUN:   -analyzer-checker=alpha.unix.Stream \
+// RUN:   -triple x86_64-unknown-linux-gnu \
+// RUN:   -verify
+
+
+// Make sure that all used functions have their summary loaded.
+
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctions \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:ModelPOSIX=true \
+// RUN:   -analyzer-checker=alpha.unix.StdCLibraryFunctionArgs \
+// RUN:   -analyzer-checker=alpha.unix.Stream \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:DisplayLoadedSummaries=true \
+// RUN:   -triple x86_64-unknown-linux 2>&1 | FileCheck %s
+
+// CHECK: Loaded summary for: int isalnum(int)
+// CHECK: Loaded summary for: unsigned long fread(void *restrict, size_t, size_t, FILE *restrict) __attribute__((nonnull(1)))
+// CHECK: Loaded summary for: int fileno(FILE *stream)
+
+void initializeSummaryMap();
+// We analyze this function first, and the call expression inside initializes
+// the summary map. This way we force the loading of the summaries. The
+// summaries would not be loaded without this because during the first bug
+// report in WeakDependency::checkPreCall we stop further evaluation. And
+// StdLibraryFunctionsChecker lazily initializes its summary map from its
+// checkPreCall.
+void analyzeThisFirst() {
+  initializeSummaryMap();
+}
+
+typedef __typeof(sizeof(int)) size_t;
+struct FILE;
+typedef struct FILE FILE;
+
+int isalnum(int);
+size_t fread(void *restrict, size_t, size_t, FILE *restrict) __attribute__((nonnull(1)));
+int fileno(FILE *stream);
+
+void test_uninit_arg() {
+  int v;
+  int r = isalnum(v); // \
+  // expected-warning{{1st function call argument is an uninitialized value [core.CallAndMessage]}}
+  (void)r;
+}
+
+void test_notnull_arg(FILE *F) {
+  int *p = 0;
+  fread(p, sizeof(int), 5, F); // \
+  expected-warning{{Null pointer passed to 1st parameter expecting 'nonnull' [core.NonNullParamChecker]}}
+}
+
+void test_notnull_stream_arg() {
+  fileno(0); // \
+  // expected-warning{{Stream pointer might be NULL [alpha.unix.Stream]}}
+}