[analyzer] Handle builtin functions in MallocChecker #88416
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit ensures that the `CallDescription`s in `MallocChecker` are matched with the mode `CDM::CLibrary`, so: - they don't match methods or functions within user-defined namespaces; - they also match builtin variants of these functions (if any), so the checker can model `__builtin_alloca()` like `alloca()`. This change fixes llvm#81597. New tests were added to verify that `std::malloc` and `std::free` (from <cstdlib>) are modeled, but a method that's named e.g. `free` isn't confused with the memory release function. The responsibility for modeling `__builtin_alloca` and `__builtin_alloca_with_align` was moved from `BuiltinFunctionChecker` to `MallocChecker`, to avoid buggy interactions between the checkers and ensure that the builtin and non-builtin variants are handled by exactly the same logic. This change might be a step backwards for the users who don't have `unix.Malloc` enabled; but I suspect that `__builtin_alloca()` is so rare that it would be a waste of time to implement backwards compatibility for them. There were several test files that relied on `__builtin_alloca()` calls to get an `AllocaRegion`, these were modified to enable `unix.Malloc`. One of these files (cxx-uninitialized-object-ptr-ref.cpp) had some tests that relied on the fact that `malloc()` was treated as a "black box" in them, these were updated to use `calloc()` (to get initialized memory) and `free()` (to avoid memory leak reports). While I was developing this change, I found a very suspicious assert in `MallocChecker`. As it isn't blocking the goals of this commit, I just marked it with a FIXME, but I'll try to investigate and fix it in a follow-up change.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: None (NagyDonat) ChangesThis commit ensures that the
This change fixes #81597. New tests were added to verify that The responsibility for modeling This change might be a step backwards for the users who don't have There were several test files that relied on While I was developing this change, I found a very suspicious assert in Full diff: https://github.com/llvm/llvm-project/pull/88416.diff 11 Files Affected:
diff --git a/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
index 01e46fa8591c07..1a75d7b52ad6e9 100644
--- a/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
@@ -6,7 +6,11 @@
//
//===----------------------------------------------------------------------===//
//
-// This checker evaluates clang builtin functions.
+// This checker evaluates "standalone" clang builtin functions that are not
+// just special-cased variants of well-known non-builtin functions.
+// Builtin functions like __builtin_memcpy and __builtin_alloca should be
+// evaluated by the same checker that handles their non-builtin variant to
+// ensure that the two variants are handled consistently.
//
//===----------------------------------------------------------------------===//
@@ -80,25 +84,6 @@ bool BuiltinFunctionChecker::evalCall(const CallEvent &Call,
return true;
}
- case Builtin::BI__builtin_alloca_with_align:
- case Builtin::BI__builtin_alloca: {
- SValBuilder &SVB = C.getSValBuilder();
- const loc::MemRegionVal R =
- SVB.getAllocaRegionVal(CE, C.getLocationContext(), C.blockCount());
-
- // Set the extent of the region in bytes. This enables us to use the SVal
- // of the argument directly. If we saved the extent in bits, it'd be more
- // difficult to reason about values like symbol*8.
- auto Size = Call.getArgSVal(0);
- if (auto DefSize = Size.getAs<DefinedOrUnknownSVal>()) {
- // This `getAs()` is mostly paranoia, because core.CallAndMessage reports
- // undefined function arguments (unless it's disabled somehow).
- state = setDynamicExtent(state, R.getRegion(), *DefSize, SVB);
- }
- C.addTransition(state->BindExpr(CE, LCtx, R));
- return true;
- }
-
case Builtin::BI__builtin_dynamic_object_size:
case Builtin::BI__builtin_object_size:
case Builtin::BI__builtin_constant_p: {
diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index 88fb42b6625aa4..c141d6dcef9a73 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -401,10 +401,10 @@ class MallocChecker
};
const CallDescriptionMap<CheckFn> FreeingMemFnMap{
- {{{"free"}, 1}, &MallocChecker::checkFree},
- {{{"if_freenameindex"}, 1}, &MallocChecker::checkIfFreeNameIndex},
- {{{"kfree"}, 1}, &MallocChecker::checkFree},
- {{{"g_free"}, 1}, &MallocChecker::checkFree},
+ {{CDM::CLibrary, {"free"}, 1}, &MallocChecker::checkFree},
+ {{CDM::CLibrary, {"if_freenameindex"}, 1}, &MallocChecker::checkIfFreeNameIndex},
+ {{CDM::CLibrary, {"kfree"}, 1}, &MallocChecker::checkFree},
+ {{CDM::CLibrary, {"g_free"}, 1}, &MallocChecker::checkFree},
};
bool isFreeingCall(const CallEvent &Call) const;
@@ -413,41 +413,46 @@ class MallocChecker
friend class NoOwnershipChangeVisitor;
CallDescriptionMap<CheckFn> AllocatingMemFnMap{
- {{{"alloca"}, 1}, &MallocChecker::checkAlloca},
- {{{"_alloca"}, 1}, &MallocChecker::checkAlloca},
- {{{"malloc"}, 1}, &MallocChecker::checkBasicAlloc},
- {{{"malloc"}, 3}, &MallocChecker::checkKernelMalloc},
- {{{"calloc"}, 2}, &MallocChecker::checkCalloc},
- {{{"valloc"}, 1}, &MallocChecker::checkBasicAlloc},
+ {{CDM::CLibrary, {"alloca"}, 1}, &MallocChecker::checkAlloca},
+ {{CDM::CLibrary, {"_alloca"}, 1}, &MallocChecker::checkAlloca},
+ // The line for "alloca" also covers "__builtin_alloca", but the
+ // _with_align variant must be listed separately because it takes an
+ // extra argument:
+ {{CDM::CLibrary, {"__builtin_alloca_with_align"}, 2},
+ &MallocChecker::checkAlloca},
+ {{CDM::CLibrary, {"malloc"}, 1}, &MallocChecker::checkBasicAlloc},
+ {{CDM::CLibrary, {"malloc"}, 3}, &MallocChecker::checkKernelMalloc},
+ {{CDM::CLibrary, {"calloc"}, 2}, &MallocChecker::checkCalloc},
+ {{CDM::CLibrary, {"valloc"}, 1}, &MallocChecker::checkBasicAlloc},
{{CDM::CLibrary, {"strndup"}, 2}, &MallocChecker::checkStrdup},
{{CDM::CLibrary, {"strdup"}, 1}, &MallocChecker::checkStrdup},
- {{{"_strdup"}, 1}, &MallocChecker::checkStrdup},
- {{{"kmalloc"}, 2}, &MallocChecker::checkKernelMalloc},
- {{{"if_nameindex"}, 1}, &MallocChecker::checkIfNameIndex},
+ {{CDM::CLibrary, {"_strdup"}, 1}, &MallocChecker::checkStrdup},
+ {{CDM::CLibrary, {"kmalloc"}, 2}, &MallocChecker::checkKernelMalloc},
+ {{CDM::CLibrary, {"if_nameindex"}, 1}, &MallocChecker::checkIfNameIndex},
{{CDM::CLibrary, {"wcsdup"}, 1}, &MallocChecker::checkStrdup},
{{CDM::CLibrary, {"_wcsdup"}, 1}, &MallocChecker::checkStrdup},
- {{{"g_malloc"}, 1}, &MallocChecker::checkBasicAlloc},
- {{{"g_malloc0"}, 1}, &MallocChecker::checkGMalloc0},
- {{{"g_try_malloc"}, 1}, &MallocChecker::checkBasicAlloc},
- {{{"g_try_malloc0"}, 1}, &MallocChecker::checkGMalloc0},
- {{{"g_memdup"}, 2}, &MallocChecker::checkGMemdup},
- {{{"g_malloc_n"}, 2}, &MallocChecker::checkGMallocN},
- {{{"g_malloc0_n"}, 2}, &MallocChecker::checkGMallocN0},
- {{{"g_try_malloc_n"}, 2}, &MallocChecker::checkGMallocN},
- {{{"g_try_malloc0_n"}, 2}, &MallocChecker::checkGMallocN0},
+ {{CDM::CLibrary, {"g_malloc"}, 1}, &MallocChecker::checkBasicAlloc},
+ {{CDM::CLibrary, {"g_malloc0"}, 1}, &MallocChecker::checkGMalloc0},
+ {{CDM::CLibrary, {"g_try_malloc"}, 1}, &MallocChecker::checkBasicAlloc},
+ {{CDM::CLibrary, {"g_try_malloc0"}, 1}, &MallocChecker::checkGMalloc0},
+ {{CDM::CLibrary, {"g_memdup"}, 2}, &MallocChecker::checkGMemdup},
+ {{CDM::CLibrary, {"g_malloc_n"}, 2}, &MallocChecker::checkGMallocN},
+ {{CDM::CLibrary, {"g_malloc0_n"}, 2}, &MallocChecker::checkGMallocN0},
+ {{CDM::CLibrary, {"g_try_malloc_n"}, 2}, &MallocChecker::checkGMallocN},
+ {{CDM::CLibrary, {"g_try_malloc0_n"}, 2}, &MallocChecker::checkGMallocN0},
};
CallDescriptionMap<CheckFn> ReallocatingMemFnMap{
- {{{"realloc"}, 2},
+ {{CDM::CLibrary, {"realloc"}, 2},
std::bind(&MallocChecker::checkRealloc, _1, _2, _3, false)},
- {{{"reallocf"}, 2},
+ {{CDM::CLibrary, {"reallocf"}, 2},
std::bind(&MallocChecker::checkRealloc, _1, _2, _3, true)},
- {{{"g_realloc"}, 2},
+ {{CDM::CLibrary, {"g_realloc"}, 2},
std::bind(&MallocChecker::checkRealloc, _1, _2, _3, false)},
- {{{"g_try_realloc"}, 2},
+ {{CDM::CLibrary, {"g_try_realloc"}, 2},
std::bind(&MallocChecker::checkRealloc, _1, _2, _3, false)},
- {{{"g_realloc_n"}, 3}, &MallocChecker::checkReallocN},
- {{{"g_try_realloc_n"}, 3}, &MallocChecker::checkReallocN},
+ {{CDM::CLibrary, {"g_realloc_n"}, 3}, &MallocChecker::checkReallocN},
+ {{CDM::CLibrary, {"g_try_realloc_n"}, 3}, &MallocChecker::checkReallocN},
// NOTE: the following CallDescription also matches the C++ standard
// library function std::getline(); the callback will filter it out.
@@ -1259,9 +1264,6 @@ static bool isStandardRealloc(const CallEvent &Call) {
assert(FD);
ASTContext &AC = FD->getASTContext();
- if (isa<CXXMethodDecl>(FD))
- return false;
-
return FD->getDeclaredReturnType().getDesugaredType(AC) == AC.VoidPtrTy &&
FD->getParamDecl(0)->getType().getDesugaredType(AC) == AC.VoidPtrTy &&
FD->getParamDecl(1)->getType().getDesugaredType(AC) ==
@@ -1273,9 +1275,6 @@ static bool isGRealloc(const CallEvent &Call) {
assert(FD);
ASTContext &AC = FD->getASTContext();
- if (isa<CXXMethodDecl>(FD))
- return false;
-
return FD->getDeclaredReturnType().getDesugaredType(AC) == AC.VoidPtrTy &&
FD->getParamDecl(0)->getType().getDesugaredType(AC) == AC.VoidPtrTy &&
FD->getParamDecl(1)->getType().getDesugaredType(AC) ==
@@ -1284,14 +1283,14 @@ static bool isGRealloc(const CallEvent &Call) {
void MallocChecker::checkRealloc(const CallEvent &Call, CheckerContext &C,
bool ShouldFreeOnFail) const {
- // HACK: CallDescription currently recognizes non-standard realloc functions
- // as standard because it doesn't check the type, or wether its a non-method
- // function. This should be solved by making CallDescription smarter.
- // Mind that this came from a bug report, and all other functions suffer from
- // this.
- // https://bugs.llvm.org/show_bug.cgi?id=46253
+ // Ignore calls to functions whose type does not match the expected type of
+ // either the standard realloc or g_realloc from GLib.
+ // FIXME: Should we perform this kind of checking consistently for each
+ // function? If yes, then perhaps extend the `CallDescription` interface to
+ // handle this.
if (!isStandardRealloc(Call) && !isGRealloc(Call))
return;
+
ProgramStateRef State = C.getState();
State = ReallocMemAux(C, Call, ShouldFreeOnFail, State, AF_Malloc);
State = ProcessZeroAllocCheck(Call, 1, State);
@@ -1842,9 +1841,18 @@ static ProgramStateRef MallocUpdateRefState(CheckerContext &C, const Expr *E,
return nullptr;
SymbolRef Sym = RetVal->getAsLocSymbol();
+
// This is a return value of a function that was not inlined, such as malloc()
// or new(). We've checked that in the caller. Therefore, it must be a symbol.
assert(Sym);
+ // FIXME: In theory this assertion should fail for `alloca()` calls (because
+ // `AllocaRegion`s are not symbolic); but in practice this does not happen.
+ // As the current code appears to work correctly, I'm not touching this issue
+ // now, but it would be good to investigate and clarify this.
+ // Also note that perhaps the special `AllocaRegion` should be replaced by
+ // `SymbolicRegion` (or turned into a subclass of `SymbolicRegion`) to enable
+ // proper tracking of memory allocated by `alloca()` -- and after that change
+ // this assertion would become valid again.
// Set the symbol's state to Allocated.
return State->set<RegionState>(Sym, RefState::getAllocated(Family, E));
diff --git a/clang/test/Analysis/Inputs/system-header-simulator-cxx.h b/clang/test/Analysis/Inputs/system-header-simulator-cxx.h
index 85db68d41a6c80..1c2be322f83c20 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator-cxx.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator-cxx.h
@@ -1106,6 +1106,7 @@ using ostream = basic_ostream<char>;
extern std::ostream cout;
ostream &operator<<(ostream &, const string &);
+
#if __cplusplus >= 202002L
template <class T>
ostream &operator<<(ostream &, const std::unique_ptr<T> &);
@@ -1122,11 +1123,12 @@ istream &getline(istream &, string &, char);
istream &getline(istream &, string &);
} // namespace std
-#ifdef TEST_INLINABLE_ALLOCATORS
namespace std {
void *malloc(size_t);
void free(void *);
-}
+} // namespace std
+
+#ifdef TEST_INLINABLE_ALLOCATORS
void* operator new(std::size_t size, const std::nothrow_t&) throw() { return std::malloc(size); }
void* operator new[](std::size_t size, const std::nothrow_t&) throw() { return std::malloc(size); }
void operator delete(void* ptr, const std::nothrow_t&) throw() { std::free(ptr); }
diff --git a/clang/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp b/clang/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp
index fc067dd04428a8..f46a2c9bc368f6 100644
--- a/clang/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp
+++ b/clang/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp
@@ -1,9 +1,9 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,optin.cplusplus.UninitializedObject \
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc,optin.cplusplus.UninitializedObject \
// RUN: -analyzer-config optin.cplusplus.UninitializedObject:Pedantic=true -DPEDANTIC \
// RUN: -analyzer-config optin.cplusplus.UninitializedObject:CheckPointeeInitialization=true \
// RUN: -std=c++11 -verify %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,optin.cplusplus.UninitializedObject \
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc,optin.cplusplus.UninitializedObject \
// RUN: -analyzer-config optin.cplusplus.UninitializedObject:CheckPointeeInitialization=true \
// RUN: -std=c++11 -verify %s
@@ -316,7 +316,10 @@ void fCyclicPointerTest2() {
// Void pointer tests are mainly no-crash tests.
-void *malloc(int size);
+typedef __typeof(sizeof(int)) size_t;
+
+void *calloc(size_t nmemb, size_t size);
+void free(void *p);
class VoidPointerTest1 {
void *vptr;
@@ -328,8 +331,9 @@ class VoidPointerTest1 {
};
void fVoidPointerTest1() {
- void *vptr = malloc(sizeof(int));
+ void *vptr = calloc(1, sizeof(int));
VoidPointerTest1(vptr, char());
+ free(vptr);
}
class VoidPointerTest2 {
@@ -342,8 +346,9 @@ class VoidPointerTest2 {
};
void fVoidPointerTest2() {
- void *vptr = malloc(sizeof(int));
+ void *vptr = calloc(1, sizeof(int));
VoidPointerTest2(&vptr, char());
+ free(vptr);
}
class VoidPointerRRefTest1 {
@@ -359,8 +364,9 @@ upon returning to the caller. This will be a dangling reference}}
};
void fVoidPointerRRefTest1() {
- void *vptr = malloc(sizeof(int));
+ void *vptr = calloc(1, sizeof(int));
VoidPointerRRefTest1(vptr, char());
+ free(vptr);
}
class VoidPointerRRefTest2 {
@@ -376,8 +382,9 @@ upon returning to the caller. This will be a dangling reference}}
};
void fVoidPointerRRefTest2() {
- void *vptr = malloc(sizeof(int));
+ void *vptr = calloc(1, sizeof(int));
VoidPointerRRefTest2(&vptr, char());
+ free(vptr);
}
class VoidPointerLRefTest {
@@ -393,8 +400,9 @@ upon returning to the caller. This will be a dangling reference}}
};
void fVoidPointerLRefTest() {
- void *vptr = malloc(sizeof(int));
+ void *vptr = calloc(1, sizeof(int));
VoidPointerLRefTest(vptr, char());
+ free(vptr);
}
struct CyclicVoidPointerTest {
diff --git a/clang/test/Analysis/exercise-ps.c b/clang/test/Analysis/exercise-ps.c
index d214c3959b2078..d1e1771afddb5e 100644
--- a/clang/test/Analysis/exercise-ps.c
+++ b/clang/test/Analysis/exercise-ps.c
@@ -1,5 +1,5 @@
// RUN: %clang_analyze_cc1 %s -verify -Wno-error=implicit-function-declaration \
-// RUN: -analyzer-checker=core \
+// RUN: -analyzer-checker=core,unix.Malloc \
// RUN: -analyzer-config core.CallAndMessage:ArgPointeeInitializedness=true
//
// Just exercise the analyzer on code that has at one point caused issues
diff --git a/clang/test/Analysis/explain-svals.cpp b/clang/test/Analysis/explain-svals.cpp
index 30368b6976cc23..33fce10c4e2b2c 100644
--- a/clang/test/Analysis/explain-svals.cpp
+++ b/clang/test/Analysis/explain-svals.cpp
@@ -1,7 +1,7 @@
// RUN: %clang_analyze_cc1 -triple i386-apple-darwin10 -verify %s \
-// RUN: -analyzer-checker=core.builtin \
// RUN: -analyzer-checker=debug.ExprInspection \
// RUN: -analyzer-checker=unix.cstring \
+// RUN: -analyzer-checker=unix.Malloc \
// RUN: -analyzer-config display-checker-name=false
typedef unsigned long size_t;
diff --git a/clang/test/Analysis/malloc-std-namespace.cpp b/clang/test/Analysis/malloc-std-namespace.cpp
new file mode 100644
index 00000000000000..d4e397bb812aa9
--- /dev/null
+++ b/clang/test/Analysis/malloc-std-namespace.cpp
@@ -0,0 +1,24 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc -verify -analyzer-output=text %s
+
+// This file tests that unix.Malloc can handle C++ code where e.g. malloc and
+// free are declared within the namespace 'std' by the header <cstdlib>.
+
+#include "Inputs/system-header-simulator-cxx.h"
+
+void leak() {
+ int *p = static_cast<int*>(std::malloc(sizeof(int))); // expected-note{{Memory is allocated}}
+} // expected-warning{{Potential leak of memory pointed to by 'p'}}
+ // expected-note@-1{{Potential leak of memory pointed to by 'p'}}
+
+void no_leak() {
+ int *p = static_cast<int*>(std::malloc(sizeof(int)));
+ std::free(p); // no-warning
+}
+
+void invalid_free() {
+ int i;
+ int *p = &i;
+ //expected-note@+2{{Argument to free() is the address of the local variable 'i', which is not memory allocated by malloc()}}
+ //expected-warning@+1{{Argument to free() is the address of the local variable 'i', which is not memory allocated by malloc()}}
+ std::free(p);
+}
diff --git a/clang/test/Analysis/malloc.c b/clang/test/Analysis/malloc.c
index 09cd4b0bfce638..e5cb45ba733524 100644
--- a/clang/test/Analysis/malloc.c
+++ b/clang/test/Analysis/malloc.c
@@ -740,6 +740,17 @@ void allocaFree(void) {
free(p); // expected-warning {{Memory allocated by alloca() should not be deallocated}}
}
+void allocaFreeBuiltin(void) {
+ int *p = __builtin_alloca(sizeof(int));
+ free(p); // expected-warning {{Memory allocated by alloca() should not be deallocated}}
+}
+
+void allocaFreeBuiltinAlign(void) {
+ int *p = __builtin_alloca_with_align(sizeof(int), 64);
+ free(p); // expected-warning {{Memory allocated by alloca() should not be deallocated}}
+}
+
+
int* mallocEscapeRet(void) {
int *p = malloc(12);
return p; // no warning
diff --git a/clang/test/Analysis/malloc.cpp b/clang/test/Analysis/malloc.cpp
index 14b4c0576384f2..fa5b75fe0560b7 100644
--- a/clang/test/Analysis/malloc.cpp
+++ b/clang/test/Analysis/malloc.cpp
@@ -214,3 +214,15 @@ void *realloc(void **ptr, size_t size) { realloc(ptr, size); } // no-crash
namespace pr46253_paramty2{
void *realloc(void *ptr, int size) { realloc(ptr, size); } // no-crash
} // namespace pr46253_paramty2
+
+namespace pr81597{
+struct S {};
+struct T {
+ void free(const S& s);
+};
+void f(T& t) {
+ S s;
+ // This is not the free you are looking for...
+ t.free(s); // no-warning
+}
+} // namespace pr81597
diff --git a/clang/test/Analysis/stack-addr-ps.c b/clang/test/Analysis/stack-addr-ps.c
index e469396e1bb22a..e69ab4189b524f 100644
--- a/clang/test/Analysis/stack-addr-ps.c
+++ b/clang/test/Analysis/stack-addr-ps.c
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core -fblocks -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc -fblocks -verify %s
int* f1(void) {
int x = 0;
diff --git a/clang/test/Analysis/stackaddrleak.c b/clang/test/Analysis/stackaddrleak.c
index 0583bfc18711c5..39c29f2a2635b5 100644
--- a/clang/test/Analysis/stackaddrleak.c
+++ b/clang/test/Analysis/stackaddrleak.c
@@ -1,5 +1,5 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify -std=c99 -Dbool=_Bool -Wno-bool-conversion %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify -x c++ -Wno-bool-conversion %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc -verify -std=c99 -Dbool=_Bool -Wno-bool-conversion %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc -verify -x c++ -Wno-bool-conversion %s
typedef __INTPTR_TYPE__ intptr_t;
char const *p;
|
|
@llvm/pr-subscribers-clang-static-analyzer-1 Author: None (NagyDonat) ChangesThis commit ensures that the
This change fixes #81597. New tests were added to verify that The responsibility for modeling This change might be a step backwards for the users who don't have There were several test files that relied on While I was developing this change, I found a very suspicious assert in Full diff: https://github.com/llvm/llvm-project/pull/88416.diff 11 Files Affected:
diff --git a/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
index 01e46fa8591c07..1a75d7b52ad6e9 100644
--- a/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
@@ -6,7 +6,11 @@
//
//===----------------------------------------------------------------------===//
//
-// This checker evaluates clang builtin functions.
+// This checker evaluates "standalone" clang builtin functions that are not
+// just special-cased variants of well-known non-builtin functions.
+// Builtin functions like __builtin_memcpy and __builtin_alloca should be
+// evaluated by the same checker that handles their non-builtin variant to
+// ensure that the two variants are handled consistently.
//
//===----------------------------------------------------------------------===//
@@ -80,25 +84,6 @@ bool BuiltinFunctionChecker::evalCall(const CallEvent &Call,
return true;
}
- case Builtin::BI__builtin_alloca_with_align:
- case Builtin::BI__builtin_alloca: {
- SValBuilder &SVB = C.getSValBuilder();
- const loc::MemRegionVal R =
- SVB.getAllocaRegionVal(CE, C.getLocationContext(), C.blockCount());
-
- // Set the extent of the region in bytes. This enables us to use the SVal
- // of the argument directly. If we saved the extent in bits, it'd be more
- // difficult to reason about values like symbol*8.
- auto Size = Call.getArgSVal(0);
- if (auto DefSize = Size.getAs<DefinedOrUnknownSVal>()) {
- // This `getAs()` is mostly paranoia, because core.CallAndMessage reports
- // undefined function arguments (unless it's disabled somehow).
- state = setDynamicExtent(state, R.getRegion(), *DefSize, SVB);
- }
- C.addTransition(state->BindExpr(CE, LCtx, R));
- return true;
- }
-
case Builtin::BI__builtin_dynamic_object_size:
case Builtin::BI__builtin_object_size:
case Builtin::BI__builtin_constant_p: {
diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index 88fb42b6625aa4..c141d6dcef9a73 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -401,10 +401,10 @@ class MallocChecker
};
const CallDescriptionMap<CheckFn> FreeingMemFnMap{
- {{{"free"}, 1}, &MallocChecker::checkFree},
- {{{"if_freenameindex"}, 1}, &MallocChecker::checkIfFreeNameIndex},
- {{{"kfree"}, 1}, &MallocChecker::checkFree},
- {{{"g_free"}, 1}, &MallocChecker::checkFree},
+ {{CDM::CLibrary, {"free"}, 1}, &MallocChecker::checkFree},
+ {{CDM::CLibrary, {"if_freenameindex"}, 1}, &MallocChecker::checkIfFreeNameIndex},
+ {{CDM::CLibrary, {"kfree"}, 1}, &MallocChecker::checkFree},
+ {{CDM::CLibrary, {"g_free"}, 1}, &MallocChecker::checkFree},
};
bool isFreeingCall(const CallEvent &Call) const;
@@ -413,41 +413,46 @@ class MallocChecker
friend class NoOwnershipChangeVisitor;
CallDescriptionMap<CheckFn> AllocatingMemFnMap{
- {{{"alloca"}, 1}, &MallocChecker::checkAlloca},
- {{{"_alloca"}, 1}, &MallocChecker::checkAlloca},
- {{{"malloc"}, 1}, &MallocChecker::checkBasicAlloc},
- {{{"malloc"}, 3}, &MallocChecker::checkKernelMalloc},
- {{{"calloc"}, 2}, &MallocChecker::checkCalloc},
- {{{"valloc"}, 1}, &MallocChecker::checkBasicAlloc},
+ {{CDM::CLibrary, {"alloca"}, 1}, &MallocChecker::checkAlloca},
+ {{CDM::CLibrary, {"_alloca"}, 1}, &MallocChecker::checkAlloca},
+ // The line for "alloca" also covers "__builtin_alloca", but the
+ // _with_align variant must be listed separately because it takes an
+ // extra argument:
+ {{CDM::CLibrary, {"__builtin_alloca_with_align"}, 2},
+ &MallocChecker::checkAlloca},
+ {{CDM::CLibrary, {"malloc"}, 1}, &MallocChecker::checkBasicAlloc},
+ {{CDM::CLibrary, {"malloc"}, 3}, &MallocChecker::checkKernelMalloc},
+ {{CDM::CLibrary, {"calloc"}, 2}, &MallocChecker::checkCalloc},
+ {{CDM::CLibrary, {"valloc"}, 1}, &MallocChecker::checkBasicAlloc},
{{CDM::CLibrary, {"strndup"}, 2}, &MallocChecker::checkStrdup},
{{CDM::CLibrary, {"strdup"}, 1}, &MallocChecker::checkStrdup},
- {{{"_strdup"}, 1}, &MallocChecker::checkStrdup},
- {{{"kmalloc"}, 2}, &MallocChecker::checkKernelMalloc},
- {{{"if_nameindex"}, 1}, &MallocChecker::checkIfNameIndex},
+ {{CDM::CLibrary, {"_strdup"}, 1}, &MallocChecker::checkStrdup},
+ {{CDM::CLibrary, {"kmalloc"}, 2}, &MallocChecker::checkKernelMalloc},
+ {{CDM::CLibrary, {"if_nameindex"}, 1}, &MallocChecker::checkIfNameIndex},
{{CDM::CLibrary, {"wcsdup"}, 1}, &MallocChecker::checkStrdup},
{{CDM::CLibrary, {"_wcsdup"}, 1}, &MallocChecker::checkStrdup},
- {{{"g_malloc"}, 1}, &MallocChecker::checkBasicAlloc},
- {{{"g_malloc0"}, 1}, &MallocChecker::checkGMalloc0},
- {{{"g_try_malloc"}, 1}, &MallocChecker::checkBasicAlloc},
- {{{"g_try_malloc0"}, 1}, &MallocChecker::checkGMalloc0},
- {{{"g_memdup"}, 2}, &MallocChecker::checkGMemdup},
- {{{"g_malloc_n"}, 2}, &MallocChecker::checkGMallocN},
- {{{"g_malloc0_n"}, 2}, &MallocChecker::checkGMallocN0},
- {{{"g_try_malloc_n"}, 2}, &MallocChecker::checkGMallocN},
- {{{"g_try_malloc0_n"}, 2}, &MallocChecker::checkGMallocN0},
+ {{CDM::CLibrary, {"g_malloc"}, 1}, &MallocChecker::checkBasicAlloc},
+ {{CDM::CLibrary, {"g_malloc0"}, 1}, &MallocChecker::checkGMalloc0},
+ {{CDM::CLibrary, {"g_try_malloc"}, 1}, &MallocChecker::checkBasicAlloc},
+ {{CDM::CLibrary, {"g_try_malloc0"}, 1}, &MallocChecker::checkGMalloc0},
+ {{CDM::CLibrary, {"g_memdup"}, 2}, &MallocChecker::checkGMemdup},
+ {{CDM::CLibrary, {"g_malloc_n"}, 2}, &MallocChecker::checkGMallocN},
+ {{CDM::CLibrary, {"g_malloc0_n"}, 2}, &MallocChecker::checkGMallocN0},
+ {{CDM::CLibrary, {"g_try_malloc_n"}, 2}, &MallocChecker::checkGMallocN},
+ {{CDM::CLibrary, {"g_try_malloc0_n"}, 2}, &MallocChecker::checkGMallocN0},
};
CallDescriptionMap<CheckFn> ReallocatingMemFnMap{
- {{{"realloc"}, 2},
+ {{CDM::CLibrary, {"realloc"}, 2},
std::bind(&MallocChecker::checkRealloc, _1, _2, _3, false)},
- {{{"reallocf"}, 2},
+ {{CDM::CLibrary, {"reallocf"}, 2},
std::bind(&MallocChecker::checkRealloc, _1, _2, _3, true)},
- {{{"g_realloc"}, 2},
+ {{CDM::CLibrary, {"g_realloc"}, 2},
std::bind(&MallocChecker::checkRealloc, _1, _2, _3, false)},
- {{{"g_try_realloc"}, 2},
+ {{CDM::CLibrary, {"g_try_realloc"}, 2},
std::bind(&MallocChecker::checkRealloc, _1, _2, _3, false)},
- {{{"g_realloc_n"}, 3}, &MallocChecker::checkReallocN},
- {{{"g_try_realloc_n"}, 3}, &MallocChecker::checkReallocN},
+ {{CDM::CLibrary, {"g_realloc_n"}, 3}, &MallocChecker::checkReallocN},
+ {{CDM::CLibrary, {"g_try_realloc_n"}, 3}, &MallocChecker::checkReallocN},
// NOTE: the following CallDescription also matches the C++ standard
// library function std::getline(); the callback will filter it out.
@@ -1259,9 +1264,6 @@ static bool isStandardRealloc(const CallEvent &Call) {
assert(FD);
ASTContext &AC = FD->getASTContext();
- if (isa<CXXMethodDecl>(FD))
- return false;
-
return FD->getDeclaredReturnType().getDesugaredType(AC) == AC.VoidPtrTy &&
FD->getParamDecl(0)->getType().getDesugaredType(AC) == AC.VoidPtrTy &&
FD->getParamDecl(1)->getType().getDesugaredType(AC) ==
@@ -1273,9 +1275,6 @@ static bool isGRealloc(const CallEvent &Call) {
assert(FD);
ASTContext &AC = FD->getASTContext();
- if (isa<CXXMethodDecl>(FD))
- return false;
-
return FD->getDeclaredReturnType().getDesugaredType(AC) == AC.VoidPtrTy &&
FD->getParamDecl(0)->getType().getDesugaredType(AC) == AC.VoidPtrTy &&
FD->getParamDecl(1)->getType().getDesugaredType(AC) ==
@@ -1284,14 +1283,14 @@ static bool isGRealloc(const CallEvent &Call) {
void MallocChecker::checkRealloc(const CallEvent &Call, CheckerContext &C,
bool ShouldFreeOnFail) const {
- // HACK: CallDescription currently recognizes non-standard realloc functions
- // as standard because it doesn't check the type, or wether its a non-method
- // function. This should be solved by making CallDescription smarter.
- // Mind that this came from a bug report, and all other functions suffer from
- // this.
- // https://bugs.llvm.org/show_bug.cgi?id=46253
+ // Ignore calls to functions whose type does not match the expected type of
+ // either the standard realloc or g_realloc from GLib.
+ // FIXME: Should we perform this kind of checking consistently for each
+ // function? If yes, then perhaps extend the `CallDescription` interface to
+ // handle this.
if (!isStandardRealloc(Call) && !isGRealloc(Call))
return;
+
ProgramStateRef State = C.getState();
State = ReallocMemAux(C, Call, ShouldFreeOnFail, State, AF_Malloc);
State = ProcessZeroAllocCheck(Call, 1, State);
@@ -1842,9 +1841,18 @@ static ProgramStateRef MallocUpdateRefState(CheckerContext &C, const Expr *E,
return nullptr;
SymbolRef Sym = RetVal->getAsLocSymbol();
+
// This is a return value of a function that was not inlined, such as malloc()
// or new(). We've checked that in the caller. Therefore, it must be a symbol.
assert(Sym);
+ // FIXME: In theory this assertion should fail for `alloca()` calls (because
+ // `AllocaRegion`s are not symbolic); but in practice this does not happen.
+ // As the current code appears to work correctly, I'm not touching this issue
+ // now, but it would be good to investigate and clarify this.
+ // Also note that perhaps the special `AllocaRegion` should be replaced by
+ // `SymbolicRegion` (or turned into a subclass of `SymbolicRegion`) to enable
+ // proper tracking of memory allocated by `alloca()` -- and after that change
+ // this assertion would become valid again.
// Set the symbol's state to Allocated.
return State->set<RegionState>(Sym, RefState::getAllocated(Family, E));
diff --git a/clang/test/Analysis/Inputs/system-header-simulator-cxx.h b/clang/test/Analysis/Inputs/system-header-simulator-cxx.h
index 85db68d41a6c80..1c2be322f83c20 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator-cxx.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator-cxx.h
@@ -1106,6 +1106,7 @@ using ostream = basic_ostream<char>;
extern std::ostream cout;
ostream &operator<<(ostream &, const string &);
+
#if __cplusplus >= 202002L
template <class T>
ostream &operator<<(ostream &, const std::unique_ptr<T> &);
@@ -1122,11 +1123,12 @@ istream &getline(istream &, string &, char);
istream &getline(istream &, string &);
} // namespace std
-#ifdef TEST_INLINABLE_ALLOCATORS
namespace std {
void *malloc(size_t);
void free(void *);
-}
+} // namespace std
+
+#ifdef TEST_INLINABLE_ALLOCATORS
void* operator new(std::size_t size, const std::nothrow_t&) throw() { return std::malloc(size); }
void* operator new[](std::size_t size, const std::nothrow_t&) throw() { return std::malloc(size); }
void operator delete(void* ptr, const std::nothrow_t&) throw() { std::free(ptr); }
diff --git a/clang/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp b/clang/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp
index fc067dd04428a8..f46a2c9bc368f6 100644
--- a/clang/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp
+++ b/clang/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp
@@ -1,9 +1,9 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,optin.cplusplus.UninitializedObject \
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc,optin.cplusplus.UninitializedObject \
// RUN: -analyzer-config optin.cplusplus.UninitializedObject:Pedantic=true -DPEDANTIC \
// RUN: -analyzer-config optin.cplusplus.UninitializedObject:CheckPointeeInitialization=true \
// RUN: -std=c++11 -verify %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,optin.cplusplus.UninitializedObject \
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc,optin.cplusplus.UninitializedObject \
// RUN: -analyzer-config optin.cplusplus.UninitializedObject:CheckPointeeInitialization=true \
// RUN: -std=c++11 -verify %s
@@ -316,7 +316,10 @@ void fCyclicPointerTest2() {
// Void pointer tests are mainly no-crash tests.
-void *malloc(int size);
+typedef __typeof(sizeof(int)) size_t;
+
+void *calloc(size_t nmemb, size_t size);
+void free(void *p);
class VoidPointerTest1 {
void *vptr;
@@ -328,8 +331,9 @@ class VoidPointerTest1 {
};
void fVoidPointerTest1() {
- void *vptr = malloc(sizeof(int));
+ void *vptr = calloc(1, sizeof(int));
VoidPointerTest1(vptr, char());
+ free(vptr);
}
class VoidPointerTest2 {
@@ -342,8 +346,9 @@ class VoidPointerTest2 {
};
void fVoidPointerTest2() {
- void *vptr = malloc(sizeof(int));
+ void *vptr = calloc(1, sizeof(int));
VoidPointerTest2(&vptr, char());
+ free(vptr);
}
class VoidPointerRRefTest1 {
@@ -359,8 +364,9 @@ upon returning to the caller. This will be a dangling reference}}
};
void fVoidPointerRRefTest1() {
- void *vptr = malloc(sizeof(int));
+ void *vptr = calloc(1, sizeof(int));
VoidPointerRRefTest1(vptr, char());
+ free(vptr);
}
class VoidPointerRRefTest2 {
@@ -376,8 +382,9 @@ upon returning to the caller. This will be a dangling reference}}
};
void fVoidPointerRRefTest2() {
- void *vptr = malloc(sizeof(int));
+ void *vptr = calloc(1, sizeof(int));
VoidPointerRRefTest2(&vptr, char());
+ free(vptr);
}
class VoidPointerLRefTest {
@@ -393,8 +400,9 @@ upon returning to the caller. This will be a dangling reference}}
};
void fVoidPointerLRefTest() {
- void *vptr = malloc(sizeof(int));
+ void *vptr = calloc(1, sizeof(int));
VoidPointerLRefTest(vptr, char());
+ free(vptr);
}
struct CyclicVoidPointerTest {
diff --git a/clang/test/Analysis/exercise-ps.c b/clang/test/Analysis/exercise-ps.c
index d214c3959b2078..d1e1771afddb5e 100644
--- a/clang/test/Analysis/exercise-ps.c
+++ b/clang/test/Analysis/exercise-ps.c
@@ -1,5 +1,5 @@
// RUN: %clang_analyze_cc1 %s -verify -Wno-error=implicit-function-declaration \
-// RUN: -analyzer-checker=core \
+// RUN: -analyzer-checker=core,unix.Malloc \
// RUN: -analyzer-config core.CallAndMessage:ArgPointeeInitializedness=true
//
// Just exercise the analyzer on code that has at one point caused issues
diff --git a/clang/test/Analysis/explain-svals.cpp b/clang/test/Analysis/explain-svals.cpp
index 30368b6976cc23..33fce10c4e2b2c 100644
--- a/clang/test/Analysis/explain-svals.cpp
+++ b/clang/test/Analysis/explain-svals.cpp
@@ -1,7 +1,7 @@
// RUN: %clang_analyze_cc1 -triple i386-apple-darwin10 -verify %s \
-// RUN: -analyzer-checker=core.builtin \
// RUN: -analyzer-checker=debug.ExprInspection \
// RUN: -analyzer-checker=unix.cstring \
+// RUN: -analyzer-checker=unix.Malloc \
// RUN: -analyzer-config display-checker-name=false
typedef unsigned long size_t;
diff --git a/clang/test/Analysis/malloc-std-namespace.cpp b/clang/test/Analysis/malloc-std-namespace.cpp
new file mode 100644
index 00000000000000..d4e397bb812aa9
--- /dev/null
+++ b/clang/test/Analysis/malloc-std-namespace.cpp
@@ -0,0 +1,24 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc -verify -analyzer-output=text %s
+
+// This file tests that unix.Malloc can handle C++ code where e.g. malloc and
+// free are declared within the namespace 'std' by the header <cstdlib>.
+
+#include "Inputs/system-header-simulator-cxx.h"
+
+void leak() {
+ int *p = static_cast<int*>(std::malloc(sizeof(int))); // expected-note{{Memory is allocated}}
+} // expected-warning{{Potential leak of memory pointed to by 'p'}}
+ // expected-note@-1{{Potential leak of memory pointed to by 'p'}}
+
+void no_leak() {
+ int *p = static_cast<int*>(std::malloc(sizeof(int)));
+ std::free(p); // no-warning
+}
+
+void invalid_free() {
+ int i;
+ int *p = &i;
+ //expected-note@+2{{Argument to free() is the address of the local variable 'i', which is not memory allocated by malloc()}}
+ //expected-warning@+1{{Argument to free() is the address of the local variable 'i', which is not memory allocated by malloc()}}
+ std::free(p);
+}
diff --git a/clang/test/Analysis/malloc.c b/clang/test/Analysis/malloc.c
index 09cd4b0bfce638..e5cb45ba733524 100644
--- a/clang/test/Analysis/malloc.c
+++ b/clang/test/Analysis/malloc.c
@@ -740,6 +740,17 @@ void allocaFree(void) {
free(p); // expected-warning {{Memory allocated by alloca() should not be deallocated}}
}
+void allocaFreeBuiltin(void) {
+ int *p = __builtin_alloca(sizeof(int));
+ free(p); // expected-warning {{Memory allocated by alloca() should not be deallocated}}
+}
+
+void allocaFreeBuiltinAlign(void) {
+ int *p = __builtin_alloca_with_align(sizeof(int), 64);
+ free(p); // expected-warning {{Memory allocated by alloca() should not be deallocated}}
+}
+
+
int* mallocEscapeRet(void) {
int *p = malloc(12);
return p; // no warning
diff --git a/clang/test/Analysis/malloc.cpp b/clang/test/Analysis/malloc.cpp
index 14b4c0576384f2..fa5b75fe0560b7 100644
--- a/clang/test/Analysis/malloc.cpp
+++ b/clang/test/Analysis/malloc.cpp
@@ -214,3 +214,15 @@ void *realloc(void **ptr, size_t size) { realloc(ptr, size); } // no-crash
namespace pr46253_paramty2{
void *realloc(void *ptr, int size) { realloc(ptr, size); } // no-crash
} // namespace pr46253_paramty2
+
+namespace pr81597{
+struct S {};
+struct T {
+ void free(const S& s);
+};
+void f(T& t) {
+ S s;
+ // This is not the free you are looking for...
+ t.free(s); // no-warning
+}
+} // namespace pr81597
diff --git a/clang/test/Analysis/stack-addr-ps.c b/clang/test/Analysis/stack-addr-ps.c
index e469396e1bb22a..e69ab4189b524f 100644
--- a/clang/test/Analysis/stack-addr-ps.c
+++ b/clang/test/Analysis/stack-addr-ps.c
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core -fblocks -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc -fblocks -verify %s
int* f1(void) {
int x = 0;
diff --git a/clang/test/Analysis/stackaddrleak.c b/clang/test/Analysis/stackaddrleak.c
index 0583bfc18711c5..39c29f2a2635b5 100644
--- a/clang/test/Analysis/stackaddrleak.c
+++ b/clang/test/Analysis/stackaddrleak.c
@@ -1,5 +1,5 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify -std=c99 -Dbool=_Bool -Wno-bool-conversion %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify -x c++ -Wno-bool-conversion %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc -verify -std=c99 -Dbool=_Bool -Wno-bool-conversion %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc -verify -x c++ -Wno-bool-conversion %s
typedef __INTPTR_TYPE__ intptr_t;
char const *p;
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
steakhal
approved these changes
Apr 14, 2024
Co-authored-by: Balazs Benics <benicsbalazs@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit ensures that the
CallDescriptions inMallocCheckerare matched with the modeCDM::CLibrary, so:__builtin_alloca()likealloca().This change fixes #81597. New tests were added to verify that
std::mallocandstd::free(from<cstdlib>) are modeled, but a method that's named e.g.freeisn't confused with the memory release function.The responsibility for modeling
__builtin_allocaand__builtin_alloca_with_alignwas moved fromBuiltinFunctionCheckertoMallocChecker, to avoid buggy interactions between the checkers and ensure that the builtin and non-builtin variants are handled by exactly the same logic.This change might be a step backwards for the users who don't have
unix.Mallocenabled; but I suspect that__builtin_alloca()is so rare that it would be a waste of time to implement backwards compatibility for them.There were several test files that relied on
__builtin_alloca()calls to get anAllocaRegion, these were modified to enableunix.Malloc. One of these files (cxx-uninitialized-object-ptr-ref.cpp) had some tests that relied on the fact thatmalloc()was treated as a "black box" in them, these were updated to usecalloc()(to get initialized memory) andfree()(to avoid memory leak reports).While I was developing this change, I found a very suspicious assert in
MallocChecker. As it isn't blocking the goals of this commit, I just marked it with a FIXME, but I'll try to investigate and fix it in a follow-up change.