https://github.com/steakhal updated https://github.com/llvm/llvm-project/pull/183583
From 305b0895d3955f281c1feb1dc1922e81a85f2db2 Mon Sep 17 00:00:00 2001 From: Balazs Benics <[email protected]> Date: Thu, 26 Feb 2026 17:59:51 +0000 Subject: [PATCH 1/2] [analyzer] Fix crash in MallocChecker when a function has both ownership_returns and ownership_takes 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 --- clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp | 10 +++------- clang/test/Analysis/malloc-annotations.c | 13 +++++++++++++ 2 files changed, 16 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..12ad5ed8224e7 100644 --- a/clang/test/Analysis/malloc-annotations.c +++ b/clang/test/Analysis/malloc-annotations.c @@ -278,3 +278,16 @@ 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'}} From 03bd8da70df2f65f5723201bc6f1fe5418fa1b5f Mon Sep 17 00:00:00 2001 From: Balazs Benics <[email protected]> Date: Fri, 27 Feb 2026 12:06:16 +0000 Subject: [PATCH 2/2] Add more tests --- clang/test/Analysis/malloc-annotations.c | 80 ++++++++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/clang/test/Analysis/malloc-annotations.c b/clang/test/Analysis/malloc-annotations.c index 12ad5ed8224e7..ce02e39e04cfb 100644 --- a/clang/test/Analysis/malloc-annotations.c +++ b/clang/test/Analysis/malloc-annotations.c @@ -291,3 +291,83 @@ 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); +} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
