martong created this revision.
martong added reviewers: vsavchenko, NoQ, steakhal.
Herald added subscribers: ASDenysPetrov, gamesh411, dkrupp, donat.nagy, 
Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, 
xazax.hun, whisperity.
Herald added a reviewer: Szelethus.
martong requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

In this patch I add a new NoteTag for each applied argument constraint.
This way, any other checker that reports a bug - where the applied
constraint is relevant - will display the corresponding note. With this
change we provide more information for the users to understand some
bug reports easier.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D101526

Files:
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/test/Analysis/std-c-library-functions-arg-constraints-note-tags.cpp
  clang/test/Analysis/std-c-library-functions-arg-constraints.c

Index: clang/test/Analysis/std-c-library-functions-arg-constraints.c
===================================================================
--- clang/test/Analysis/std-c-library-functions-arg-constraints.c
+++ clang/test/Analysis/std-c-library-functions-arg-constraints.c
@@ -39,6 +39,7 @@
 
 void test_alnum_symbolic(int x) {
   int ret = isalnum(x);
+  // bugpath-note@-1{{Applied constraint: The 1st arg should be within the range [[-1, -1], [0, 255]]}}
   (void)ret;
 
   clang_analyzer_eval(EOF <= x && x <= 255); // \
@@ -79,6 +80,7 @@
 
 void test_toupper_symbolic(int x) {
   int ret = toupper(x);
+  // bugpath-note@-1{{Applied constraint: The 1st arg should be within the range [[-1, -1], [0, 255]]}}
   (void)ret;
 
   clang_analyzer_eval(EOF <= x && x <= 255); // \
@@ -119,6 +121,7 @@
 
 void test_tolower_symbolic(int x) {
   int ret = tolower(x);
+  // bugpath-note@-1{{Applied constraint: The 1st arg should be within the range [[-1, -1], [0, 255]]}}
   (void)ret;
 
   clang_analyzer_eval(EOF <= x && x <= 255); // \
@@ -159,6 +162,7 @@
 
 void test_toascii_symbolic(int x) {
   int ret = toascii(x);
+  // bugpath-note@-1{{Applied constraint: The 1st arg should be within the range [[-1, -1], [0, 255]]}}
   (void)ret;
 
   clang_analyzer_eval(EOF <= x && x <= 255); // \
@@ -198,6 +202,9 @@
 }
 void test_notnull_symbolic(FILE *fp, int *buf) {
   fread(buf, sizeof(int), 10, fp);
+  // bugpath-note@-1{{Applied constraint: The 1st arg should not be NULL}}
+  // bugpath-note@-2{{Applied constraint: The 4th arg should not be NULL}}
+  // bugpath-note@-3{{Applied constraint: The size of the 1st arg should be equal to or less than the value of the 2nd arg times the 3rd arg}}
   clang_analyzer_eval(buf != 0); // \
   // report-warning{{TRUE}} \
   // bugpath-warning{{TRUE}} \
@@ -238,6 +245,12 @@
   // State split should not happen here. I.e. x == 1 should not be evaluated
   // FALSE.
   __two_constrained_args(x, y);
+  // bugpath-note@-1{{Applied constraint: The 1st arg should be within the range [1, 1]}}
+  // bugpath-note@-2{{Applied constraint: The 2nd arg should be within the range [1, 1]}}
+  //NOTE! Because of the second `clang_analyzer_eval` call we have two bug
+  //reports, thus the 'Applied constraint' notes appear twice.
+  // bugpath-note@-5{{Applied constraint: The 1st arg should be within the range [1, 1]}}
+  // bugpath-note@-6{{Applied constraint: The 2nd arg should be within the range [1, 1]}}
   clang_analyzer_eval(x == 1); // \
   // report-warning{{TRUE}} \
   // bugpath-warning{{TRUE}} \
@@ -251,6 +264,8 @@
 int __arg_constrained_twice(int);
 void test_multiple_constraints_on_same_arg(int x) {
   __arg_constrained_twice(x);
+  // bugpath-note@-1{{The 1st arg should be out of the range [1, 1]}}
+  // bugpath-note@-2{{The 1st arg should be out of the range [2, 2]}}
   // Check that both constraints are applied and only one branch is there.
   clang_analyzer_eval(x < 1 || x > 2); // \
   // report-warning{{TRUE}} \
@@ -283,6 +298,7 @@
 void test_buf_size_symbolic(int s) {
   char buf[3];
   __buf_size_arg_constraint(buf, s);
+  // bugpath-note@-1{{The size of the 1st arg should be equal to or less than the value of the 2nd arg}}
   clang_analyzer_eval(s <= 3); // \
   // report-warning{{TRUE}} \
   // bugpath-warning{{TRUE}} \
@@ -292,6 +308,7 @@
 void test_buf_size_symbolic_and_offset(int s) {
   char buf[3];
   __buf_size_arg_constraint(buf + 1, s);
+  // bugpath-note@-1{{The size of the 1st arg should be equal to or less than the value of the 2nd arg}}
   clang_analyzer_eval(s <= 2); // \
   // report-warning{{TRUE}} \
   // bugpath-warning{{TRUE}} \
@@ -312,6 +329,7 @@
 void test_buf_size_symbolic_with_multiplication(size_t s) {
   short buf[3];
   __buf_size_arg_constraint_mul(buf, s, sizeof(short));
+  // bugpath-note@-1{{The size of the 1st arg should be equal to or less than the value of the 2nd arg times the 3rd arg}}
   clang_analyzer_eval(s * sizeof(short) <= 6); // \
   // report-warning{{TRUE}} \
   // bugpath-warning{{TRUE}} \
@@ -320,6 +338,7 @@
 void test_buf_size_symbolic_and_offset_with_multiplication(size_t s) {
   short buf[3];
   __buf_size_arg_constraint_mul(buf + 1, s, sizeof(short));
+  // bugpath-note@-1{{The size of the 1st arg should be equal to or less than the value of the 2nd arg times the 3rd arg}}
   clang_analyzer_eval(s * sizeof(short) <= 4); // \
   // report-warning{{TRUE}} \
   // bugpath-warning{{TRUE}} \
Index: clang/test/Analysis/std-c-library-functions-arg-constraints-note-tags.cpp
===================================================================
--- /dev/null
+++ clang/test/Analysis/std-c-library-functions-arg-constraints-note-tags.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctions \
+// RUN:   -analyzer-checker=alpha.unix.StdCLibraryFunctionArgs \
+// RUN:   -analyzer-checker=debug.StdCLibraryFunctionsTester \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:DisplayLoadedSummaries=true \
+// RUN:   -analyzer-checker=debug.ExprInspection \
+// RUN:   -analyzer-config eagerly-assume=false \
+// RUN:   -triple i686-unknown-linux \
+// RUN:   -analyzer-output=text \
+// RUN:   -verify
+
+int __single_val_1(int);      // [1, 1]
+
+int test_note(int x, int y) {
+    __single_val_1(x);  // expected-note{{Applied constraint: The 1st arg should be within the range [1, 1]}}
+    return y / (1 - x); // expected-warning{{Division by zero}} \
+                        // expected-note{{Division by zero}}
+}
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -810,6 +810,7 @@
   ProgramStateRef State = C.getState();
 
   ProgramStateRef NewState = State;
+  ExplodedNode *NewNode = C.getPredecessor();
   for (const ValueConstraintPtr &Constraint : Summary.getArgConstraints()) {
     ProgramStateRef SuccessSt = Constraint->apply(NewState, Call, Summary, C);
     ProgramStateRef FailureSt =
@@ -819,17 +820,23 @@
       if (ExplodedNode *N = C.generateErrorNode(NewState))
         reportBug(Call, N, Constraint.get(), Summary, C);
       break;
-    } else {
-      // We will apply the constraint even if we cannot reason about the
-      // argument. This means both SuccessSt and FailureSt can be true. If we
-      // weren't applying the constraint that would mean that symbolic
-      // execution continues on a code whose behaviour is undefined.
-      assert(SuccessSt);
-      NewState = SuccessSt;
+    }
+    // We will apply the constraint even if we cannot reason about the
+    // argument. This means both SuccessSt and FailureSt can be true. If we
+    // weren't applying the constraint that would mean that symbolic
+    // execution continues on a code whose behaviour is undefined.
+    assert(SuccessSt);
+    NewState = SuccessSt;
+    if (NewState != State) {
+      SmallString<64> Msg;
+      Msg += "Applied constraint: ";
+      Msg += Constraint->describe(NewState, Summary);
+      NewNode = C.addTransition(
+          NewState, NewNode,
+          C.getNoteTag([Msg](PathSensitiveBugReport &BR,
+                             llvm::raw_ostream &OS) { OS << Msg; }));
     }
   }
-  if (NewState && NewState != State)
-    C.addTransition(NewState);
 }
 
 void StdLibraryFunctionsChecker::checkPostCall(const CallEvent &Call,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to