Fix #14990: FP memleak with function call through function pointer

[aadanen]

Modify checkleakautovar to consider calls to lambdas and function pointers as function calls that can allow resources to escape the current scope.

[aadanen]

I am suprised to see all these segmentation faults. In my testing everything seemed to work nicely but I guess it was just crashing silently and working properly. I will need to modify functionCall() more heavily to not send nullptr to the library functions. This kinda undermines my idea of reusing functionCall for anonymous functions but I still think it is better than duplicating the function.

[aadanen]

I switched from the makefile to cmake and ran the CI workflows locally so I think next time they run, they should pass. Sorry about that. Also I noticed that git was using my work account, so I sorted that out too.

[danmar]

I am not sure why but there was significant slowdown in CI (selfchecking) after this?

[danmar]

in the checkers, a parenthesis token always has a link:

Suggested change

	tok->linkAt(-1) && !tok->isStandardType()) {
	!tok->isStandardType()) {

[aadanen]

isFunctionCall() has several similar checks such as
if (nameToken && nameToken->link() && !nameToken->isCast() && nameToken->str() == "(") {

are those also redundant? I can update my code now or we could do another PR to do a pass over the whole file.

[danmar]

the order is also important. I imagine that a string compare is more expensive than a pointer compare (a pointer compare is only 1 compare..) and in that code you showed there is only string compare if there is a link and it's a cast.. so the string compare will rarelly be executed. That string compare almost looks redundant to me actually but it does make it explicit about what the token should be..

[aadanen]

Okay, I cleaned up the comparisons, and tried to make the code/comments less verbose.

[aadanen]

Thanks for the review! I will look into it more on monday.

[aadanen]

I found the issue with the selfcheck slowdown. isAnonymousFunctionCall would sometimes link() backwards causing the checker to loop forever.

void f(char ch16) {
    ch = static_cast<unsigned char>(((ch16 >= 0x80) ? 0xff : ch16));
}

[aadanen]

the sanitizers/build check failed because of some network thing:
Connecting to apt.llvm.org (apt.llvm.org)|2a04:4e42:8a::561|:443... failed: Network is unreachable.
I think it probably just needs to be re-run.

Thanks!

[danmar]

This is an AI review take it with a grain of salt. Please feel free to reject it by clicking on Resolve button

I built this branch and its merge-base, and found a new false positive when the argument of the anonymous call deallocates the resource:

#include <stdio.h>
void a(void (*report)(int), const char *name) {
    FILE *f = fopen(name, "r");
    if (!f) return;
    (*report)(fclose(f));
}
typedef void (*fn_t)(int);
fn_t get_fn(void);
void b(const char *name) {
    FILE *f = fopen(name, "r");
    if (!f) return;
    get_fn()(fclose(f));
}

6: error deallocuse Dereferencing 'f' after it is deallocated / released
13: error deallocuse Dereferencing 'f' after it is deallocated / released

The merge-base gives no warning for either function. It looks like the argument tokens are processed twice: once as the normal fclose(f) call, and then again by functionCall(nullptr, lpar, ...), which sees f after the dealloc. Could you add these as test cases?

The other shapes I tried gave no new warnings: (*cb)(p); free(p);, (cb)(NULL); free(p);, if ((*cb)(1)) { free(p); return; }, (free)(p);, (*cb)(1) + (free(p), 0), (*cb)(1), fclose(f);.

[aadanen]

My change didn't introduce a new false positive, but brought the behavior of function pointer calls into line with the current behavior of normal function calls.

void b(const char *name) {
    FILE *f = fopen(name, "r");
    if (!f) return;
    foo(fclose(f));
}

the code above throws the false positive deallocuse on main.

This seems to me like it should be a separate pull request and bug report because it isn't really part of fixing the #14990 FP memleak. I understand if that fix needs to land first before we merge this one, but I don't currently have the cycles to look into it.