[PATCH] Warn when NULL is returned from 'operator new' without 'throw()'

[Artyom Skrobov]

Hello Richard,

Would you please be able to review my updated patch?
(attaching it again, just in case)



From: metafoo at gmail.com [mailto:metafoo at gmail.com] On Behalf Of Richard
Smith
Sent: 10 January 2014 23:25
To: Artyom Skrobov
Cc: cfe commits
Subject: Re: [PATCH] Warn when NULL is returned from 'operator new' without
'throw()'

On Fri, Jan 10, 2014 at 5:07 AM, Artyom Skrobov <Artyom.Skrobov at arm.com>
wrote:
Thank you for your suggestions Richard!
One point though:
> Please use RetValExp->isNullPointerConstant instead.
> Please also add testcases for operator new returning nullptr, and
returning expressions such as 1 - 1, and for operator new marked as
'noexcept'.
I want to note that expressions such as 1-1 are invalid as return values
from operator new, and produce "error: cannot initialize return object of
type 'void *' with an rvalue of type 'int'"

Expressions such as 1 - 1 are valid null pointer constants in C++98 but not
in C++11.
 
At the same time, expressions such as (void*)(1-1) are not recognized either
by isNullPointerConstant or by EvaluateAsInt as integer zeroes, namely
because they are not integers.

In C++11, (void*)(1 - 1) is a reinterpret_cast of 0 to void*, and isn't
(necessarily) a null pointer. In C++98, it's a static_cast, and is a null
pointer.
 
For a reference, GCC doesn't warn on

   void *operator new(size_t n) {
     return (void*)(1-1);
   }

which isn't too bad; but neither does it warn on

   void *operator new(size_t n) {
     void* blah = 0;
     return blah;
   }

OK, I would not expect a warning here.
 
nor even on

   void *operator new(size_t n) {
     return (void*)0;
   }

I *would* expect a warning here.
 
which seems quite valuable to be able to detect.

isNullPointerConstant doesn't recognize the two latter cases as null pointer
constants, either.

Sorry for sending you in the wrong direction!
 
Do you think Clang should be able to detect such null-pointer-expressions?
If so, could you advise how it could be implemented, seeing that
isNullPointerConstant and EvaluateAsInt prove ineffective?

It looks like the best way to achieve this is to use
Expr::EvaluateAsBooleanCondition on the (converted) return expression. That
matches what we do for __attribute__((nonnull)).
-------------- next part --------------
A non-text attachment was scrubbed...
Name: operator-new-NULL.updated.patch
Type: application/octet-stream
Size: 6048 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140115/7f54944e/attachment.obj>