llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
26.78k stars 10.97k forks source link

False Positive - null dereference in clang-tidy #88356

Open mushenoy opened 2 months ago

mushenoy commented 2 months ago

A False positive in clang-tidy for below sample program.

sample.c

#include <stdio.h>
#include <stdlib.h>
#include <dlfcn.h>

double (*cosine)(double) = NULL;
extern void test (void);

int
main(int argc, char **argv)
{
    void *handle;

    handle = dlopen("libtest.so", RTLD_LAZY);
    if (!handle) {
        exit(EXIT_FAILURE);
    }

   cosine = dlsym(handle, "cos");
   if (cosine) {
       test();
       printf("%f\n", cosine (2.0));
    }
    dlclose(handle);
    exit(EXIT_SUCCESS);
}

Warnings:

clang-tidy --checks="clang-analyzer-*" /tmp/sample.c
Error while trying to load a compilation database:
Could not auto-detect compilation database for file "/tmp/sample.c"
No compilation database found in /tmp or any parent directory
fixed-compilation-database: Error while opening fixed database: No such file or directory
json-compilation-database: Error while opening JSON database: No such file or directory
Running without flags.
1 warning generated.
/tmp/sample.c:21:23: warning: Called function pointer is null (null dereference) [clang-analyzer-core.CallAndMessage]
   21 |        printf("%f\n", cosine (2.0));
      |                       ^~~~~~
/tmp/sample.c:14:9: note: Assuming 'handle' is non-null
   14 |     if (!handle) {
      |         ^~~~~~~
/tmp/sample.c:14:5: note: Taking false branch
   14 |     if (!handle) {
      |     ^
/tmp/sample.c:19:8: note: Assuming 'cosine' is non-null
   19 |    if (cosine) {
      |        ^~~~~~
/tmp/sample.c:19:4: note: Taking true branch
   19 |    if (cosine) {
      |    ^
/tmp/sample.c:20:8: note: Null pointer value stored to 'cosine'
   20 |        test();
      |        ^~~~~~
/tmp/sample.c:21:23: note: Called function pointer is null (null dereference)
   21 |        printf("%f\n", cosine (2.0));
      |                       ^~~~~~

Version clang-tidy --version LLVM (http://llvm.org/): LLVM version 19.0.0

Observations:

llvmbot commented 2 months ago

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

Author: Mukund Shenoy (mushenoy)

A False positive in clang-tidy for below sample program. **sample.c** ``` #include <stdio.h> #include <stdlib.h> #include <dlfcn.h> double (*cosine)(double) = NULL; extern void test (void); int main(int argc, char **argv) { void *handle; handle = dlopen("libtest.so", RTLD_LAZY); if (!handle) { exit(EXIT_FAILURE); } cosine = dlsym(handle, "cos"); if (cosine) { test(); printf("%f\n", cosine (2.0)); } dlclose(handle); exit(EXIT_SUCCESS); } ``` **Warnings**: ``` clang-tidy --checks="clang-analyzer-*" /tmp/sample.c Error while trying to load a compilation database: Could not auto-detect compilation database for file "/tmp/sample.c" No compilation database found in /tmp or any parent directory fixed-compilation-database: Error while opening fixed database: No such file or directory json-compilation-database: Error while opening JSON database: No such file or directory Running without flags. 1 warning generated. /tmp/sample.c:21:23: warning: Called function pointer is null (null dereference) [clang-analyzer-core.CallAndMessage] 21 | printf("%f\n", cosine (2.0)); | ^~~~~~ /tmp/sample.c:14:9: note: Assuming 'handle' is non-null 14 | if (!handle) { | ^~~~~~~ /tmp/sample.c:14:5: note: Taking false branch 14 | if (!handle) { | ^ /tmp/sample.c:19:8: note: Assuming 'cosine' is non-null 19 | if (cosine) { | ^~~~~~ /tmp/sample.c:19:4: note: Taking true branch 19 | if (cosine) { | ^ /tmp/sample.c:20:8: note: Null pointer value stored to 'cosine' 20 | test(); | ^~~~~~ /tmp/sample.c:21:23: note: Called function pointer is null (null dereference) 21 | printf("%f\n", cosine (2.0)); | ^~~~~~ ``` **Version** clang-tidy --version LLVM (http://llvm.org/): LLVM version 19.0.0 **Observations:** - Warnings are not reported if the cosine function ptr is made as a local variable - Warnings are not reported if the test function definition is provided - Warnings are not reported if the test() function call is removed - Warnings are not reported if the “= NULL” is removed in line 5. - Warnings are still reported even if the function ptr is made as static variable
steakhal commented 2 months ago

This case could be reduced into this:

void escape_golbals();
void (*fn)() = (void*)0;
int main() {
    escape_golbals();
    fn();
}

We assume that from main, even the mutable global variable initializers hold (in C). Technically that assumption doesn't hold even for C, in the presence of __attribute__((destructor)). Personally I'd just drop special-casing main for trusting mutable global inits.

Basically, we can't differentiate now if we don't have a binding because 1) We don't know anything about the variable, or 2) because we invalidated the it (globals in this case). This leads RegionStoreManager::getBindingForVar hitting the branch for If we're in main(), then global initializers have not become stale yet.

To fix this we would need to either

The first option could be done fairly easily, if we had invalidation artifacts. In that case, we could just check if e.g. the internal system memory space was ever invalidated.

My guess would be that this special casing for main must be there for a reason - which I don't know ofc. So, I'd prefer the first option for fixing this.