[clang][analyzer] Model more getline/getdelim pre and postconditions

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"
@@ -113,8 +112,7 @@ class OperatorKind {
 OperatorKind operationKindFromOverloadedOperator(OverloadedOperatorKind OOK,
                                                  bool IsBinary);
 
-std::optional<DefinedSVal> getPointeeDefVal(SVal PtrSVal,
-                                            ProgramStateRef State);
+std::optional<SVal> getPointeeVal(SVal PtrSVal, ProgramStateRef State);
 
 } // namespace ento
 

clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

@@ -1441,7 +1441,7 @@ void MallocChecker::preGetdelim(const CallEvent &Call,
     return;
 
   ProgramStateRef State = C.getState();
-  const auto LinePtr = getPointeeDefVal(Call.getArgSVal(0), State);
+  const auto LinePtr = getPointeeVal(Call.getArgSVal(0), State);
   if (!LinePtr)
     return;
 
@@ -1470,8 +1470,10 @@ void MallocChecker::checkGetdelim(const CallEvent &Call,
 
   SValBuilder &SVB = C.getSValBuilder();
 
-  const auto LinePtr = getPointeeDefVal(Call.getArgSVal(0), State);
-  const auto Size = getPointeeDefVal(Call.getArgSVal(1), State);
+  const auto LinePtr =
+      getPointeeVal(Call.getArgSVal(0), State)->getAs<DefinedSVal>();
+  const auto Size =
+      getPointeeVal(Call.getArgSVal(1), State)->getAs<DefinedSVal>();
   if (!LinePtr || !Size || !LinePtr->getAsRegion())
     return;
 

clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp

@@ -1200,10 +1200,25 @@ void StreamChecker::evalGetdelim(const FnDescription *Desc,
 
     // Add transition for the successful state.
     NonLoc RetVal = makeRetVal(C, E.CE).castAs<NonLoc>();
-    ProgramStateRef StateNotFailed =
-        State->BindExpr(E.CE, C.getLocationContext(), RetVal);
+    ProgramStateRef StateNotFailed = E.bindReturnValue(State, C, RetVal);
     StateNotFailed =
         E.assumeBinOpNN(StateNotFailed, BO_GE, RetVal, E.getZeroVal(Call));
+
+    // On success, a buffer is allocated.
+    auto NewLinePtr = getPointeeVal(Call.getArgSVal(0), State);
+    if (NewLinePtr && isa<DefinedOrUnknownSVal>(*NewLinePtr))
+      StateNotFailed = StateNotFailed->assume(
+          NewLinePtr->castAs<DefinedOrUnknownSVal>(), true);
+
+    // 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'.
+    SVal SizePtrSval = Call.getArgSVal(1);
+    auto NVal = getPointeeVal(SizePtrSval, State);
+    if (NVal && isa<NonLoc>(*NVal)) {
+      StateNotFailed = E.assumeBinOpNN(StateNotFailed, BO_GT,
+                                       NVal->castAs<NonLoc>(), RetVal);
+      StateNotFailed = E.bindReturnValue(StateNotFailed, C, RetVal);
+    }
     if (!StateNotFailed)
       return;
     C.addTransition(StateNotFailed);
@@ -1217,6 +1232,10 @@ 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 = getPointeeVal(Call.getArgSVal(0), State))
+    StateFailed = StateFailed->bindLoc(*NewLinePtr, UndefinedVal(),
+                                       C.getLocationContext());
   C.addTransition(StateFailed, E.getFailureNoteTag(this, C));
 }
 

clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp

@@ -17,8 +17,10 @@
 #include "clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
@@ -41,25 +43,38 @@ enum class OpenVariant {
 namespace {
 
 class UnixAPIMisuseChecker
-    : public Checker<check::PreStmt<CallExpr>,
-                     check::ASTDecl<TranslationUnitDecl>> {
+    : public Checker<check::PreCall, check::ASTDecl<TranslationUnitDecl>> {
   const BugType BT_open{this, "Improper use of 'open'", categories::UnixAPI};
+  const BugType BT_getline{this, "Improper use of getdelim",
+                           categories::UnixAPI};
   const BugType BT_pthreadOnce{this, "Improper use of 'pthread_once'",
                                categories::UnixAPI};
+  const BugType BT_ArgumentNull{this, "NULL pointer", categories::UnixAPI};
   mutable std::optional<uint64_t> Val_O_CREAT;
 
+  ProgramStateRef
+  EnsurePtrNotNull(SVal PtrVal, const Expr *PtrExpr, CheckerContext &C,
+                   ProgramStateRef State, const StringRef PtrDescr,
+                   std::optional<std::reference_wrapper<const BugType>> BT =
+                       std::nullopt) const;
+
+  ProgramStateRef EnsureGetdelimBufferAndSizeCorrect(
+      SVal LinePtrPtrSVal, SVal SizePtrSVal, const Expr *LinePtrPtrExpr,
+      const Expr *SizePtrExpr, CheckerContext &C, ProgramStateRef State) const;
+
 public:
   void checkASTDecl(const TranslationUnitDecl *TU, AnalysisManager &Mgr,
                     BugReporter &BR) const;
 
-  void checkPreStmt(const CallExpr *CE, CheckerContext &C) const;
+  void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
 
-  void CheckOpen(CheckerContext &C, const CallExpr *CE) const;
-  void CheckOpenAt(CheckerContext &C, const CallExpr *CE) const;
-  void CheckPthreadOnce(CheckerContext &C, const CallExpr *CE) const;
+  void CheckOpen(CheckerContext &C, const CallEvent &Call) const;
+  void CheckOpenAt(CheckerContext &C, const CallEvent &Call) const;
+  void CheckGetDelim(CheckerContext &C, const CallEvent &Call) const;
+  void CheckPthreadOnce(CheckerContext &C, const CallEvent &Call) const;
 
-  void CheckOpenVariant(CheckerContext &C,
-                        const CallExpr *CE, OpenVariant Variant) const;
+  void CheckOpenVariant(CheckerContext &C, const CallEvent &Call,
+                        OpenVariant Variant) const;
 
   void ReportOpenBug(CheckerContext &C, ProgramStateRef State, const char *Msg,
                      SourceRange SR) const;
@@ -95,6 +110,30 @@ class UnixAPIPortabilityChecker : public Checker< check::PreStmt<CallExpr> > {
 
 } // end anonymous namespace
 
+ProgramStateRef UnixAPIMisuseChecker::EnsurePtrNotNull(
+    SVal PtrVal, const Expr *PtrExpr, CheckerContext &C, ProgramStateRef State,
+    const StringRef PtrDescr,
+    std::optional<std::reference_wrapper<const BugType>> BT) const {
+  const auto Ptr = PtrVal.getAs<DefinedSVal>();
+  if (!Ptr)
+    return State;
+
+  const auto [PtrNotNull, PtrNull] = State->assume(*Ptr);
+  if (!PtrNotNull && PtrNull) {
+    if (ExplodedNode *N = C.generateErrorNode(PtrNull)) {
+      auto R = std::make_unique<PathSensitiveBugReport>(
+          BT.value_or(std::cref(BT_ArgumentNull)),
+          (PtrDescr + " pointer might be NULL.").str(), N);
+      if (PtrExpr)
+        bugreporter::trackExpressionValue(N, PtrExpr, *R);
+      C.emitReport(std::move(R));
+    }
+    return nullptr;
+  }
+
+  return PtrNotNull;
+}
+
 void UnixAPIMisuseChecker::checkASTDecl(const TranslationUnitDecl *TU,
                                         AnalysisManager &Mgr,
                                         BugReporter &) const {
@@ -113,9 +152,9 @@ void UnixAPIMisuseChecker::checkASTDecl(const TranslationUnitDecl *TU,
 // "open" (man 2 open)
 //===----------------------------------------------------------------------===/
 
-void UnixAPIMisuseChecker::checkPreStmt(const CallExpr *CE,
+void UnixAPIMisuseChecker::checkPreCall(const CallEvent &Call,
                                         CheckerContext &C) const {
-  const FunctionDecl *FD = C.getCalleeDecl(CE);
+  const FunctionDecl *FD = dyn_cast_if_present<FunctionDecl>(Call.getDecl());
   if (!FD || FD->getKind() != Decl::Function)
     return;
 
@@ -130,13 +169,16 @@ void UnixAPIMisuseChecker::checkPreStmt(const CallExpr *CE,
     return;
 
   if (FName == "open")
-    CheckOpen(C, CE);
+    CheckOpen(C, Call);
 
   else if (FName == "openat")
-    CheckOpenAt(C, CE);
+    CheckOpenAt(C, Call);
 
   else if (FName == "pthread_once")
-    CheckPthreadOnce(C, CE);
+    CheckPthreadOnce(C, Call);
+
+  else if (is_contained({"getdelim", "getline"}, FName))
+    CheckGetDelim(C, Call);
 }
 void UnixAPIMisuseChecker::ReportOpenBug(CheckerContext &C,
                                          ProgramStateRef State,
@@ -152,17 +194,17 @@ void UnixAPIMisuseChecker::ReportOpenBug(CheckerContext &C,
 }
 
 void UnixAPIMisuseChecker::CheckOpen(CheckerContext &C,
-                                     const CallExpr *CE) const {
-  CheckOpenVariant(C, CE, OpenVariant::Open);
+                                     const CallEvent &Call) const {
+  CheckOpenVariant(C, Call, OpenVariant::Open);
 }
 
 void UnixAPIMisuseChecker::CheckOpenAt(CheckerContext &C,
-                                       const CallExpr *CE) const {
-  CheckOpenVariant(C, CE, OpenVariant::OpenAt);
+                                       const CallEvent &Call) const {
+  CheckOpenVariant(C, Call, OpenVariant::OpenAt);
 }
 
 void UnixAPIMisuseChecker::CheckOpenVariant(CheckerContext &C,
-                                            const CallExpr *CE,
+                                            const CallEvent &Call,
                                             OpenVariant Variant) const {
   // The index of the argument taking the flags open flags (O_RDONLY,
   // O_WRONLY, O_CREAT, etc.),
@@ -191,11 +233,11 @@ void UnixAPIMisuseChecker::CheckOpenVariant(CheckerContext &C,
 
   ProgramStateRef state = C.getState();
 
-  if (CE->getNumArgs() < MinArgCount) {
+  if (Call.getNumArgs() < MinArgCount) {
     // The frontend should issue a warning for this case. Just return.
     return;
-  } else if (CE->getNumArgs() == MaxArgCount) {
-    const Expr *Arg = CE->getArg(CreateModeArgIndex);
+  } else if (Call.getNumArgs() == MaxArgCount) {
+    const Expr *Arg = Call.getArgExpr(CreateModeArgIndex);
     QualType QT = Arg->getType();
     if (!QT->isIntegerType()) {
       SmallString<256> SBuf;
@@ -209,15 +251,14 @@ void UnixAPIMisuseChecker::CheckOpenVariant(CheckerContext &C,
                     Arg->getSourceRange());
       return;
     }
-  } else if (CE->getNumArgs() > MaxArgCount) {
+  } else if (Call.getNumArgs() > MaxArgCount) {
     SmallString<256> SBuf;
     llvm::raw_svector_ostream OS(SBuf);
     OS << "Call to '" << VariantName << "' with more than " << MaxArgCount
        << " arguments";
 
-    ReportOpenBug(C, state,
-                  SBuf.c_str(),
-                  CE->getArg(MaxArgCount)->getSourceRange());
+    ReportOpenBug(C, state, SBuf.c_str(),
+                  Call.getArgExpr(MaxArgCount)->getSourceRange());
     return;
   }
 
@@ -226,8 +267,8 @@ void UnixAPIMisuseChecker::CheckOpenVariant(CheckerContext &C,
   }
 
   // Now check if oflags has O_CREAT set.
-  const Expr *oflagsEx = CE->getArg(FlagsArgIndex);
-  const SVal V = C.getSVal(oflagsEx);
+  const Expr *oflagsEx = Call.getArgExpr(FlagsArgIndex);
+  const SVal V = Call.getArgSVal(FlagsArgIndex);
   if (!isa<NonLoc>(V)) {
     // The case where 'V' can be a location can only be due to a bad header,
     // so in this case bail out.
@@ -253,7 +294,7 @@ void UnixAPIMisuseChecker::CheckOpenVariant(CheckerContext &C,
   if (!(trueState && !falseState))
     return;
 
-  if (CE->getNumArgs() < MaxArgCount) {
+  if (Call.getNumArgs() < MaxArgCount) {
     SmallString<256> SBuf;
     llvm::raw_svector_ostream OS(SBuf);
     OS << "Call to '" << VariantName << "' requires a "
@@ -266,23 +307,110 @@ void UnixAPIMisuseChecker::CheckOpenVariant(CheckerContext &C,
   }
 }
 
+//===----------------------------------------------------------------------===//
+// getdelim and getline
+//===----------------------------------------------------------------------===//
+
+ProgramStateRef UnixAPIMisuseChecker::EnsureGetdelimBufferAndSizeCorrect(
+    SVal LinePtrPtrSVal, SVal SizePtrSVal, const Expr *LinePtrPtrExpr,
+    const Expr *SizePtrExpr, CheckerContext &C, ProgramStateRef State) const {
+  static constexpr llvm::StringLiteral SizeGreaterThanBufferSize =
+      "The buffer from the first argument is smaller than the size "
+      "specified by the second parameter";
+  static constexpr llvm::StringLiteral SizeUndef =
+      "The buffer from the first argument is not NULL, but the size specified "
+      "by the second parameter is undefined.";
+
+  auto EmitBugReport = [this, &C, SizePtrExpr, LinePtrPtrExpr](
+                           ProgramStateRef BugState, StringRef ErrMsg) {
+    if (ExplodedNode *N = C.generateErrorNode(BugState)) {
+      auto R = std::make_unique<PathSensitiveBugReport>(BT_getline, ErrMsg, N);
+      bugreporter::trackExpressionValue(N, SizePtrExpr, *R);
+      bugreporter::trackExpressionValue(N, LinePtrPtrExpr, *R);
+      C.emitReport(std::move(R));
+    }
+  };
+
+  // We have a pointer to a pointer to the buffer, and a pointer to the size.
+  // We want what they point at.
+  auto LinePtrSVal = getPointeeVal(LinePtrPtrSVal, State)->getAs<DefinedSVal>();
+  auto NSVal = getPointeeVal(SizePtrSVal, State);
+  if (!LinePtrSVal || !NSVal || NSVal->isUnknown())
+    return nullptr;
+
+  assert(LinePtrPtrExpr && SizePtrExpr);
+
+  const auto [LinePtrNotNull, LinePtrNull] = State->assume(*LinePtrSVal);
+  if (LinePtrNotNull && !LinePtrNull) {
+    // If `*lineptr` is not null, but `*n` is undefined, there is UB.
+    if (NSVal->isUndef()) {
+      EmitBugReport(LinePtrNotNull, SizeUndef);
+      return nullptr;
+    }
+
+    // If it is defined, and known, its size must be less than or equal to
+    // the buffer size.
+    auto NDefSVal = NSVal->getAs<DefinedSVal>();
+    auto &SVB = C.getSValBuilder();
+    auto LineBufSize =
+        getDynamicExtent(LinePtrNotNull, LinePtrSVal->getAsRegion(), SVB);
+    auto LineBufSizeGtN = SVB.evalBinOp(LinePtrNotNull, BO_GE, LineBufSize,
+                                        *NDefSVal, SVB.getConditionType())
+                              .getAs<DefinedOrUnknownSVal>();
+    if (!LineBufSizeGtN)
+      return LinePtrNotNull;
+    if (auto LineBufSizeOk = LinePtrNotNull->assume(*LineBufSizeGtN, true))
+      return LineBufSizeOk;
+
+    EmitBugReport(LinePtrNotNull, SizeGreaterThanBufferSize);
+    return nullptr;
+  }
+  return State;
+}
+
+void UnixAPIMisuseChecker::CheckGetDelim(CheckerContext &C,
+                                         const CallEvent &Call) const {
+  ProgramStateRef State = C.getState();
+
+  // The parameter `n` must not be NULL.
+  SVal SizePtrSval = Call.getArgSVal(1);
+  State = EnsurePtrNotNull(SizePtrSval, Call.getArgExpr(1), C, State, "Size");
+  if (!State)
+    return;
+
+  // The parameter `lineptr` must not be NULL.
+  SVal LinePtrPtrSVal = Call.getArgSVal(0);
+  State =
+      EnsurePtrNotNull(LinePtrPtrSVal, Call.getArgExpr(0), C, State, "Line");
+  if (!State)
+    return;
+
+  State = EnsureGetdelimBufferAndSizeCorrect(LinePtrPtrSVal, SizePtrSval,
+                                             Call.getArgExpr(0),
+                                             Call.getArgExpr(1), C, State);
+  if (!State)
+    return;
+
+  C.addTransition(State);
+}
+
 //===----------------------------------------------------------------------===//
 // pthread_once
 //===----------------------------------------------------------------------===//
 
 void UnixAPIMisuseChecker::CheckPthreadOnce(CheckerContext &C,
-                                      const CallExpr *CE) const {
+                                            const CallEvent &Call) const {
 
   // This is similar to 'CheckDispatchOnce' in the MacOSXAPIChecker.
   // They can possibly be refactored.
 
-  if (CE->getNumArgs() < 1)
+  if (Call.getNumArgs() < 1)
     return;
 
   // Check if the first argument is stack allocated.  If so, issue a warning
   // because that's likely to be bad news.
   ProgramStateRef state = C.getState();
-  const MemRegion *R = C.getSVal(CE->getArg(0)).getAsRegion();
+  const MemRegion *R = Call.getArgSVal(0).getAsRegion();
   if (!R || !isa<StackSpaceRegion>(R->getMemorySpace()))
     return;
 
@@ -304,7 +432,7 @@ void UnixAPIMisuseChecker::CheckPthreadOnce(CheckerContext &C,
 
   auto report =
       std::make_unique<PathSensitiveBugReport>(BT_pthreadOnce, os.str(), N);
-  report->addRange(CE->getArg(0)->getSourceRange());
+  report->addRange(Call.getArgExpr(0)->getSourceRange());
   C.emitReport(std::move(report));
 }