Repository navigation
Conversation
|
The Windows 2025 release job failed in The two inputs contain only global declarations, so the changed function-call/member-address path is not exercised. Dump creation/removal and this test are unchanged by the PR. Five repetitions with the untouched baseline and five with the patch passed locally with Could a maintainer rerun this Windows job once? The rerun API requires repository administrator rights. I have left the cleanup assertion and source unchanged rather than masking this intermittent failure. |
| if (arg->isCpp()) { | ||
| // A class/enum address-of expression may call a member, inherited or | ||
| // free operator& that returns storage unrelated to this object. | ||
| const ValueType* vt = member ? member->valueType() : nullptr; | ||
| if (!vt || vt->typeScope || (!vt->pointer && !vt->isIntegral() && !vt->isFloat())) | ||
| return nullptr; | ||
| } |
There was a problem hiding this comment.
This is an AI review. Take it with a grain of salt and feel free to reject it by resolving the comment.
I confirmed that the C false positive is fixed. However, this C++ restriction means the typical intrusive-list case is still reported when the same code is compiled as C++. The embedded node is a struct, so vt->typeScope is set:
#include <stdlib.h>
struct node { struct node *next; };
struct item { int x; struct node n; };
void list_add(struct node *n);
void f1(void) {
struct item *p = malloc(sizeof(*p));
if (!p) return;
list_add(&p->n);
p->x = 0;
}With this PR, a.c gives no warning but a.cpp still gives memleak at line 10.
Rejecting every class type to guard against an overloaded operator& seems very conservative. Maybe only reject when the type actually could overload it, for example when typeScope (or a base class) declares an operator&, or allow C-like structs whose functionList is empty? A free operator& is admittedly harder to rule out. If the restriction is kept on purpose, a TODO test for the C++ variant would document the limitation.
|
I have rerun the CI and it passes. |
|
Test results for commit 264fe96 (tools/test-my-pr.py, main compared to this PR): Test: http://ec2-16-170-140-253.eu-north-1.compute.amazonaws.com/pr-8891/ Posted automatically by the cppcheck PR test runner. +N: warnings only with this PR, -N: warnings only with main. The AI review is written by Claude and can be wrong. |
dmcppcheck
left a comment
dmcppcheck
left a comment
There was a problem hiding this comment.
AI review
I found no new problems. The only open point is the one already raised in the review: in C++, an embedded member of class type (the usual intrusive-list node) is still reported as a leak.
The new path follows the existing possibleUsage handling for retain(p). That covers the merge of branches, a later p = 0, an explicit return and free afterwards. For every variant I tried, the PR's result matched what the base already gives for a plain retain(p). Real leaks stay reported when the address is used in a comparison, an arithmetic expression, a !, a scalar cast or a discarded comma operand. Leak-ignore functions such as memset still report the leak as before. One small FN that I left out of the comments: sizeof(&p->x) / _Alignof(&p->x) now count as a possible use. This matches what base already does for sizeof(p).
Testing
- I built
cppcheckandtestrunnerinpr/andcppcheckinbase/; both builds succeeded. ./testrunner TestLeakAutoVar: 157 tests, 0 failed, 10 todos.- I wrote scratch C and C++ files and ran both binaries with
cppcheck -qto compare their output:- Suppressed now, as intended:
retain(&p->x)with a following statementops->cb(&p->x)l.add(&p->x)in C++retain(1, (&p->x))struct Uof unknown type&p->ptrwith a pointer member in C++- the same in a template function
- Still reported, same as base:
&p->arr[0]c ? &p->x : 0&(*p).x&p[0].x&p->x + 1!&p->xstd::make_pair(1, &p->x)new int(&p->x == nullptr)- a second, unrelated allocation
qin the same function
- Behaves like base does for
retain(p):- the conditional
if (c) retain(&p->x); retain(&p->x); p = 0;retain(&p->x); return;retain(&p->x); free(p);
- the conditional
- Small FN, same as base for
sizeof(p):*st = sizeof(&p->x);and_Alignof(&p->x)no longer give a memleak, becausesizeof(is handled as a function call. Base already suppresses*st = sizeof(p);the same way.
- Suppressed now, as intended:
- I did not run the CLI Python tests because the PR does not touch them.
Automated review of 264fe96 by Claude. The pull request was built and tested locally.
Passing an embedded list node to a retaining function currently leaves its containing allocation classified as unused, producing the C memory-leak false positive in Trac #6259. A following unrelated statement exposes the issue; a final unknown call can already suppress it through a separate fallback.
Record possible usage of the allocation when a call receives the address of one of its embedded members. Walk member/type information to find the owning raw-pointer variable, stopping at a separately pointed-to object. Require the address to be the complete argument value, optionally through pointer casts. Keep allocation/deallocation history intact, and respect leak-ignore and pure-function configuration.
The C++ path accepts only known scalar/pointer member types and rejects static/reference storage and overloaded member access. Class, enum and unknown C++ terminal member types are conservatively excluded to avoid assuming built-in
operator&semantics. This extends possible-use handling; it does not prove that an unknown callee retains or frees the allocation. Existing--check-libraryconfiguration information remains appropriate for unmodeled calls.Validation on Windows with Clang 22.1.8 and CMake/Ninja (Debug, PCH disabled, serial build):
git diff --checkpasses. GUI and non-Windows CI have not been run locally.Please assign this work to KiritoYG if needed and consider it under the published $10 bounty schedule, subject to acceptance and the required ticket closure. Please confirm eligibility and the supported settlement route; GitHub Sponsors is available if accepted. No award or payment is being claimed.