[clang] [clang][analyzer] Model more getline/getdelim pre and postconditions (PR #83027)

Alejandro Álvarez Ayllón via cfe-commits cfe-commits at lists.llvm.org
Thu Mar 7 04:49:22 PST 2024


https://github.com/alejandro-alvarez-sonarsource updated https://github.com/llvm/llvm-project/pull/83027

>From 5c919832f9176d4b1af1312a4ee7cf30b788958a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
 <alejandro.alvarez at sonarsource.com>
Date: Wed, 21 Feb 2024 14:46:01 +0100
Subject: [PATCH 1/4] [clang][analyzer] Model getline/getdelim preconditions

1. lineptr, n and stream can not be NULL
2. if *lineptr is NULL, *n must be 0
---
 .../StaticAnalyzer/Checkers/StreamChecker.cpp | 155 ++++++++-
 clang/test/Analysis/getline-stream.c          | 327 ++++++++++++++++++
 2 files changed, 480 insertions(+), 2 deletions(-)
 create mode 100644 clang/test/Analysis/getline-stream.c

diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 2ec47bf55df76b..bacac7613f880c 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -21,6 +21,7 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/Sequence.h"
 #include <functional>
 #include <optional>
@@ -234,6 +235,9 @@ class StreamChecker : public Checker<check::PreCall, eval::Call,
   BugType BT_StreamEof{this, "Stream already in EOF", "Stream handling error"};
   BugType BT_ResourceLeak{this, "Resource leak", "Stream handling error",
                           /*SuppressOnSink =*/true};
+  BugType BT_SizeNull{this, "NULL size pointer", "Stream handling error"};
+  BugType BT_SizeNotZero{this, "NULL line pointer and size not zero",
+                         "Stream handling error"};
 
 public:
   void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
@@ -346,10 +350,10 @@ class StreamChecker : public Checker<check::PreCall, eval::Call,
        {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, false),
         std::bind(&StreamChecker::evalUngetc, _1, _2, _3, _4), 1}},
       {{{"getdelim"}, 4},
-       {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, true),
+       {std::bind(&StreamChecker::preGetdelim, _1, _2, _3, _4),
         std::bind(&StreamChecker::evalGetdelim, _1, _2, _3, _4), 3}},
       {{{"getline"}, 3},
-       {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, true),
+       {std::bind(&StreamChecker::preGetdelim, _1, _2, _3, _4),
         std::bind(&StreamChecker::evalGetdelim, _1, _2, _3, _4), 2}},
       {{{"fseek"}, 3},
        {&StreamChecker::preFseek, &StreamChecker::evalFseek, 0}},
@@ -436,6 +440,9 @@ class StreamChecker : public Checker<check::PreCall, eval::Call,
   void evalUngetc(const FnDescription *Desc, const CallEvent &Call,
                   CheckerContext &C) const;
 
+  void preGetdelim(const FnDescription *Desc, const CallEvent &Call,
+                   CheckerContext &C) const;
+
   void evalGetdelim(const FnDescription *Desc, const CallEvent &Call,
                     CheckerContext &C) const;
 
@@ -510,6 +517,14 @@ class StreamChecker : public Checker<check::PreCall, eval::Call,
   ProgramStateRef ensureFseekWhenceCorrect(SVal WhenceVal, CheckerContext &C,
                                            ProgramStateRef State) const;
 
+  ProgramStateRef ensurePtrNotNull(SVal PtrVal, const Expr *PtrExpr,
+                                   CheckerContext &C, ProgramStateRef State,
+                                   const StringRef PtrDescr) const;
+  ProgramStateRef
+  ensureSizeZeroIfLineNull(SVal LinePtrPtrSVal, SVal SizePtrSVal,
+                           const Expr *LinePtrPtrExpr, const Expr *SizePtrExpr,
+                           CheckerContext &C, ProgramStateRef State) const;
+
   /// Generate warning about stream in EOF state.
   /// There will be always a state transition into the passed State,
   /// by the new non-fatal error node or (if failed) a normal transition,
