[llvm-branch-commits] [clang] Backport fix gh 183344 ownership returns crash (PR #183734)
Balázs Benics via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Fri Feb 27 04:54:28 PST 2026
https://github.com/steakhal created https://github.com/llvm/llvm-project/pull/183734
None
>From 50cb3defe9e37d4b658c3813686d19b7185f854f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20Benics?= <benicsbalazs at gmail.com>
Date: Fri, 27 Feb 2026 12:36:29 +0000
Subject: [PATCH 1/2] [analyzer] Fix crash in MallocChecker when a function has
both ownership_returns and ownership_takes (#183583)
When a function was annotated with both `ownership_returns` and
`ownership_takes` (or `ownership_holds`), MallocChecker::evalCall would
fall into the freeing-only branch (isFreeingOwnershipAttrCall) and call
checkOwnershipAttr without first calling MallocBindRetVal. That meant no
heap symbol had been conjured for the return value, so
checkOwnershipAttr later dereferenced a null/invalid symbol and crashed.
Fix: merge the two dispatch branches so that MallocBindRetVal is always
called first whenever ownership_returns is present, regardless of
whether the function also carries ownership_takes/ownership_holds.
The crash was introduced in #106081
339282d49f5310a2837da45c0ccc19da15675554.
Released in clang-20, and crashing ever since.
Fixes #183344.
Assisted-By: claude
(cherry picked from commit 2f4624613d05b2982dccc99ac0a06e87e6f3efd0)
---
.../StaticAnalyzer/Checkers/MallocChecker.cpp | 10 +-
clang/test/Analysis/malloc-annotations.c | 93 +++++++++++++++++++
2 files changed, 96 insertions(+), 7 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index ec7ef237b7c31..68369f8e81eb2 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -1705,13 +1705,9 @@ bool MallocChecker::evalCall(const CallEvent &Call, CheckerContext &C) const {
return true;
}
- if (isFreeingOwnershipAttrCall(Call)) {
- checkOwnershipAttr(State, Call, C);
- return true;
- }
-
- if (isAllocatingOwnershipAttrCall(Call)) {
- State = MallocBindRetVal(C, Call, State, false);
+ if (isFreeingOwnershipAttrCall(Call) || isAllocatingOwnershipAttrCall(Call)) {
+ if (isAllocatingOwnershipAttrCall(Call))
+ State = MallocBindRetVal(C, Call, State, false);
checkOwnershipAttr(State, Call, C);
return true;
}
diff --git a/clang/test/Analysis/malloc-annotations.c b/clang/test/Analysis/malloc-annotations.c
index fdfd1d014ded4..ce02e39e04cfb 100644
--- a/clang/test/Analysis/malloc-annotations.c
+++ b/clang/test/Analysis/malloc-annotations.c
@@ -278,3 +278,96 @@ void testNoUninitAttr(void) {
user_free(p);
}
+// Regression test for GH#183344 — crash when a function has both
+// ownership_returns and ownership_takes attributes.
+typedef struct GH183344_X GH183344_X;
+typedef struct GH183344_Y GH183344_Y;
+
+GH183344_Y *GH183344_X_to_Y(GH183344_X *x)
+ __attribute__((ownership_returns(GH183344_Y)))
+ __attribute__((ownership_takes(GH183344_X, 1)));
+
+void testGH183344(void) {
+ GH183344_Y *y = GH183344_X_to_Y(0); // no-crash
+ (void)y;
+} // expected-warning{{Potential leak of memory pointed to by 'y'}}
+
+// Extended regression tests for GH#183344 — additional combinations of
+// ownership_returns, ownership_takes, and ownership_holds.
+
+GH183344_X *GH183344_alloc_X(void)
+ __attribute__((ownership_returns(GH183344_X)));
+void GH183344_free_X(GH183344_X *)
+ __attribute__((ownership_takes(GH183344_X, 1)));
+void GH183344_free_Y(GH183344_Y *)
+ __attribute__((ownership_takes(GH183344_Y, 1)));
+
+// Returns + Holds variant: Y is allocated, X is held (not consumed) by callee.
+GH183344_Y *GH183344_X_to_Y_hold(GH183344_X *x)
+ __attribute__((ownership_returns(GH183344_Y)))
+ __attribute__((ownership_holds(GH183344_X, 1)));
+
+// Returns + two Takes (same pool): both X arguments are consumed, Y is
+// returned. Multiple arg indices in one attribute (same pool) is valid;
+// two ownership_takes attributes with *different* pool names are not.
+GH183344_Y *GH183344_combine_XX(GH183344_X *x1, GH183344_X *x2)
+ __attribute__((ownership_returns(GH183344_Y)))
+ __attribute__((ownership_takes(GH183344_X, 1, 2)));
+
+// No-crash for Returns+Holds with null input — same crash pattern as the
+// original GH183344 bug but with ownership_holds instead of ownership_takes.
+void testGH183344_ReturnsHolds_NullInput(void) {
+ GH183344_Y *y = GH183344_X_to_Y_hold(0); // no-crash
+ (void)y;
+} // expected-warning{{Potential leak of memory pointed to by 'y'}}
+
+// Returns+Takes: allocate X, convert to Y (X consumed), free Y — no warnings.
+void testGH183344_ReturnsTakes_CleanUse(void) {
+ GH183344_X *x = GH183344_alloc_X();
+ GH183344_Y *y = GH183344_X_to_Y(x);
+ GH183344_free_Y(y);
+} // no-warning
+
+// Returns+Takes: after the conversion X is consumed; freeing it again is a
+// double-free.
+void testGH183344_ReturnsTakes_DoubleFreeInput(void) {
+ GH183344_X *x = GH183344_alloc_X();
+ GH183344_Y *y = GH183344_X_to_Y(x);
+ GH183344_free_X(x); // expected-warning{{Attempt to release already released memory}}
+ GH183344_free_Y(y);
+}
+
+// Returns+Takes: X is consumed but Y is never freed — leak on Y.
+void testGH183344_ReturnsTakes_LeakRetval(void) {
+ GH183344_X *x = GH183344_alloc_X();
+ GH183344_Y *y = GH183344_X_to_Y(x);
+ (void)y;
+} // expected-warning{{Potential leak of memory pointed to by 'y'}}
+
+// Returns+Holds: after the hold, X is non-owned by the caller; freeing it
+// produces a "non-owned memory" warning (analogous to af3).
+void testGH183344_ReturnsHolds_FreeHeldInput(void) {
+ GH183344_X *x = GH183344_alloc_X();
+ GH183344_Y *y = GH183344_X_to_Y_hold(x);
+ GH183344_free_X(x); // expected-warning{{Attempt to release non-owned memory}}
+ GH183344_free_Y(y);
+}
+
+// Multiple Takes (same pool) + Returns: both X inputs are consumed, Y is
+// returned and freed — no warnings.
+void testGH183344_CombineXX_CleanUse(void) {
+ GH183344_X *x1 = GH183344_alloc_X();
+ GH183344_X *x2 = GH183344_alloc_X();
+ GH183344_Y *y = GH183344_combine_XX(x1, x2);
+ GH183344_free_Y(y);
+} // no-warning
+
+// Multiple Takes (same pool): after the combine, x2 is already consumed;
+// freeing it again is a double-free.
+void testGH183344_CombineXX_DoubleFreeSecondInput(void) {
+ GH183344_X *x1 = GH183344_alloc_X();
+ GH183344_X *x2 = GH183344_alloc_X();
+ GH183344_Y *y = GH183344_combine_XX(x1, x2);
+ GH183344_free_X(x2); // expected-warning{{Attempt to release already released memory}}
+ GH183344_free_Y(y);
+}
>From c27892cdb2fdd4104890882e04eed401c315ca24 Mon Sep 17 00:00:00 2001
From: Balazs Benics <benicsbalazs at gmail.com>
Date: Fri, 27 Feb 2026 12:51:35 +0000
Subject: [PATCH 2/2] fixup! [analyzer] Fix crash in MallocChecker when a
function has both ownership_returns and ownership_takes (#183583)
Mention in the release docs
---
clang/docs/ReleaseNotes.rst | 3 +++
1 file changed, 3 insertions(+)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 7ac3e272beff2..044b176c30810 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -912,6 +912,9 @@ Crash and bug fixes
checked arithmetic builtin functions. (#GH173795)
- Fixed a crash introduced in clang-20 when analyzing some "swap" functions.
(#GH178797)
+- Fixed a crash introduced in clang-20 in the ``unix.Malloc`` checker when
+ analyzing functions that had both of the ``ownership_returns`` and
+ ``ownership_takes`` attributes. (#GH183344)
Improvements
^^^^^^^^^^^^
More information about the llvm-branch-commits
mailing list