[analyzer] StdLibraryFunctionsChecker: Add option to display loaded summaries

[Szelethus]

This is a great idea, but the tests just seem to, well, test the new functionality?

On a different issue, take a look at how certain help related frontend flags (not -analyzer-config ones!) skip the analysis, like -analyzer-checker-help, or -analyzer-list-enabled-checkers. If I were to set DisplayLoadedSummaries to true, most probably I'd only be interested in the capabilities of the checker, not the analysis itself. Though, I don't have an out-of-the-box solution that could work here, unless we outright emit an error in the checker registry function. That wouldn't be very nice, would it? :^) So, for the time being, regard this as me thinking aloud.

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

299

I see what you mean, but "loaded" is a bit ambiguous unless you know how the checker operates.

[balazske]

The new option will simply list the (found) functions that have a summary based check enabled. (The term "Loaded" may be misleading in the current implementation, somebody can think if it is loaded from file?) A more detailed output would be better (display the signature too, or maybe the whole summary?). For simple purpose the current way may be enough but it may be useful (probably for a non-checker-developer too) to see the summary details.

[martong]

• Rebase to master
• Add the option to the lit test analyzer-config.c

[martong]

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

299

Yeah, okay, what about "the found summaries" or "applicable summaries" or "? This list can be different for every TU. So I am adding "... for the translation unit".

[martong]

• Better description for the option in Checkers.td

[martong]

In D78118#1981592, @Szelethus wrote:

This is a great idea, but the tests just seem to, well, test the new functionality?

On a different issue, take a look at how certain help related frontend flags (not -analyzer-config ones!) skip the analysis, like -analyzer-checker-help, or -analyzer-list-enabled-checkers. If I were to set DisplayLoadedSummaries to true, most probably I'd only be interested in the capabilities of the checker, not the analysis itself. Though, I don't have an out-of-the-box solution that could work here, unless we outright emit an error in the checker registry function. That wouldn't be very nice, would it? :^) So, for the time being, regard this as me thinking aloud.

This new config option will simply list the (found) functions that have a summary based check enabled. This can be different for each TU. If a function cannot be looked up in a TU then we will never use its summary.
On the other hand, yes, it would be a good idea to have an option to list all the configured summaries. This list would be the same for each TU. Do you mind if I'd do that rather in another patch?

[martong]

In D78118#1988464, @balazske wrote:

The new option will simply list the (found) functions that have a summary based check enabled. (The term "Loaded" may be misleading in the current implementation, somebody can think if it is loaded from file?) A more detailed output would be better (display the signature too, or maybe the whole summary?). For simple purpose the current way may be enough but it may be useful (probably for a non-checker-developer too) to see the summary details.

Yes, "loaded" might not be the best, I changed it to "found". About the more detailed output: is there a way to dump nicely the signature of a function decl? I don't want to dump the AST of it because that might be overkill. Could I just dump the content of the buffer that relates to the SourceRange of the FunctionDecl (if that does not have a definition)?

[martong]

• Handle and test the display of signatures

[martong]

In D78118#2007943, @martong wrote:

In D78118#1988464, @balazske wrote:

The new option will simply list the (found) functions that have a summary based check enabled. (The term "Loaded" may be misleading in the current implementation, somebody can think if it is loaded from file?) A more detailed output would be better (display the signature too, or maybe the whole summary?). For simple purpose the current way may be enough but it may be useful (probably for a non-checker-developer too) to see the summary details.

Yes, "loaded" might not be the best, I changed it to "found". About the more detailed output: is there a way to dump nicely the signature of a function decl? I don't want to dump the AST of it because that might be overkill. Could I just dump the content of the buffer that relates to the SourceRange of the FunctionDecl (if that does not have a definition)?

I just have updated the patch so now the signature is printed.

[martong]

This is a great idea, but the tests just seem to, well, test the new functionality?

I updated the patch so now we have a new RUN line that checks with FileCheck.

[balazske]

clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

651

Maybe the Decl::print method can be used? I pretty-prints the declaration (hopefully there is an option to print only the prototype), otherwise we can get the code as written in the source-file(?) that makes comparing the output more difficult.

665

I would prefer to display the function prototype in a separate line (if not only the name is included in the string). Or at least enclose the prototype with ' characters. Or the Loaded summary for: form.

[martong]

clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

651

Yeah, thanks! I switched to use that!

665

Ok I use the Loaded summary for: form from now on.

[martong]

• Use Decl::print and 'for: '

[Szelethus]

It would be nice to have a user-facing option that lists the functions this checker is capable of modeling in another patch :)

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

299

There is no need to keep this short :) Feel free to go on a bit of a tangent, if you prefer. But, to me, it seems like this isn't a user-facing option, this is a debug flag to check which functions are present in the TU, which is okay, but in that case we better make it Hidden, so it will only show up in -analyzer-checker-help-developer.

