dcoughlin added a comment.
Gabor, thanks for taking a look at this!
I'm not sure `RegionStore::getBinding()` is the right place to put this logic.
First, not all dereferences of a `std::nullptr_t` value go through
`getBinding()`, so, for example, the following snippet triggers the assertion
even with your patch:
decltype(nullptr) returnsNullPtrType();
void fromReturnType() {
((X *)returnsNullPtrType())->f(); // Crash in analysis!
}
Second, not all locations of type `std::nullptr_t` contain `0`. For example,
the following prints "1" on my machine:
#include <iostream>
int main() {
decltype(nullptr) x;
std::cout << (bool)x << "\n";
}
That is, a default-initialized (garbage) nullptr_t may not be '0'. Prior to
your patch the analyzer would warn here about `x` being uninitialized (when the
analyzer is compiled without asserts) -- but with your patch we lose that
warning.
What do you think about moving your new logic to the symbol value creation
methods in SValBuilder? More specifically, you could add the check for
nullptr_t to `getRegionValueSymbolVal()`, `conjureSymbolVal()` (both variants),
`getConjuredHeapSymbolVal()`, and `getDerivedRegionValueSymbolVal()`. This
would let the analyzer warn when it knows there is a garbage value in a
location of type nullptr_t but still optimistically assume that the value is is
'0' when the value would otherwise be symbolic.
With this approach we would retain uninitialization warnings about accesses via
definitely uninitialized storage:
void fromUninitializedLocal() {
decltype(nullptr) p;
((X *)p)->f(); // Warn about p being uninitialized
}
void fromUninitializedLocalStruct() {
Type t;
((X *)t.x)->f(); // Warn about t.x being uninitialized.
}
but would warn about about a null access in `returnsNullPtrType()` and would
keep the behavior you specify in your `f()` test because `p` is a parameter
(and thus its value is symbolic).
http://reviews.llvm.org/D15007
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
