Detecting a Memory leak when using __attribute__((__cleanup__(<method>)))

Clean Code Report False-positive / False-negative...
1

For a C (gcc 8.5) project we utilize a AUTO FREE Macro that is getting hit by c:S3584 on sonarQube 10.3.0.82913. This memory is cleaned up when the pointer goes out of scope but every use of it is indicated by the last use of the variable.

Relevant Code:

#define AUTO_FREE_MACRO __attribute__((__cleanup__(auto_free)))
void auto_free(void* var);

void auto_free(void* var)
{
    void* ptr = *(void**)var;
    free(ptr);
    ptr = 0;
}

Example Usage:

int* some_int_array AUTO_FREE_MACRO = (int*)malloc(MAX_SIZE * sizeof(int));

The line immediately after the last usage of some_int_array comes back as a Potential leak of memory which I believe to be a false positive. I obviously don’t want to ignore this rather important rule so looking for some suggestions/advice.

We are building using the build wrapper in a gitlab pipeline.

2

This syntax seems to fix it…

void auto_free(void** var) 
{
    free(*var);
    *var = NULL;
}

However If I make functions for each type eg: int** the error comes back. The problem with this solution is the compiler starts complaining about the type mismatch between the various types.

I tried to make type specific implementations but then sonarqube started detecting it as a memory leak again.

4

Hello @krb8686,

Thanks for the report. This attribute is not well supported by the symbolic execution engine, so the rule does not “see” the free happening.

I have created a ticket [CPP-4983] - Jira to follow up on this.

We need to think about this carefully and see whether we modify this rule to make it aware of __cleanup__ attributes, or if we add support for it on the engine. The former is easier, and the latter is more complete as it could catch accidental double frees as well, but it is more involved.

As for workarounds, I can not think of any option besides disabling the rule, unfortunately.

1 Like
6

Thank you for confirming the issue and sharing the ticket. I don’t find ignoring this rule acceptable cause it would cause too many missed issues in this legacy code so knowing this is something you guys might be able to add is great news.