False-positive: fopen() without fclose() in main() reported as a "resource leak" clang:static analyzer

[alavrentiev]

int main(int argc, char** argv)
{
    int   port;
    FILE* fp;

    /* Cmd.-line args */
    if (argc != 4  ||
        (fp = fopen(argv[1], "r")) == 0  ||
        ((port = atoi(argv[3])) & ~0xFFFF)) {
        fputs("\nUSAGE:\n"
              "test_fw <inp_file> <host> <port>\n\n", stderr);
        return 1;  // <== Opened stream never closed. Potential resource leak
    }
   ...
   /* some work is done here */
   ...
}

Return from main() with an open file does not pose any resource leaks, as the file gets closed by C RTL right away.
It if was any other function, then yes, in such a situation the report would be justified and true, but not for main().

[llvmbot]

@llvm/issue-subscribers-clang-static-analyzer

Author: None (alavrentiev)

``` int main(int argc, char** argv) { int port; FILE* fp;

/* Cmd.-line args */
if (argc != 4  ||
    (fp = fopen(argv[1], "r")) == 0  ||
    ((port = atoi(argv[3])) & ~0xFFFF)) {
    fputs("\nUSAGE:\n"
          "test_fw <inp_file> <host> <port>\n\n", stderr);
    return 1;  // <== Opened stream never closed. Potential resource leak
}

...
/* some work is done here */
...
}

Return from `main()` with an open file does not pose any resource leaks, as the file gets closed by C RTL right away.
It if was any other function, then yes, in such a situation the report would be justified and true, but *not* for `main()`.

</details>

[chrchr-github]

What if main() leaked e.g. 1000 handles in a loop? Is there a reason not to use fclose()?

[firewave]

If this would occur in code run with fork() or exec*(), wouldn't that leak still manifest itself?

[alavrentiev]

If this would occur in code run with fork() or exec*(), wouldn't that leak still manifest itself?

@firewave : the report is specifically about return from main(); not any other ways to leave it.

[alavrentiev]

What if main() leaked e.g. 1000 handles in a loop?

Without a FILE* variable reassigned or reinstated (loop, block, etc) there's no leak in main(), and that is easy to logically distinguish from any other situation / function.

So if main() has an array of 1000 FILE*s and opens them all, then does some work, and returns, there's no leak, either.

Is there a reason not to use fclose()?

Yes, there is. If main() has multiple (many!) exit points (returns) while there's a FILE* previously opened, there's no need to use fclose() at each and every one of them, because that's what the language offers to do.

[firewave]

@firewave : the report is specifically about return from main(); not any other ways to leave it.

exec*() and spawn*() can be used to run another executable from a process. I am wondering if file descriptors or memory from those subprocesses might be inherited by the parent process and thus accumulate the leaks from subprocesses which do not properly clean up.

[alavrentiev]

I am wondering if file descriptors or memory from those subprocesses might be inherited by the parent process and thus accumulate the leaks from subprocesses which do not properly clean up.

@firewave : none of these calls create a "subprocess" (in the meaning that it's some sort of being a part of the original one). exec() completely replaces the entire process memory with a new image (file descriptors remain open, but NOT FILE*). fork() splits (copies) the process into two independent entities, each with its own memory. So whatever the copy does with its resources is not going to affect the original (parent) process, and vice versa.

But let us not digress from the issue at hand, which is: when main() returns then the leakage of FILE*s (or file descriptors) should be handled more gratuitously than in any other C function under the same circumstances.

[firewave]

So whatever the copy does with its resources is not going to affect the original (parent) process, and vice versa.

Thanks for elaborating. I have only ever used it in the most basic and default way and I am not familiar with all the behavior implied by the various variants and flags.

I brought this up because I have seen issues where file descriptor exhaustion was trigger by underlying processes. It has been a while though so my memory is very faint.

[steakhal]

Usually, special-casing main leads to marginal benefits because it's a tiny narrow subset of the cases when we ever see main.
W.r.t. this bug, it appears to me that it's also fairly easy to spot this sort of "false-positive" so it shouldn't be a huge pain point.
This makes this for me a less important issue.

We could however check if we are directly in main before constructing the bug report and just skip it if that's the case. That should be fairly easy to implement.

[steakhal]

We could however check if we are directly in main before constructing the bug report and just skip it if that's the case. That should be fairly easy to implement.

@Flandini Do you wanna give it a try?

[vakatov]

when main() returns then the leakage of FILE*s (or file descriptors) should be handled more gratuitously than in any other C function under the same circumstances.

Because the Static Analyzer (SA) may actually (in the future, if not already) analyze code pathways which run through different functions (and even different compilation units), then -- won't the suggested approach basically mean that SA must not track any FILE*'s (or bare file descriptors), period? Also, will it include sockets, etc?

[alavrentiev]

@firewave

where file descriptor exhaustion was trigger by underlying processes

JFTR, there may be a system-wide limit of simultaneously open files, which can affect any process, regardless of their hierarchal dependencies.