[xazax.hun]

LGTM!

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<[NonNullParamChecker, CallAndMessageChecker]>,
+  CheckerOptions<[
+    CmdLineOption<Boolean,
+                  "DisplayLoadedSummaries",
+                  "If set to true, the checker displays the found summaries "
+                  "for the given translation unit.",
+                  "false",
+                  Released>
+  ]>,
   Documentation<NotDocumented>;
 
 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]>,
   Documentation<NotDocumented>;

clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

public:
   enum CheckKind {
     CK_StdCLibraryFunctionArgsChecker,
     CK_StdCLibraryFunctionsTesterChecker,
     CK_NumCheckKinds
   };
   DefaultBool ChecksEnabled[CK_NumCheckKinds];
   CheckerNameRef CheckNames[CK_NumCheckKinds];
 
+  bool DisplayLoadedSummaries = false;
+
 private:
   Optional<Summary> findFunctionSummary(const FunctionDecl *FD,
                                         CheckerContext &C) const;
   Optional<Summary> findFunctionSummary(const CallEvent &Call,
                                         CheckerContext &C) const;
 
   void initFunctionSummaries(CheckerContext &C) const;
 
if (const llvm::Optional<int> OptInt =
       return *OptInt;
     return -1;
   }();
 
   // Auxiliary class to aid adding summaries to the summary map.
   struct AddToFunctionSummaryMap {
     const ASTContext &ACtx;
     FunctionSummaryMapType &Map;
-    AddToFunctionSummaryMap(const ASTContext &ACtx, FunctionSummaryMapType &FSM)
-        : ACtx(ACtx), Map(FSM) {}
+    bool DisplayLoadedSummaries;
+    AddToFunctionSummaryMap(const ASTContext &ACtx, FunctionSummaryMapType &FSM,
+                            bool DisplayLoadedSummaries)
+        : ACtx(ACtx), Map(FSM), DisplayLoadedSummaries(DisplayLoadedSummaries) {
+    }
+
     // Add a summary to a FunctionDecl found by lookup. The lookup is performed
     // by the given Name, and in the global scope. The summary will be attached
     // to the found FunctionDecl only if the signatures match.
     void operator()(StringRef Name, const Summary &S) {
       IdentifierInfo &II = ACtx.Idents.get(Name);
       auto LookupRes = ACtx.getTranslationUnitDecl()->lookup(&II);
       if (LookupRes.size() == 0)
         return;
       for (Decl *D : LookupRes) {
         if (auto *FD = dyn_cast<FunctionDecl>(D)) {
           if (S.matchesSignature(FD)) {
             auto Res = Map.insert({FD->getCanonicalDecl(), S});
             assert(Res.second && "Function already has a summary set!");
             (void)Res;
+            if (DisplayLoadedSummaries) {
+              llvm::errs() << "Loaded summary for: ";
+              FD->print(llvm::errs());
+              llvm::errs() << "\n";
+            }
             return;
           }
         }
       }
     }
     // Add several summaries for the given name.
     void operator()(StringRef Name, const std::vector<Summary> &Summaries) {
       for (const Summary &S : Summaries)
         operator()(Name, S);
     }
-  } addToFunctionSummaryMap(ACtx, FunctionSummaryMap);
+  } addToFunctionSummaryMap(ACtx, FunctionSummaryMap, DisplayLoadedSummaries);
 
   // We are finally ready to define specifications for all supported functions.
   //
   // The signature needs to have the correct number of arguments.
   // However, we insert `Irrelevant' when the type is insignificant.
   //
   // Argument ranges should always cover all variants. If return value
   // is completely unknown, omit it from the respective range set.
addToFunctionSummaryMap("__variadic",
                             Summary(ArgTypes{VoidPtrTy, ConstCharPtrTy},
                                     RetType{IntTy}, EvalCallAsPure)
                                 .ArgConstraint(NotNull(ArgNo(0)))
                                 .ArgConstraint(NotNull(ArgNo(1))));
   }
 }
 
 void ento::registerStdCLibraryFunctionsChecker(CheckerManager &mgr) {
-  mgr.registerChecker<StdLibraryFunctionsChecker>();
+  auto *Checker = mgr.registerChecker<StdLibraryFunctionsChecker>();
+  Checker->DisplayLoadedSummaries =
+      mgr.getAnalyzerOptions().getCheckerBooleanOption(
+          Checker, "DisplayLoadedSummaries");
 }
 
 bool ento::shouldRegisterStdCLibraryFunctionsChecker(const CheckerManager &mgr) {
   return true;
 }
 
 #define REGISTER_CHECKER(name)                                                 \
   void ento::register##name(CheckerManager &mgr) {                             \

