[analyzer] [MallocChecker] Fix Store modification in checkPreCall

[pskrgag]

This is small follow-up for #106081

While trying to add sanity check for Enviroment and Store being consistent during checkPostCall and checkPreCall I found out that MallocChecker still violates that rule.

The problem lies in FreeMemAux, which invalidates freed region while being called from preGetdelim. This invalidation was added to prevent "use of uninitialized memory" for malloc and friends while unix.Malloc is disabled.

Fix adds another bool flag to FreeMemAux to control invalidation from call-site and set it to false in preGetdelim

[llvmbot]

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-static-analyzer-1

Author: Pavel Skripkin (pskrgag)

Changes

This is small follow-up for #106081

While trying to add sanity check for Enviroment and Store being consistent during checkPostCall and checkPreCall I found out that MallocChecker still violates that rule.

The problem lies in FreeMemAux, which invalidates freed region while being called from preGetdelim. This invalidation was added to prevent "use of uninitialized memory" for malloc and friends while unix.Malloc is disabled.

Fix adds another bool flag to FreeMemAux to control invalidation from call-site and set it to false in preGetdelim

---

Full diff: https://github.com/llvm/llvm-project/pull/109337.diff

2 Files Affected:

• (modified) clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp (+30-25)
• (modified) clang/test/Analysis/NewDelete-intersections.mm (+9)

diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index 81ec8e1b516986..7a265d3bbcda47 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -668,7 +668,8 @@ class MallocChecker
   [[nodiscard]] ProgramStateRef
   FreeMemAux(CheckerContext &C, const CallEvent &Call, ProgramStateRef State,
              unsigned Num, bool Hold, bool &IsKnownToBeAllocated,
-             AllocationFamily Family, bool ReturnsNullOnFailure = false) const;
+             AllocationFamily Family, bool Invalidate = true,
+             bool ReturnsNullOnFailure = false) const;
 
   /// Models memory deallocation.
   ///
@@ -694,7 +695,8 @@ class MallocChecker
   [[nodiscard]] ProgramStateRef
   FreeMemAux(CheckerContext &C, const Expr *ArgExpr, const CallEvent &Call,
              ProgramStateRef State, bool Hold, bool &IsKnownToBeAllocated,
-             AllocationFamily Family, bool ReturnsNullOnFailure = false,
+             AllocationFamily Family, bool Invalidate = true,
+             bool ReturnsNullOnFailure = false,
              std::optional<SVal> ArgValOpt = {}) const;
 
   // TODO: Needs some refactoring, as all other deallocation modeling
@@ -1474,9 +1476,10 @@ void MallocChecker::preGetdelim(ProgramStateRef State, const CallEvent &Call,
   // We do not need this value here, as FreeMemAux will take care
   // of reporting any violation of the preconditions.
   bool IsKnownToBeAllocated = false;
-  State = FreeMemAux(C, Call.getArgExpr(0), Call, State, false,
-                     IsKnownToBeAllocated, AllocationFamily(AF_Malloc), false,
-                     LinePtr);
+  State =
+      FreeMemAux(C, Call.getArgExpr(0), Call, State, false,
+                 IsKnownToBeAllocated, AllocationFamily(AF_Malloc),
+                 /*Invalidate=*/false, /*ReturnsNullOnFailure=*/false, LinePtr);
   if (State)
     C.addTransition(State);
 }
