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

[Richard Smith]

This warning seems like a really nice idea, thanks!

+def warn_operator_new_returns_null : Warning<
+  "%0 must not return NULL unless it is declared 'throw()'">,
+  InGroup<OperatorNewReturnsNull>;

Rather than "NULL", please say "a null pointer", and please say "declared
'throw()' or 'noexcept'" when we're in C++11 mode.

+      llvm::APSInt ReturnValue;
+
+      if (!Proto->isNothrow(Context) &&
+          RetValExp->EvaluateAsInt(ReturnValue, Context,
+                                   Expr::SE_AllowSideEffects) &&
+          ReturnValue == 0)
+      llvm::APSInt ReturnValue;
+
+      if (!Proto->isNothrow(Context) &&
+          RetValExp->EvaluateAsInt(ReturnValue, Context,
+                                   Expr::SE_AllowSideEffects) &&
+          ReturnValue == 0)

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'.


On Thu, Jan 9, 2014 at 12:03 PM, Artyom Skrobov <Artyom.Skrobov at arm.com>wrote:

> Hello,
>
> C++03 section 3.7.3.1 item 3 specifies that "If an allocation function
> declared with an empty exception-specification, throw(), fails to allocate
> storage, it shall return a null pointer. Any other allocation function that
> fails to allocate storage shall only indicate failure by throwing an
> exception of class std::bad_alloc or a class derived from std::bad_alloc."
>
> GCC does indeed issue a warning "'operator new' must not return NULL unless
> it is declared 'throw()' (or -fcheck-new is in effect)" for blatant
> violations of the aforementioned clause. Clang didn't support such a
> warning, and implementing the support for -fcheck-new was dismissed in 2012
> as "dubious" :
> http://lists.cs.uiuc.edu/pipermail/cfe-dev/2012-November/025646.html
>
> There is a related bugzilla ticket at
> http://llvm.org/bugs/show_bug.cgi?id=16557 where Eric van Gyzen approaches
> the issue at hand from a different direction: inserting runtime checks to
> ensure the code is "safe" (i.e. not calling the constructor on a null
> pointer) even when it's blatantly wrong as per above. We aren't sure about
> the validity of that approach (catering for the incorrect code, in addition
> to accepting it silently); so instead, we propose a patch that adds the
> missing warning, matching GCC's.
>
> OK to commit?
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140109/5c998433/attachment.html>