clang/test/Analysis/analyzer-config.c

 // RUN: %clang_analyze_cc1 -analyzer-checker=debug.ConfigDumper > %t 2>&1
 // RUN: FileCheck --input-file=%t %s --match-full-lines
 
 // CHECK: [config]
 // CHECK-NEXT: add-pop-up-notes = true
 // CHECK-NEXT: aggressive-binary-operation-simplification = false
 // CHECK-NEXT: alpha.clone.CloneChecker:IgnoredFilesPattern = ""
 // CHECK-NEXT: alpha.clone.CloneChecker:MinimumCloneComplexity = 50
 // CHECK-NEXT: alpha.clone.CloneChecker:ReportNormalClones = true
 // CHECK-NEXT: alpha.cplusplus.STLAlgorithmModeling:AggressiveStdFindModeling = false
 // CHECK-NEXT: alpha.security.MmapWriteExec:MmapProtExec = 0x04
 // CHECK-NEXT: alpha.security.MmapWriteExec:MmapProtRead = 0x01
 // CHECK-NEXT: alpha.security.taint.TaintPropagation:Config = ""
+// CHECK-NEXT: apiModeling.StdCLibraryFunctions:DisplayLoadedSummaries = false
 // CHECK-NEXT: apply-fixits = false
 // CHECK-NEXT: avoid-suppressing-null-argument-paths = false
 // CHECK-NEXT: c++-allocator-inlining = true
 // CHECK-NEXT: c++-container-inlining = false
 // CHECK-NEXT: c++-inlining = destructors
 // CHECK-NEXT: c++-shared_ptr-inlining = false
 // CHECK-NEXT: c++-stdlib-inlining = true
 // CHECK-NEXT: c++-temp-dtor-inlining = true

 // CHECK-NEXT: suppress-inlined-defensive-checks = true
 // CHECK-NEXT: suppress-null-return-paths = true
 // CHECK-NEXT: track-conditions = true
 // CHECK-NEXT: track-conditions-debug = false
 // CHECK-NEXT: unix.DynamicMemoryModeling:Optimistic = false
 // CHECK-NEXT: unroll-loops = false
 // CHECK-NEXT: widen-loops = false
 // CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 103
+// CHECK-NEXT: num-entries = 104

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

 // RUN: %clang_analyze_cc1 %s \
 // RUN:   -analyzer-checker=core \
 // RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctions \
 // RUN:   -analyzer-checker=debug.ExprInspection \
 // RUN:   -analyzer-config eagerly-assume=false \
 // RUN:   -triple thumbv7-a15-linux \
 // RUN:   -verify
 
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctions \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:DisplayLoadedSummaries=true \
+// RUN:   -analyzer-checker=debug.ExprInspection \
+// RUN:   -analyzer-config eagerly-assume=false \
+// RUN:   -triple i686-unknown-linux 2>&1 | FileCheck %s
+
+//      CHECK: Loaded summary for: int isalnum(int)
+// CHECK-NEXT: Loaded summary for: int isalpha(int)
+// CHECK-NEXT: Loaded summary for: int isascii(int)
+// CHECK-NEXT: Loaded summary for: int isblank(int)
+// CHECK-NEXT: Loaded summary for: int isdigit(int)
+// CHECK-NEXT: Loaded summary for: int isgraph(int)
+// CHECK-NEXT: Loaded summary for: int islower(int)
+// CHECK-NEXT: Loaded summary for: int isprint(int)
+// CHECK-NEXT: Loaded summary for: int ispunct(int)
+// CHECK-NEXT: Loaded summary for: int isspace(int)
+// CHECK-NEXT: Loaded summary for: int isupper(int)
+// CHECK-NEXT: Loaded summary for: int isxdigit(int)
+// CHECK-NEXT: Loaded summary for: int getc(FILE *)
+// CHECK-NEXT: Loaded summary for: int fgetc(FILE *)
+// CHECK-NEXT: Loaded summary for: int getchar()
+// CHECK-NEXT: Loaded summary for: ssize_t read(int, void *, size_t)
+// CHECK-NEXT: Loaded summary for: ssize_t write(int, const void *, size_t)
+// CHECK-NEXT: Loaded summary for: unsigned int fread(void *restrict, size_t, size_t, FILE *)
+// CHECK-NEXT: Loaded summary for: unsigned int fwrite(const void *restrict, size_t, size_t, FILE *restrict)
+// CHECK-NEXT: Loaded summary for: ssize_t getline(char **, size_t *, FILE *)
+
 void clang_analyzer_eval(int);
 
 int glob;
 
 typedef struct FILE FILE;
 #define EOF -1
 
 int getc(FILE *);