@@ -1793,6 +1796,7 @@ void MallocChecker::checkPostObjCMessage(const ObjCMethodCall &Call,
   ProgramStateRef State = FreeMemAux(C, Call.getArgExpr(0), Call, C.getState(),
                                      /*Hold=*/true, IsKnownToBeAllocatedMemory,
                                      AllocationFamily(AF_Malloc),
+                                     /*Invalidate=*/false,
                                      /*ReturnsNullOnFailure=*/true);
 
   C.addTransition(State);
@@ -1986,12 +1990,11 @@ ProgramStateRef MallocChecker::FreeMemAttr(CheckerContext &C,
   return State;
 }
 
-ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C,
-                                          const CallEvent &Call,
-                                          ProgramStateRef State, unsigned Num,
-                                          bool Hold, bool &IsKnownToBeAllocated,
-                                          AllocationFamily Family,
-                                          bool ReturnsNullOnFailure) const {
+ProgramStateRef
+MallocChecker::FreeMemAux(CheckerContext &C, const CallEvent &Call,
+                          ProgramStateRef State, unsigned Num, bool Hold,
+                          bool &IsKnownToBeAllocated, AllocationFamily Family,
+                          bool Invalidate, bool ReturnsNullOnFailure) const {
   if (!State)
     return nullptr;
 
@@ -1999,7 +2002,8 @@ ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C,
     return nullptr;
 
   return FreeMemAux(C, Call.getArgExpr(Num), Call, State, Hold,
-                    IsKnownToBeAllocated, Family, ReturnsNullOnFailure);
+                    IsKnownToBeAllocated, Family, Invalidate,
+                    ReturnsNullOnFailure);
 }
 
 /// Checks if the previous call to free on the given symbol failed - if free
@@ -2134,12 +2138,11 @@ static void printExpectedDeallocName(raw_ostream &os, AllocationFamily Family) {
   }
 }
 
-ProgramStateRef
-MallocChecker::FreeMemAux(CheckerContext &C, const Expr *ArgExpr,
-                          const CallEvent &Call, ProgramStateRef State,
-                          bool Hold, bool &IsKnownToBeAllocated,
-                          AllocationFamily Family, bool ReturnsNullOnFailure,
-                          std::optional<SVal> ArgValOpt) const {
+ProgramStateRef MallocChecker::FreeMemAux(
+    CheckerContext &C, const Expr *ArgExpr, const CallEvent &Call,
+    ProgramStateRef State, bool Hold, bool &IsKnownToBeAllocated,
+    AllocationFamily Family, bool Invalidate, bool ReturnsNullOnFailure,
+    std::optional<SVal> ArgValOpt) const {
 
   if (!State)
     return nullptr;
@@ -2299,13 +2302,15 @@ MallocChecker::FreeMemAux(CheckerContext &C, const Expr *ArgExpr,
   // that.
   assert(!RsBase || (RsBase && RsBase->getAllocationFamily() == Family));
 
-  // Assume that after memory is freed, it contains unknown values. This
-  // conforts languages standards, since reading from freed memory is considered
-  // UB and may result in arbitrary value.
-  State = State->invalidateRegions({location}, Call.getOriginExpr(),
-                                   C.blockCount(), C.getLocationContext(),
-                                   /*CausesPointerEscape=*/false,
-                                   /*InvalidatedSymbols=*/nullptr);
+  if (Invalidate) {
+    // Assume that after memory is freed, it contains unknown values. This
+    // conforts languages standards, since reading from freed memory is
+    // considered UB and may result in arbitrary value.
+    State = State->invalidateRegions({location}, Call.getOriginExpr(),
+                                     C.blockCount(), C.getLocationContext(),
+                                     /*CausesPointerEscape=*/false,
+                                     /*InvalidatedSymbols=*/nullptr);
+  }
 
   // Normal free.
   if (Hold)
diff --git a/clang/test/Analysis/NewDelete-intersections.mm b/clang/test/Analysis/NewDelete-intersections.mm
index e897f48b839620..29f4d35d868b2e 100644
--- a/clang/test/Analysis/NewDelete-intersections.mm
+++ b/clang/test/Analysis/NewDelete-intersections.mm
@@ -17,6 +17,7 @@
 
 #include "Inputs/system-header-simulator-cxx.h"
 #include "Inputs/system-header-simulator-objc.h"
+#include "Inputs/system-header-simulator.h"
 
 typedef __typeof__(sizeof(int)) size_t;
 extern "C" void *malloc(size_t);
@@ -86,3 +87,11 @@ void testStandardPlacementNewAfterDelete() {
   delete p;
   p = new (p) int; // newdelete-warning{{Use of memory after it is freed}}
 }
+
+void testGetLine(FILE *f) {
+  char *buffer = (char *)malloc(10);
+  char *old = buffer;
+  size_t n = 0;
+  getline(&buffer, &n, f);
+  int a = *old; // no-warn
+}