https://github.com/pskrgag created
https://github.com/llvm/llvm-project/pull/109337
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`
>From 2645cf0333590c6d30b93958494e6b4f60b423c4 Mon Sep 17 00:00:00 2001
From: Pavel Skripkin <paskrip...@gmail.com>
Date: Fri, 20 Sep 2024 00:21:03 +0300
Subject: [PATCH 1/2] make MallocChecker not modify state in checkPreCall
---
.../StaticAnalyzer/Checkers/MallocChecker.cpp | 55 ++++++++++---------
1 file changed, 30 insertions(+), 25 deletions(-)
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)
>From 229695eb6586d4164957058b480dd840f5e4db2f Mon Sep 17 00:00:00 2001
From: Pavel Skripkin <paskrip...@gmail.com>
Date: Fri, 20 Sep 2024 00:32:48 +0300
Subject: [PATCH 2/2] add test that no new warnings introduced for getline
---
clang/test/Analysis/NewDelete-intersections.mm | 9 +++++++++
1 file changed, 9 insertions(+)
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
+}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
