[clang] da06606 - [clang][analyzer] No new nodes when bug is detected in StdLibraryFunctionsChecker.

[Balázs Kéri via cfe-commits]

Author: Balázs Kéri
Date: 2022-12-14T09:51:43+01:00
New Revision: da0660691f74b0350dee8e15f4ac942457e397e4

URL: https://github.com/llvm/llvm-project/commit/da0660691f74b0350dee8e15f4ac942457e397e4
DIFF: https://github.com/llvm/llvm-project/commit/da0660691f74b0350dee8e15f4ac942457e397e4.diff

LOG: [clang][analyzer] No new nodes when bug is detected in StdLibraryFunctionsChecker.

The checker applies constraints in a sequence and adds new nodes for these states.
If a constraint violation is found this sequence should be stopped with a sink
(error) node. Instead the `generateErrorNode` did add a new error node as a new
branch that is parallel to the other node sequence, the other branch was not
stopped and analysis was continuing on that invalid branch.
To add an error node after any previous node a new version of `generateErrorNode`
is needed, this function is added here and used by `StdLibraryFunctionsChecker`.
The added test executes a situation where the checker adds a number of
constraints before it finds a constraint violation.

Reviewed By: NoQ

Differential Revision: https://reviews.llvm.org/D137722

Added: 
    

Modified: 
    clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
    clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
    clang/test/Analysis/std-c-library-functions-arg-constraints.c

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
index 960131cfc396a..a40d883a15ecf 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
@@ -211,6 +211,22 @@ class CheckerContext {
                        (Tag ? Tag : Location.getTag()));
   }
 
+  /// Generate a transition to a node that will be used to report
+  /// an error. This node will be a sink. That is, it will stop exploration of
+  /// the given path.
+  ///
+  /// @param State The state of the generated node.
+  /// @param Pred The transition will be generated from the specified Pred node
+  ///             to the newly generated node.
+  /// @param Tag The tag to uniquely identify the creation site. If null,
+  ///        the default tag for the checker will be used.
+  ExplodedNode *generateErrorNode(ProgramStateRef State,
+                                  ExplodedNode *Pred,
+                                  const ProgramPointTag *Tag = nullptr) {
+    return generateSink(State, Pred,
+                       (Tag ? Tag : Location.getTag()));
+  }
+
   /// Generate a transition to a node that will be used to report
   /// an error. This node will not be a sink. That is, exploration will
   /// continue along this path.

diff  --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
index 2535bf5a368f1..04e8048352baa 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -951,7 +951,7 @@ void StdLibraryFunctionsChecker::checkPreCall(const CallEvent &Call,
         Constraint->negate()->apply(NewState, Call, Summary, C);
     // The argument constraint is not satisfied.
     if (FailureSt && !SuccessSt) {
-      if (ExplodedNode *N = C.generateErrorNode(NewState))
+      if (ExplodedNode *N = C.generateErrorNode(NewState, NewNode))
         reportBug(Call, N, Constraint.get(), Summary, C);
       break;
     }

diff  --git a/clang/test/Analysis/std-c-library-functions-arg-constraints.c b/clang/test/Analysis/std-c-library-functions-arg-constraints.c
index b5f1df8877943..1e9165321232d 100644
--- a/clang/test/Analysis/std-c-library-functions-arg-constraints.c
+++ b/clang/test/Analysis/std-c-library-functions-arg-constraints.c
@@ -20,6 +20,7 @@
 // RUN:   -verify=bugpath
 
 void clang_analyzer_eval(int);
+void clang_analyzer_warnIfReached();
 
 int glob;
 
@@ -215,6 +216,20 @@ void test_notnull_symbolic2(FILE *fp, int *buf) {
     // bugpath-note{{}} \
     // bugpath-note{{Function argument constraint is not satisfied}}
 }
+void test_no_node_after_bug(FILE *fp, size_t size, size_t n, void *buf) {
+  if (fp) // \
+  // bugpath-note{{Assuming 'fp' is null}} \
+  // bugpath-note{{Taking false branch}}
+    return;
+  size_t ret = fread(buf, size, n, fp); // \
+  // report-warning{{Function argument constraint is not satisfied}} \
+  // report-note{{}} \
+  // bugpath-warning{{Function argument constraint is not satisfied}} \
+  // bugpath-note{{}} \
+  // bugpath-note{{Function argument constraint is not satisfied}}
+  clang_analyzer_warnIfReached(); // not reachable
+}
+
 typedef __WCHAR_TYPE__ wchar_t;
 // This is one test case for the ARR38-C SEI-CERT rule.
 void ARR38_C_F(FILE *file) {