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

[Jonathan Roelofs via cfe-dev]

On 6/21/16 11:28 PM, Riyaz Puthiyapurayil via cfe-dev 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?
>

How about a project-specific clang-tidy check that looks for uses of the 
latter constructor with the corresponding dereference?


Jon

>
>
>
>
>
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>

-- 
Jon Roelofs
jonathan at codesourcery.com
CodeSourcery / Mentor Embedded