[PATCH] D101358: [analyzer][StdLibraryFunctionsChecker] Track dependent arguments

[Gabor Marton via Phabricator via cfe-commits]

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.

When we report an argument constraint violation, we should track those
other arguments that participate in the evaluation of the violation. By
default, we depend only on the argument that is constrained, however,
there are some special cases like the buffer size constraint that might
be encoded in another argument(s).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D101358

Files:
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/test/Analysis/std-c-library-functions-arg-constraints-tracking-notes.c
  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
@@ -220,8 +220,8 @@
   enum { BUFFER_SIZE = 1024 };
   wchar_t wbuf[BUFFER_SIZE]; // bugpath-note{{'wbuf' initialized here}}
 
-  const size_t size = sizeof(*wbuf);
-  const size_t nitems = sizeof(wbuf);
+  const size_t size = sizeof(*wbuf);   // bugpath-note{{'size' initialized to}}
+  const size_t nitems = sizeof(wbuf);  // bugpath-note{{'nitems' initialized to}}
 
   // The 3rd parameter should be the number of elements to read, not
   // the size in bytes.
Index: clang/test/Analysis/std-c-library-functions-arg-constraints-tracking-notes.c
===================================================================
--- /dev/null
+++ clang/test/Analysis/std-c-library-functions-arg-constraints-tracking-notes.c
@@ -0,0 +1,33 @@
+// Check the bugpath related to the reports.
+// 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-checker=debug.ExprInspection \
+// RUN:   -triple x86_64-unknown-linux-gnu \
+// RUN:   -analyzer-output=text \
+// RUN:   -verify=bugpath
+
+typedef typeof(sizeof(int)) size_t;
+
+int __buf_size_arg_constraint(const void *, size_t);
+void test_buf_size_concrete() {
+  char buf[3];                       // bugpath-note{{'buf' initialized here}}
+  int s = 4;                         // bugpath-note{{'s' initialized to 4}}
+  __buf_size_arg_constraint(buf, s); // \
+  // bugpath-warning{{Function argument constraint is not satisfied}} \
+  // bugpath-note{{}} \
+  // bugpath-note{{Function argument constraint is not satisfied}}
+}
+
+int __buf_size_arg_constraint_mul(const void *, size_t, size_t);
+void test_buf_size_concrete_with_multiplication() {
+  short buf[3];                               // bugpath-note{{'buf' initialized here}}
+  int s1 = 4;                                 // bugpath-note{{'s1' initialized to 4}}
+  int s2 = sizeof(short);                     // bugpath-note{{'s2' initialized to}}
+  __buf_size_arg_constraint_mul(buf, s1, s2); // \
+  // bugpath-warning{{Function argument constraint is not satisfied}} \
+  // bugpath-note{{}} \
+  // bugpath-note{{Function argument constraint is not satisfied}}
+}
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -134,6 +134,12 @@
     }
     ArgNo getArgNo() const { return ArgN; }
 
+    // Return those arguments that should be tracked when we report a bug. By
+    // default it is the argument that is constrained, however, in some special
+    // cases we need to track other arguments as well. E.g. a buffer size might
+    // be encoded in another argument.
+    virtual std::vector<ArgNo> getArgsToTrack() const { return {ArgN}; }
+
     virtual StringRef getName() const = 0;
 
     // Give a description that explains the constraint to the user. Used when
@@ -309,6 +315,15 @@
         : ValueConstraint(Buffer), SizeArgN(BufSize),
           SizeMultiplierArgN(BufSizeMultiplier) {}
 
+    std::vector<ArgNo> getArgsToTrack() const override {
+      std::vector<ArgNo> Result{ArgN};
+      if (SizeArgN)
+        Result.push_back(*SizeArgN);
+      if (SizeMultiplierArgN)
+        Result.push_back(*SizeMultiplierArgN);
+      return Result;
+    }
+
     std::string describe(ProgramStateRef State,
                          const Summary &Summary) const override;
 
@@ -576,7 +591,9 @@
           CheckNames[CK_StdCLibraryFunctionArgsChecker],
           "Unsatisfied argument constraints", categories::LogicError);
     auto R = std::make_unique<PathSensitiveBugReport>(*BT_InvalidArg, Msg, N);
-    bugreporter::trackExpressionValue(N, Call.getArgExpr(VC->getArgNo()), *R);
+
+    for (ArgNo ArgN : VC->getArgsToTrack())
+      bugreporter::trackExpressionValue(N, Call.getArgExpr(ArgN), *R);
 
     // Highlight the range of the argument that was violated.
     R->addRange(Call.getArgSourceRange(VC->getArgNo()));


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D101358.340812.patch
Type: text/x-patch
Size: 4471 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20210427/6a049de5/attachment-0001.bin>