Skip to content

Fix Trac #6259: recognize possible use of embedded member addresses #8891

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
KiritoYG wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix Trac #6259: recognize possible use of embedded member addresses #8891

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 54 additions & 1 deletion lib/checkleakautovar.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1016,10 +1016,56 @@ void CheckLeakAutoVarImpl::changeAllocStatus(VarInfo &varInfo, const VarInfo::Al
}
}

static const Token* addressedMemberOwner(const Token* arg, const Token* argStart, const Token* callOpening)
{
if (!arg || !arg->isUnaryOp("&"))
return nullptr;
const Token* nextArg = argStart->nextArgument();
const Token* argEnd = nextArg ? nextArg->previous() : callOpening->link();
const Token* parent = arg->astParent();
// The address must be the passed value, not a comparison, discarded
// comma operand, or scalar cast. Commas outside this argument are separators.
while (parent && parent->index() >= argStart->index() && parent->index() < argEnd->index()) {
if (!parent->isCast() || !parent->valueType() || !parent->valueType()->pointer)
return nullptr;
parent = parent->astParent();
}
if (parent != callOpening && !Token::simpleMatch(parent, ","))
return nullptr;
const Token* member = arg->astOperand1();
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;
}
Comment on lines +1036 to +1042

@danmar danmar Oct 1, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

while (Token::simpleMatch(member, ".")) {
const Token* field = member->astOperand2();
if (!field || (field->variable() && (field->variable()->isStatic() || field->variable()->isReference())))
return nullptr;
const Token* object = member->astOperand1();
const ValueType* vt = object ? object->valueType() : nullptr;
if (!vt)
return nullptr;
if (vt->pointer) {
// Stop at a pointer member: its pointee is not embedded storage
// of the allocation containing that member.
return vt->pointer == 1 && object->variable() && object->isName() ? object : nullptr;
}
if (member->originalName() == "->")
return nullptr; // overloaded member access
member = object;
}
return nullptr;
}

