Author: Balázs Benics Date: 2026-02-27T12:36:29Z New Revision: 2f4624613d05b2982dccc99ac0a06e87e6f3efd0
URL: https://github.com/llvm/llvm-project/commit/2f4624613d05b2982dccc99ac0a06e87e6f3efd0 DIFF: https://github.com/llvm/llvm-project/commit/2f4624613d05b2982dccc99ac0a06e87e6f3efd0.diff LOG: [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 Added: Modified: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp clang/test/Analysis/malloc-annotations.c Removed: ################################################################################ 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 * diff erent* 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
