Allowing redundant null-pointer check at trust boundary

user_5603247 0
Viewed 212

I was recently looking at some code where clang generated warnings due to -Wtautological-pointer-compare.

The code could be simplified to something like:

void foo(const char*s) __attribute__((nonnull)) {
   if (s) { /* Test added just in case*/
      if (s[0]=='a') s[0]='b'; /* Dummy code using the pointer */
   }
}

Obviously if we trust the attributes then s cannot be null and the warning is redundant. However, to me it seems best to handle the null-case in function (since we cannot trust that the calling code is compiled with these warnings, or that people read warnings) - while still detecting other null-pointer issues in the code.

Thus disabling this warning (using a pragma for the entire function) seems sub-optimal.

In Visual Studio with SAL it seems you can use _In_ _Pre_defensive_ to handle this case.

In that case, _In_ _Pre_defensive_ is preferred at a trust boundary to indicate that although a caller will get an error if it attempts to pass NULL, the function body will be analyzed as if the parameter might be NULL, and any attempts to de-reference the pointer without first checking it for NULL will be flagged.

Is something similar possible with clang?

static-analysis clang c
1 Answers

Note that the issue is worse than just seeing an unwanted warning. Since the function has that attribute, the compiler will remove the if as if you wrote:

if (true)

because you promised that the pointer will not be NULL. So your null check has no effect. See:

https://godbolt.org/z/l8w4x1

int func(void* ptr) __attribute__((nonnull))
{
    if (ptr)
        return 1;
    return 0;
}

It unconditionally returns 1:

mov eax, 1
ret

So you should take that warning seriously.

I don't know of any workaround for this other than compiling with -fno-delete-null-pointer-checks to prevent null pointer checks from being optimized out, and -Wno-tautological-pointer-compare to silence the warning. You don't want to use these flags globally, obviously. So you should consolidate functions with this attribute into their own source file and use these flags only when compiling that file.

user_8561990
Related