void CheckLeakAutoVarImpl::functionCall(const Token *tokName, const Token *tokOpeningPar, VarInfo &varInfo, const VarInfo::AllocInfo& allocation, const Library::AllocFunc* af)
{
// Ignore function call?
const bool isLeakIgnore = mSettings.library.isLeakIgnore(mSettings.library.getFunctionName(tokName));
const std::string functionName = mSettings.library.getFunctionName(tokName);
const bool isLeakIgnore = mSettings.library.isLeakIgnore(functionName);
const bool isPure = mSettings.library.isFunctionConst(functionName, true);
if (mSettings.library.getReallocFuncInfo(tokName))
return;
if (tokName->next()->valueType() && tokName->next()->valueType()->container && tokName->next()->valueType()->container->stdStringLike)
Expand Down Expand Up @@ -1054,6 +1100,13 @@ void CheckLeakAutoVarImpl::functionCall(const Token *tokName, const Token *tokOp
arg = arg->astOperand2() ? arg->astOperand2() : arg->astOperand1();
const Token * const argTypeStartTok = arg;

if (!isLeakIgnore && !isPure && allocation.status == VarInfo::NOALLOC) {
if (const Token* owner = addressedMemberOwner(arg, funcArg, tokOpeningPar)) {
if (varInfo.alloctype.count(owner->varId()))
varInfo.possibleUsage[owner->varId()] = {tokName, VarInfo::USED};
}
}

if (Token::simpleMatch(arg, "."))
arg = arg->next();

Expand Down
241 changes: 241 additions & 0 deletions test/testleakautovar.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,15 @@ class TestLeakAutoVar : public TestFixture {

// handling function calls
TEST_CASE(functioncall1);
TEST_CASE(functioncallMemberAddress);
TEST_CASE(functioncallMemberAddressOwnership);
TEST_CASE(functioncallMemberAddressValue);
TEST_CASE(functioncallMemberAddressDerived);
TEST_CASE(functioncallMemberAddressIndirect);
TEST_CASE(functioncallMemberAddressLeakIgnore);
TEST_CASE(functioncallMemberAddressDoubleFree);
TEST_CASE(functioncallMemberAddressNested);
TEST_CASE(functioncallMemberAddressCppStorage);

// goto
TEST_CASE(goto1);
Expand Down Expand Up @@ -1910,6 +1919,238 @@ class TestLeakAutoVar : public TestFixture {
ASSERT_EQUALS("[test.cpp:4:1]: (error) Memory leak: b [memleak]\n", errout_str());
}

void functioncallMemberAddress() { // #6259
check("void f() {\n"
" line_element *wall = malloc(sizeof(line_element));\n"
" list_add_tail(&(wall->list), &(state.l_elements_head));\n"
" state.l_direction = direction;\n"
"}\n");
ASSERT_EQUALS("[test.c:5:1]: (information) --check-library: Function list_add_tail() should have <use>/<leak-ignore> configuration [checkLibraryUseIgnore]\n", errout_str());

check("struct S { int value; };\n"
"void retain(void *);\n"
"void f(int *state) {\n"
" struct S *p = malloc(sizeof(*p));\n"
" retain(&p->value);\n"
" *state = 1;\n"
"}\n");
ASSERT_EQUALS("[test.c:7:1]: (information) --check-library: Function retain() should have <use>/<leak-ignore> configuration [checkLibraryUseIgnore]\n", errout_str());

check("struct S { int value; };\n"
"void retain(void *);\n"
"void f(int *state) {\n"
" struct S *p = malloc(sizeof(*p));\n"
" retain(&(p->value));\n"
" *state = 1;\n"
"}\n");
ASSERT_EQUALS("[test.c:7:1]: (information) --check-library: Function retain() should have <use>/<leak-ignore> configuration [checkLibraryUseIgnore]\n", errout_str());

check("struct S { int value; };\n"
"void retain(void *);\n"
"void f(int *state) {\n"
" struct S *p = malloc(sizeof(*p));\n"
" retain((void *)&p->value);\n"
" *state = 1;\n"
"}\n");
ASSERT_EQUALS("[test.c:7:1]: (information) --check-library: Function retain() should have <use>/<leak-ignore> configuration [checkLibraryUseIgnore]\n", errout_str());
}

void functioncallMemberAddressOwnership() {
check("struct link { struct link *next; };\n"
"struct item { int value; struct link entry; };\n"
"struct link *head;\n"
"void retain_entry(struct link *entry) {\n"
" entry->next = head;\n"
" head = entry;\n"
"}\n"
"void append_item(int value, int *state) {\n"
" struct item *p = malloc(sizeof(*p));\n"
" if (!p) return;\n"
" p->value = value;\n"
" retain_entry(&p->entry);\n"
" *state = 1;\n"
"}\n");
ASSERT_EQUALS("", errout_str());
}

void functioncallMemberAddressDerived() {
check("struct S { int value; };\n"
"void observe(int);\n"
"void f(int *saved, int *state) {\n"
" struct S *p = malloc(sizeof(*p));\n"
" observe(&p->value == saved);\n"
" *state = 1;\n"
"}\n");
ASSERT_EQUALS("[test.c:7:1]: (error) Memory leak: p [memleak]\n", errout_str());

check("struct S { int value; };\n"
"void observe(int);\n"
"void f(int *state) {\n"
" struct S *p = malloc(sizeof(*p));\n"
" observe((&p->value, 1));\n"
" *state = 1;\n"
"}\n");
ASSERT_EQUALS("[test.c:7:1]: (error) Memory leak: p [memleak]\n", errout_str());

check("struct S { int value; };\n"
"void observe(long);\n"
"void f(int *state) {\n"
" struct S *p = malloc(sizeof(*p));\n"
" observe((long)&p->value);\n"
" *state = 1;\n"
"}\n");
ASSERT_EQUALS("[test.c:7:1]: (error) Memory leak: p [memleak]\n", errout_str());
}

void functioncallMemberAddressValue() {
check("struct S { int value; };\n"
"void observe(int);\n"
"void f(int *state) {\n"
" struct S *p = malloc(sizeof(*p));\n"
" p->value = 1;\n"
" observe(p->value);\n"
" *state = 1;\n"
"}\n");
ASSERT_EQUALS("[test.c:8:1]: (error) Memory leak: p [memleak]\n", errout_str());
}

void functioncallMemberAddressIndirect() {
check("struct S { int value; struct S *child; };\n"
"void retain(int *);\n"
"void f(struct S *child, int *state) {\n"
" struct S *p = malloc(sizeof(*p));\n"
" p->child = child;\n"
" retain(&p->child->value);\n"
" *state = 1;\n"
"}\n");
ASSERT_EQUALS("[test.c:8:1]: (error) Memory leak: p [memleak]\n", errout_str());

check("struct S { int *value; };\n"
"void retain(int *);\n"
"void f(int *value, int *state) {\n"
" struct S *p = malloc(sizeof(*p));\n"
" p->value = value;\n"
" retain(p->value);\n"
" *state = 1;\n"
"}\n");
ASSERT_EQUALS("[test.c:8:1]: (error) Memory leak: p [memleak]\n", errout_str());
}

void functioncallMemberAddressLeakIgnore() {
check("struct S { int value; };\n"
"void f(int *state) {\n"
" struct S *p = malloc(sizeof(*p));\n"
" memset(&p->value, 0, sizeof(p->value));\n"
" *state = 1;\n"
"}\n");
ASSERT_EQUALS("[test.c:6:1]: (error) Memory leak: p [memleak]\n", errout_str());

const Settings pureSettings = settingsBuilder(settings).libraryxml(
"<def format=\"2\"><function name=\"observe\"><pure/><noreturn>false</noreturn>"
"<arg nr=\"1\"><not-uninit/></arg></function></def>").build();
check("struct S { int value; };\n"
"int observe(const int *);\n"
"void f(int *state) {\n"
" struct S *p = malloc(sizeof(*p));\n"
" p->value = 1;\n"
" *state = observe(&p->value);\n"
" *state += 1;\n"
"}\n", dinit(CheckOptions, $.s = &pureSettings));
ASSERT_EQUALS("[test.c:8:1]: (error) Memory leak: p [memleak]\n", errout_str());
}

void functioncallMemberAddressDoubleFree() {
check("struct S { int value; };\n"
"void retain(int *);\n"
"void f() {\n"
" struct S *p = malloc(sizeof(*p));\n"
" retain(&p->value);\n"
" free(p);\n"
" free(p);\n"
"}\n");
ASSERT_EQUALS("[test.c:6:5] -> [test.c:7:5]: (error) Memory pointed to by 'p' is freed twice. [doubleFree]\n", errout_str());

check("struct S { int value; };\n"
"void retain(int *);\n"
"void f() {\n"
" S *p = new S;\n"
" retain(&p->value);\n"
" free(p);\n"
"}\n", dinit(CheckOptions, $.cpp = true));
ASSERT_EQUALS("[test.cpp:4:12] -> [test.cpp:6:5]: (error) Mismatching allocation and deallocation: p [mismatchAllocDealloc]\n", errout_str());
}

void functioncallMemberAddressNested() {
check("struct Entry { int value; };\n"
"struct S { struct Entry embedded; };\n"
"void retain(int *);\n"
"void f(int *state) {\n"
" struct S *p = malloc(sizeof(*p));\n"
" retain(&p->embedded.value);\n"
" *state = 1;\n"
"}\n");
ASSERT_EQUALS("[test.c:8:1]: (information) --check-library: Function retain() should have <use>/<leak-ignore> configuration [checkLibraryUseIgnore]\n", errout_str());
}

void functioncallMemberAddressCppStorage() {
check("struct S { int value; };\n"
"void retain(int *);\n"
"void f(int *state) {\n"
" S *p = new S;\n"
" retain(&p->value);\n"
" *state = 1;\n"
"}\n", dinit(CheckOptions, $.cpp = true));
ASSERT_EQUALS("[test.cpp:7:1]: (information) --check-library: Function retain() should have <use>/<leak-ignore> configuration [checkLibraryUseIgnore]\n", errout_str());

check("struct S { int *value; };\n"
"void retain(int **);\n"
"void f(int *state) {\n"
" S *p = new S;\n"
" retain(&p->value);\n"
" *state = 1;\n"
"}\n", dinit(CheckOptions, $.cpp = true));
ASSERT_EQUALS("[test.cpp:7:1]: (information) --check-library: Function retain() should have <use>/<leak-ignore> configuration [checkLibraryUseIgnore]\n", errout_str());

check("struct S { static int value; };\n"
"void retain(int *);\n"
"void f(int *state) {\n"
" S *p = new S;\n"
" retain(&p->value);\n"
" *state = 1;\n"
"}\n", dinit(CheckOptions, $.cpp = true));
ASSERT_EQUALS("[test.cpp:7:1]: (error) Memory leak: p [memleak]\n", errout_str());

check("struct S { int &value; };\n"
"void retain(int *);\n"
"void f(int &value, int *state) {\n"
" S *p = new S{value};\n"
" retain(&p->value);\n"
" *state = 1;\n"
"}\n", dinit(CheckOptions, $.cpp = true));
ASSERT_EQUALS("[test.cpp:7:1]: (error) Memory leak: p [memleak]\n", errout_str());

check("struct Entry { int *operator&(); };\n"
"struct S { Entry entry; };\n"
"void retain(int *);\n"
"void f(int *state) {\n"
" S *p = (S *)malloc(sizeof(S));\n"
" retain(&p->entry);\n"
" *state = 1;\n"
"}\n", dinit(CheckOptions, $.cpp = true));
ASSERT_EQUALS("[test.cpp:8:1]: (error) Memory leak: p [memleak]\n", errout_str());

check("struct Entry { int value; };\n"
"struct Link { Entry *operator->(); };\n"
"struct S { Link link; };\n"
"void retain(int *);\n"
"void f(int *state) {\n"
" S *p = (S *)malloc(sizeof(S));\n"
" retain(&p->link->value);\n"
" *state = 1;\n"
"}\n", dinit(CheckOptions, $.cpp = true));
ASSERT_EQUALS("[test.cpp:9:1]: (error) Memory leak: p [memleak]\n", errout_str());
}

void goto1() {
check("static void f() {\n"
" int err = -ENOMEM;\n"
Expand Down
Loading