[PATCH] analyzer: Fix -Wanalyzer-possible-null-argument warning

[Jonathan Wakely]

On 05/07/20 21:48 -0400, David Malcolm wrote:
>On Wed, 2020-07-01 at 18:29 +0100, Jonathan Wakely wrote:
>> On 30/06/20 17:43 +0100, Jonathan Wakely wrote:
>> > gcc/testsuite/ChangeLog:
>> >
>> > 	* g++.dg/analyzer/pr94028.C: Make operator new non-throwing so
>> > 	that the compiler doesn't implicitly mark it as returning
>> > 	non-null.
>> >
>> > Fixes these:
>> >
>> > FAIL: g++.dg/analyzer/pr94028.C  -std=c++98 (test for excess
>> > errors)
>> > FAIL: g++.dg/analyzer/pr94028.C  -std=c++14 (test for excess
>> > errors)
>> > FAIL: g++.dg/analyzer/pr94028.C  -std=c++17 (test for excess
>> > errors)
>> > FAIL: g++.dg/analyzer/pr94028.C  -std=c++2a (test for excess
>> > errors)
>>
>> Updated to add PR 96014 to the commit log.
>>
>> OK for master?
>
>Sorry for not responding to this earlier.
>
>My knowledge of C++ exceptions is a little rusty; I found the addition
>of "throw()" to mark the decl as non-throwing to be confusing.

An operator new is required to report allocation failure by throwing
an exception that can be caught by `catch (const std::bad_alloc&)`,
unless it is marked as non-throwing, in which case it reports
allocation failure by returning a null pointer.

I believe the C++ front end adds the returns_nonnull attribute to
operator new unless it is marked non-throwing.

The operator new in this test just calls calloc, which can return
null, but it isn't marked as non-throwing so has the returns_nonnull
attribute. Therefore it has undefined behaviour if calloc ever fails
and returns null. Your analyzer seems to be noticing this, and so
warning, which is nice.

The way to fix it is to either check the return value of calloc and
throw std::bad_alloc() if calloc returned null, or to simply mark the
operator new as non-throwing so that returning null is OK.


>Looking in my copy of Stroustrup 4th edition (C++11) p367 it says this
>is an empty exception specification, and is equivalent to "noexcept",
>and Stroustrup recommends using the latter instead.  Did you use this
>syntax for backwards compat with C++98, or is "noexcept" available in
>the earlier C++ dialects?

noexcept is not valid in C++98/C++03. But I forgot that throw() is
deprecated in C++17 and removed in C++20, so although G++ still
accepts throw() to be futureproof we should use:

#if __cplusplus < 201103L
# define NOTHROW throw()
#else
# define NOTHROW noexcept
#endif

and then mark it NOTHROW.

Although it would take fewer lines of code to just check what calloc
returns and turn a null pointer into a std::bad_alloc exception.