mgehre/llvm-project

Assigning to `values[0]` is different than assigning to `*values`

Closed this issue · 4 comments

Reduced example from my code base:

int f(const char** values)
{
    values[0] = "hello";
    return 0;
}

emits the warning:

foo.cxx:4:5: warning: returning a dangling pointer as output value '*values' [-Wlifetime]
    return 0;
    ^~~~~~~~
foo.cxx:1:7: note: it was never initialized here
int f(const char** values)
      ^~~~~~~~~~~~~~~~~~~
1 warning generated.

I'm not sure what's going on here, but this function is fine.

const char** values is interpreted to be an output parameter, i.e. we expect the callsite to be like

char* v;
f(&v);
use(*v);

Thus for this output parameter, the function f needs to set *values to something valid.
Slightly modifying this example to

int f(const char** values)
{
    if (!values)
      return 0;
    *values = "hello";
    return 0;
}

seems to work: https://www.godbolt.org/z/MxEGnL

I fear that we are not correctly interpreting values[0] to initialize the Pointer (while we do it correctly for *values).
Additionally, we also don't provide a possible null deref warning for values[0] while we do for *values.

To add some context, the code in question is more like this:

char const* storage[10];
auto size = populate_this_array_for_me(storage);

That is, the function parameter is a char const** only because it can't take a char const*(&)[] (C++20 will allow references to arrays of unknown bound, at which point that becomes the best choice, or maybe span<char const*>, but until then...)

I finally found the real issue: value[0] is identified as pointer arithmetic, and thus ignored.
One gets the appropriate warning with -Wlifetime-disabled (https://www.godbolt.org/z/Ow9n2Y)
The lifetime profile builds on top of the type and bounds profiles, (https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#SS-bounds), which disallow array indexing into pointers. They suggest to use a gsl::span/std::span or similar custom type instead.

Our implementation detects the array subscript, but only disables part of the analysis (the subscript itself is not checked for validity), but it should also disable the the warning for returning a dangling pointer as output value.

Passing a std::array or std::vector by reference might also work in your case.