[cfe-dev] Can clang generate a warning for this?

[Richard Trieu via cfe-dev]

There's two suggestions for warnings here:
1) Warn that pointer p is given a pointer from somewhere and then
dereferenced without a null check.
2) Warn that two similar constructions only differing by pointer versus
reference, and should suggest the appropriate constructor.

1 is hard because the assignment and dereference can be separated by some
amount of intervening code that needs to be checked.  This requires at
least a path-sensitive checker.  There's also the case that the check is
performed by a different function, which further complicates the checking.
Also, getFooL() may have guarantees not visible to Clang.  If it already
returns a non-null pointer, then this warning would be a false positive.

2 requires the comparison of the structure of functions to determine that
they are identical except for the pointer/reference difference.  That would
be a very fragile check and even small changes to the constructors would
render Clang unable to see the equivalence between the two functions.

As you've noted, Clang does have specific warnings for very obvious and
very simple cases.  More complicated cases involving examining code across
different functions is difficult and time-consuming for Clang, if not
impossible if the functions are across different translation units.  If you
are concerned about this problem in the future, I recommend the dynamic
analysis tool Undefined Behavior Sanitizer (
http://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html).  Using the
null checker will allow you to catch the null dereference at runtime.


On Tue, Jun 21, 2016 at 10:28 PM, Riyaz Puthiyapurayil via cfe-dev <
cfe-dev at lists.llvm.org> wrote:

> [corrected some typos in the earlier email]
>
>
>
> We have some very old code that resembles the following that causes it to
> crash when compiled with clang (it “works” with gcc).  The problem is the
> dereference of ‘p’ in the constructor argument of fooLI below (line marked
> as [2]).  Due to this dereference, the constructor FooLI(const Foo<T>& foo)
> is invoked which in turn invokes FooBI(…). Since p has already been
> dereferenced, clang assumes that p is non-null (which is OK because
> well-formed C++ code would not dereference a NULL pointer). Clang therefore
> eliminates the null check on foo_ at line [1] assuming that it always
> non-NULL. So when a null pointer is returned by getFooL(), a crash occurs.
>
>
>
> class FooBI {
>
> public:
>
>     :
>
>     FooBI(const FooB* foo_) :
>
>         foo(foo_ && foo_->bar ? foo_ : NULL) {}   // [1]
>
>     :
>
> };
>
> template<class T>class FooLI : public FooBI {
>
> public:
>
>     :
>
>     FooLI(const FooL<T>* foo) :
>
>         FooBI(foo)
>
>     {}
>
>     FooLI(const Foo<T>& foo) :
>
>         FooBI(&foo)
>
>     {}
>
>     :
>
> };
>
> :
>
>     FooL<Boo>* p = getFooL();
>
>     FooLI fooLI = FooLI(*p); // [2] p can be sometimes NULL
>
>
>
> The fix for this problem in our code is to avoid the dereference on line
> [2] which will then invoke FooLI(const FooL<T>* foo) and the null check on
> line [1] will be retained. Is it possible for clang to generate a warning
> about this? -Wundefined-bool-conversion does generate warnings in similar
> situations but not specifically for this case. Perhaps, this should be a
> different warning. There are two choices of constructors, one accepting a
> pointer and another accepting a reference. When the pointer is then
> dereferenced and converted to a reference, do you think it merits a warning?
>
>
>
>
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20160623/33eee022/attachment.html>