@@ -1158,6 +1173,123 @@ void StreamChecker::evalUngetc(const FnDescription *Desc, const CallEvent &Call,
   C.addTransition(StateFailed);
 }
 
+ProgramStateRef
+StreamChecker::ensurePtrNotNull(SVal PtrVal, const Expr *PtrExpr,
+                                CheckerContext &C, ProgramStateRef State,
+                                const StringRef PtrDescr) const {
+  const auto Ptr = PtrVal.getAs<DefinedSVal>();
+  if (!Ptr)
+    return nullptr;
+
+  assert(PtrExpr && "Expected an argument");
+
+  const auto [PtrNotNull, PtrNull] = State->assume(*Ptr);
+  if (!PtrNotNull && PtrNull) {
+    if (ExplodedNode *N = C.generateErrorNode(PtrNull)) {
+      SmallString<256> buf;
+      llvm::raw_svector_ostream os(buf);
+      os << PtrDescr << " pointer might be NULL.";
+
+      auto R = std::make_unique<PathSensitiveBugReport>(BT_SizeNull, buf, N);
+      bugreporter::trackExpressionValue(N, PtrExpr, *R);
+      C.emitReport(std::move(R));
+    }
+    return nullptr;
+  }
+
+  return PtrNotNull;
+}
+
+ProgramStateRef StreamChecker::ensureSizeZeroIfLineNull(
+    SVal LinePtrPtrSVal, SVal SizePtrSVal, const Expr *LinePtrPtrExpr,
+    const Expr *SizePtrExpr, CheckerContext &C, ProgramStateRef State) const {
+  static constexpr char SizeNotZeroMsg[] =
+      "Line pointer might be null while n value is not zero";
+
+  // We have a pointer to a pointer to the buffer, and a pointer to the size.
+  // We want what they point at.
+  auto LinePtrSVal = getPointeeDefVal(LinePtrPtrSVal, State);
+  auto NSVal = getPointeeDefVal(SizePtrSVal, State);
+  if (!LinePtrSVal || !NSVal)
+    return nullptr;
+
+  assert(LinePtrPtrExpr &&
+         "Expected an argument with a pointer to a pointer to the buffer.");
+  assert(SizePtrExpr &&
+         "Expected an argument with a pointer to the buffer size.");
+
+  // If the line pointer is null, and n is > 0, there is UB.
+  const auto [LinePtrNotNull, LinePtrNull] = State->assume(*LinePtrSVal);
+  if (LinePtrNull && !LinePtrNotNull) {
+    const auto [NIsNotZero, NIsZero] = LinePtrNull->assume(*NSVal);
+    if (NIsNotZero && !NIsZero) {
+      if (ExplodedNode *N = C.generateErrorNode(NIsNotZero)) {
+        auto R = std::make_unique<PathSensitiveBugReport>(BT_SizeNotZero,
+                                                          SizeNotZeroMsg, N);
+        bugreporter::trackExpressionValue(N, SizePtrExpr, *R);
+        bugreporter::trackExpressionValue(N, LinePtrPtrExpr, *R);
+        C.emitReport(std::move(R));
+      }
+      return nullptr;
+    }
+    return NIsZero;
+  }
+  return LinePtrNotNull;
+}
+
+void StreamChecker::preGetdelim(const FnDescription *Desc,
+                                const CallEvent &Call,
+                                CheckerContext &C) const {
+  ProgramStateRef State = C.getState();
+  SVal StreamVal = getStreamArg(Desc, Call);
+
+  auto AddTransitionOnReturn = llvm::make_scope_exit([&] {
+    if (State != nullptr) {
+      C.addTransition(State);
+    }
+  });
+
+  State = ensureStreamNonNull(StreamVal, Call.getArgExpr(Desc->StreamArgNo), C,
+                              State);
+  if (!State)
+    return;
+  State = ensureStreamOpened(StreamVal, C, State);
+  if (!State)
+    return;
+  State = ensureNoFilePositionIndeterminate(StreamVal, C, State);
+  if (!State)
+    return;
+
+  // n must not be NULL
+  SVal SizePtrSval = Call.getArgSVal(1);
+  State = ensurePtrNotNull(SizePtrSval, Call.getArgExpr(1), C, State, "Size");
+  if (!State)
+    return;
+
+  // lineptr must not be NULL
+  SVal LinePtrPtrSVal = Call.getArgSVal(0);
+  State =
+      ensurePtrNotNull(LinePtrPtrSVal, Call.getArgExpr(0), C, State, "Line");
+  if (!State)
+    return;
+
+  // If lineptr points to a NULL pointer, *n must be 0
+  State =
+      ensureSizeZeroIfLineNull(LinePtrPtrSVal, SizePtrSval, Call.getArgExpr(0),
+                               Call.getArgExpr(1), C, State);
+  if (!State)
+    return;
+
+  SymbolRef Sym = StreamVal.getAsSymbol();
+  if (Sym && State->get<StreamMap>(Sym)) {
+    const StreamState *SS = State->get<StreamMap>(Sym);
+    if (SS->ErrorState & ErrorFEof)
+      reportFEofWarning(Sym, C, State);
+  } else {
+    C.addTransition(State);
+  }
+}
+
 void StreamChecker::evalGetdelim(const FnDescription *Desc,
                                  const CallEvent &Call,
                                  CheckerContext &C) const {
@@ -1183,6 +1315,20 @@ void StreamChecker::evalGetdelim(const FnDescription *Desc,
         State->BindExpr(E.CE, C.getLocationContext(), RetVal);
     StateNotFailed =
         E.assumeBinOpNN(StateNotFailed, BO_GE, RetVal, E.getZeroVal(Call));
+    // The buffer size *n must be enough to hold the whole line, and
+    // greater than the return value, since it has to account for \0
+    auto SizePtrSval = Call.getArgSVal(1);
+    auto NVal = getPointeeDefVal(SizePtrSval, State);
+    if (NVal) {
+      StateNotFailed = StateNotFailed->assume(
+          E.SVB
+              .evalBinOp(StateNotFailed, BinaryOperator::Opcode::BO_GT, *NVal,
+                         RetVal, E.SVB.getConditionType())
+              .castAs<DefinedOrUnknownSVal>(),
+          true);
+      StateNotFailed =
+          StateNotFailed->BindExpr(E.CE, C.getLocationContext(), RetVal);
+    }
     if (!StateNotFailed)
       return;
     C.addTransition(StateNotFailed);
@@ -1196,6 +1342,11 @@ void StreamChecker::evalGetdelim(const FnDescription *Desc,
       E.isStreamEof() ? ErrorFEof : ErrorFEof | ErrorFError;
   StateFailed = E.setStreamState(
       StateFailed, StreamState::getOpened(Desc, NewES, !NewES.isFEof()));
+  // On failure, the content of the buffer is undefined
+  if (auto NewLinePtr = getPointeeDefVal(Call.getArgSVal(0), State)) {
+    StateFailed = StateFailed->bindLoc(*NewLinePtr, UndefinedVal(),
+                                       C.getLocationContext());
+  }
   C.addTransition(StateFailed, E.getFailureNoteTag(this, C));
 }
 
diff --git a/clang/test/Analysis/getline-stream.c b/clang/test/Analysis/getline-stream.c
new file mode 100644
index 00000000000000..b4f389e4505f73
--- /dev/null
+++ b/clang/test/Analysis/getline-stream.c
@@ -0,0 +1,327 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.Stream,debug.ExprInspection -verify %s
+
+#include "Inputs/system-header-simulator.h"
+#include "Inputs/system-header-simulator-for-malloc.h"
+#include "Inputs/system-header-simulator-for-valist.h"
+
+void clang_analyzer_eval(int);
+void clang_analyzer_dump_int(int);
+extern void clang_analyzer_dump_ptr(void*);
+extern void clang_analyzer_warnIfReached();
+
+void test_getline_null_file() {
+  char *buffer = NULL;
+  size_t n = 0;
+  getline(&buffer, &n, NULL); // expected-warning {{Stream pointer might be NULL}}
+}
+
+void test_getline_null_lineptr() {
+  FILE *F1 = tmpfile();
+  if (!F1)
+    return;
+
+  char **buffer = NULL;
+  size_t n = 0;
+  getline(buffer, &n, F1); // expected-warning {{Line pointer might be NULL}}
+  fclose(F1);
+}
+
+void test_getline_null_size() {
+  FILE *F1 = tmpfile();
+  if (!F1)
+    return;
+  char *buffer = NULL;
+  getline(&buffer, NULL, F1); // expected-warning {{Size pointer might be NULL}}
+  fclose(F1);
+}
+
+void test_getline_null_buffer_bad_size() {
+  FILE *F1 = tmpfile();
+  if (!F1)
+    return;
+  char *buffer = NULL;
+  size_t n = 8;
+  getline(&buffer, &n, F1); // expected-warning {{Line pointer might be null while n value is not zero}}
+  fclose(F1);
+}
+
+void test_getline_null_buffer_bad_size_2(size_t n) {
+  FILE *F1 = tmpfile();
+  if (!F1)
+    return;
+  char *buffer = NULL;
+  if (n > 0) {
+    getline(&buffer, &n, F1);  // expected-warning {{Line pointer might be null while n value is not zero}}
+  }
+  fclose(F1);
+}
+
+void test_getline_null_buffer_unknown_size(size_t n) {
+  FILE *F1 = tmpfile();
+  if (!F1)
+    return;
+  char *buffer = NULL;
+
+  getline(&buffer, &n, F1);  // ok
+  fclose(F1);
+  free(buffer);
+}
+
+void test_getline_null_buffer() {
+  FILE *F1 = tmpfile();
+  if (!F1)
+    return;
+  char *buffer = NULL;
+  size_t n = 0;
+  ssize_t r = getline(&buffer, &n, F1);
+  // getline returns -1 on failure, number of char reads on success (>= 0)
+  if (r < -1) {
+    clang_analyzer_warnIfReached(); // must not happen
+  } else {
+    // The buffer could be allocated both on failure and success
+    clang_analyzer_dump_int(n);      // expected-warning {{conj_$}}
+    clang_analyzer_dump_ptr(buffer); // expected-warning {{conj_$}}
+  }
+  free(buffer);
+  fclose(F1);
+}
+
+void test_getdelim_null_file() {
+  char *buffer = NULL;
+  size_t n = 0;
+  getdelim(&buffer, &n, '\n', NULL); // expected-warning {{Stream pointer might be NULL}}
+}
+
+void test_getdelim_null_size() {
+  FILE *F1 = tmpfile();
+  if (!F1)
+    return;
+  char *buffer = NULL;
+  getdelim(&buffer, NULL, ',', F1); // expected-warning {{Size pointer might be NULL}}
+  fclose(F1);
+}
+
+void test_getdelim_null_buffer_bad_size() {
+  FILE *F1 = tmpfile();
+  if (!F1)
+    return;
+  char *buffer = NULL;
+  size_t n = 8;
+  getdelim(&buffer, &n, ';', F1); // expected-warning {{Line pointer might be null while n value is not zero}}
+  fclose(F1);
+}
+
+void test_getdelim_null_buffer_bad_size_2(size_t n) {
+  FILE *F1 = tmpfile();
+  if (!F1)
+    return;
+  char *buffer = NULL;
+  if (n > 0) {
+    getdelim(&buffer, &n, ' ', F1);  // expected-warning {{Line pointer might be null while n value is not zero}}
+  }
+  fclose(F1);
+}
+
+void test_getdelim_null_buffer_unknown_size(size_t n) {
+  FILE *F1 = tmpfile();
+  if (!F1)
+    return;
+  char *buffer = NULL;
+  getdelim(&buffer, &n, '-', F1);  // ok
+  fclose(F1);
+  free(buffer);
+}
+
+void test_getdelim_null_buffer() {
+  FILE *F1 = tmpfile();
+  if (!F1)
+    return;
+  char *buffer = NULL;
+  size_t n = 0;
+  ssize_t r = getdelim(&buffer, &n, '\r', F1);
+  // getdelim returns -1 on failure, number of char reads on success (>= 0)
+  if (r < -1) {
+    clang_analyzer_warnIfReached(); // must not happen
+  }
+  else {
+    // The buffer could be allocated both on failure and success
+    clang_analyzer_dump_int(n);      // expected-warning {{conj_$}}
+    clang_analyzer_dump_ptr(buffer); // expected-warning {{conj_$}}
+  }
+  free(buffer);
+  fclose(F1);
+}
+
+void test_getline_while() {
+  FILE *file = fopen("file.txt", "r");
+  if (file == NULL) {
+    return;
+  }
+
+  char *line = NULL;
+  size_t len = 0;
+  ssize_t read;
+
+  while ((read = getline(&line, &len, file)) != -1) {
+    printf("%s\n", line);
+  }
+
+  free(line);
+  fclose(file);
+}
+
+void test_getline_no_return_check() {
+  FILE *file = fopen("file.txt", "r");
+  if (file == NULL) {
+    return;
+  }
+
+  char *line = NULL;
+  size_t len = 0;
+  getline(&line, &len, file);
+
+  if (line[0] == '\0') {} // expected-warning {{The left operand of '==' is a garbage value}}
+
+  free(line);
+  fclose(file);
+}
+
+void test_getline_return_check() {
+  FILE *file = fopen("file.txt", "r");
+  if (file == NULL) {
+    return;
+  }
+
+  char *line = NULL;
+  size_t len = 0;
+  ssize_t r = getline(&line, &len, file);
+
+  if (r != -1) {
+    if (line[0] == '\0') {} // ok
+  }
+  free(line);
+  fclose(file);
+}
+
+void test_getline_feof_check() {
+  FILE *file = fopen("file.txt", "r");
+  if (file == NULL) {
+    return;
+  }
+
+  char *line = NULL;
+  size_t len = 0;
+  ssize_t r = getline(&line, &len, file);
+
+  if (r != -1) {
+    // success, end-of-file is not possible
+    int f = feof(file);
+    clang_analyzer_eval(f == 0); // expected-warning {{TRUE}}
+  } else {
+    // failure, end-of-file is possible, but not the only reason to fail
+    int f = feof(file);
+    clang_analyzer_eval(f == 0); // expected-warning {{TRUE}} \\
+                                   expected-warning {{FALSE}}
+  }
+  free(line);
+  fclose(file);
+}
+
+void test_getline_after_eof() {
+  FILE *file = fopen("file.txt", "r");
+  if (file == NULL) {
+    return;
+  }
+
+  size_t n = 10;
+  char *buffer = malloc(n);
+  ssize_t read = fread(buffer, n, 1, file);
+  if (!feof(file)) {
+    getline(&buffer, &n, file); // expected-warning {{File position of the stream might be 'indeterminate' after a failed operation. Can cause undefined behavior}}
+  }
+  fclose(file);
+  free(buffer);
+}
+
+void test_getline_feof() {
+  FILE *file = fopen("file.txt", "r");
+  if (file == NULL) {
+    return;
+  }
+
+  size_t n = 10;
+  char *buffer = malloc(n);
+  ssize_t read = fread(buffer, n, 1, file);
+  getline(&buffer, &n, file); // expected-warning {{File position of the stream might be 'indeterminate' after a failed operation. Can cause undefined behavior}} \\
+                                 expected-warning {{Read function called when stream is in EOF state. Function has no effect}}
+  fclose(file);
+  free(buffer);
+}
+
+void test_getline_clear_eof() {
+  FILE *file = fopen("file.txt", "r");
+  if (file == NULL) {
+    return;
+  }
+
+  size_t n = 10;
+  char *buffer = malloc(n);
+  ssize_t read = fread(buffer, n, 1, file);
+  if (feof(file)) {
+    clearerr(file);
+    getline(&buffer, &n, file); // ok
+  }
+  fclose(file);
+  free(buffer);
+}
+
+void test_getline_not_null(char **buffer, size_t *size) {
+  FILE *file = fopen("file.txt", "r");
+  if (file == NULL) {
+    return;
+  }
+
+  getline(buffer, size, file);
+  fclose(file);
+
+  if (size == NULL || buffer == NULL) {
+    clang_analyzer_warnIfReached(); // must not happen
+  }
+}
+
+void test_getline_size_0(size_t size) {
+  FILE *file = fopen("file.txt", "r");
+  if (file == NULL) {
+    return;
+  }
+
+  size_t old_size = size;
+  char *buffer = NULL;
+  ssize_t r = getline(&buffer, &size, file);
+  if (r >= 0) {
+    // Since buffer is NULL, old_size should be 0. Otherwise, there would be UB.
+    clang_analyzer_eval(old_size == 0); // expected-warning{{TRUE}}
+  }
+  fclose(file);
+  free(buffer);
+}
+
+void test_getline_ret_value() {
+  FILE *file = fopen("file.txt", "r");
+  if (file == NULL) {
+    return;
+  }
+
+  size_t n = 0;
+  char *buffer = NULL;
+  ssize_t r = getline(&buffer, &n, file);
+
+  if (r > -1) {
+    // The return value does *not* include the terminating null byte.
+    // The buffer must be large enough to include it.
+    clang_analyzer_eval(n > r); // expected-warning{{TRUE}}
+  }
+
+  fclose(file);
+  free(buffer);
+}

>From 414fd9bcbfefeca0d5f030ec759f5ddd1b2b8d78 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
 <alejandro.alvarez at sonarsource.com>
Date: Thu, 7 Mar 2024 09:55:00 +0100
Subject: [PATCH 2/4] Feedback from PR

---
 .../Core/PathSensitive/CheckerHelpers.h              |  1 -
 clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp  | 12 ++++++------
 clang/test/Analysis/getline-stream.c                 |  4 ++--
 3 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
index 60421e5437d82f..759caaf61b891c 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
@@ -15,7 +15,6 @@
 
 #include "ProgramState_Fwd.h"
 #include "SVals.h"
-
 #include "clang/AST/OperationKinds.h"
 #include "clang/AST/Stmt.h"
 #include "clang/Basic/OperatorKinds.h"
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index bacac7613f880c..92bcf66e5129db 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -1260,20 +1260,20 @@ void StreamChecker::preGetdelim(const FnDescription *Desc,
   if (!State)
     return;
 
-  // n must not be NULL
+  // The parameter `n` must not be NULL.
   SVal SizePtrSval = Call.getArgSVal(1);
   State = ensurePtrNotNull(SizePtrSval, Call.getArgExpr(1), C, State, "Size");
   if (!State)
     return;
 
-  // lineptr must not be NULL
+  // The parameter `lineptr` must not be NULL.
   SVal LinePtrPtrSVal = Call.getArgSVal(0);
   State =
       ensurePtrNotNull(LinePtrPtrSVal, Call.getArgExpr(0), C, State, "Line");
   if (!State)
     return;
 
-  // If lineptr points to a NULL pointer, *n must be 0
+  // If `lineptr` points to a NULL pointer, `*n` must be 0.
   State =
       ensureSizeZeroIfLineNull(LinePtrPtrSVal, SizePtrSval, Call.getArgExpr(0),
                                Call.getArgExpr(1), C, State);
@@ -1315,8 +1315,8 @@ void StreamChecker::evalGetdelim(const FnDescription *Desc,
         State->BindExpr(E.CE, C.getLocationContext(), RetVal);
     StateNotFailed =
         E.assumeBinOpNN(StateNotFailed, BO_GE, RetVal, E.getZeroVal(Call));
-    // The buffer size *n must be enough to hold the whole line, and
-    // greater than the return value, since it has to account for \0
+    // The buffer size `*n` must be enough to hold the whole line, and
+    // greater than the return value, since it has to account for '\0'.
     auto SizePtrSval = Call.getArgSVal(1);
     auto NVal = getPointeeDefVal(SizePtrSval, State);
     if (NVal) {
@@ -1342,7 +1342,7 @@ void StreamChecker::evalGetdelim(const FnDescription *Desc,
       E.isStreamEof() ? ErrorFEof : ErrorFEof | ErrorFError;
   StateFailed = E.setStreamState(
       StateFailed, StreamState::getOpened(Desc, NewES, !NewES.isFEof()));
-  // On failure, the content of the buffer is undefined
+  // On failure, the content of the buffer is undefined.
   if (auto NewLinePtr = getPointeeDefVal(Call.getArgSVal(0), State)) {
     StateFailed = StateFailed->bindLoc(*NewLinePtr, UndefinedVal(),
                                        C.getLocationContext());
diff --git a/clang/test/Analysis/getline-stream.c b/clang/test/Analysis/getline-stream.c
index b4f389e4505f73..8677e35eb10d97 100644
--- a/clang/test/Analysis/getline-stream.c
+++ b/clang/test/Analysis/getline-stream.c
@@ -6,8 +6,8 @@
 
 void clang_analyzer_eval(int);
 void clang_analyzer_dump_int(int);
-extern void clang_analyzer_dump_ptr(void*);
-extern void clang_analyzer_warnIfReached();
+void clang_analyzer_dump_ptr(void*);
+void clang_analyzer_warnIfReached();
 
 void test_getline_null_file() {
   char *buffer = NULL;

>From 32dd108899522c5c4c7fa2c4ae7ed7127a71c5ac Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
 <alejandro.alvarez at sonarsource.com>
Date: Thu, 7 Mar 2024 13:48:59 +0100
Subject: [PATCH 3/4] Update
 clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp

Co-authored-by: Balazs Benics <benicsbalazs at gmail.com>
---
 clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 92bcf66e5129db..0a7618c75a834f 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -1186,11 +1186,7 @@ StreamChecker::ensurePtrNotNull(SVal PtrVal, const Expr *PtrExpr,
   const auto [PtrNotNull, PtrNull] = State->assume(*Ptr);
   if (!PtrNotNull && PtrNull) {
     if (ExplodedNode *N = C.generateErrorNode(PtrNull)) {
-      SmallString<256> buf;
-      llvm::raw_svector_ostream os(buf);
-      os << PtrDescr << " pointer might be NULL.";
-
-      auto R = std::make_unique<PathSensitiveBugReport>(BT_SizeNull, buf, N);
+      auto R = std::make_unique<PathSensitiveBugReport>(BT_SizeNull, (PtrDescr + " pointer might be NULL.").str(), N);
       bugreporter::trackExpressionValue(N, PtrExpr, *R);
       C.emitReport(std::move(R));
     }

>From 253cfa84b0a0e140ce9cbdd9626ce165d24bc9e4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
 <alejandro.alvarez at sonarsource.com>
Date: Thu, 7 Mar 2024 13:49:14 +0100
Subject: [PATCH 4/4] Update
 clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp

Co-authored-by: Balazs Benics <benicsbalazs at gmail.com>
---
 clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp | 2 --
 1 file changed, 2 deletions(-)

diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 0a7618c75a834f..f028fff712bcaa 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -1281,8 +1281,6 @@ void StreamChecker::preGetdelim(const FnDescription *Desc,
     const StreamState *SS = State->get<StreamMap>(Sym);
     if (SS->ErrorState & ErrorFEof)
       reportFEofWarning(Sym, C, State);
-  } else {
-    C.addTransition(State);
   }
 }
 



More information about the cfe-commits mailing list