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

[Jonathan Wakely]

On 06/07/20 11:27 +0100, Jonathan Wakely wrote:
>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.

It looks like I accidentally pushed this patch without approval (when
pushing my own a1a0dc4548979f8a340a7ea71624a52a20e1e0b3 change).

As discussed above, the throw() is not actually valid in C++20. Should
I push this patch to use noexcept instead for C++11 and up?


Tested x86_64-linux:

PASS: g++.dg/analyzer/pr94028.C  -std=c++14  (test for warnings, line 22)
PASS: g++.dg/analyzer/pr94028.C  -std=c++14  (test for warnings, line 26)
PASS: g++.dg/analyzer/pr94028.C  -std=c++14  (test for warnings, line 33)
PASS: g++.dg/analyzer/pr94028.C  -std=c++14 (test for excess errors)
PASS: g++.dg/analyzer/pr94028.C  -std=c++17  (test for warnings, line 22)
PASS: g++.dg/analyzer/pr94028.C  -std=c++17  (test for warnings, line 26)
PASS: g++.dg/analyzer/pr94028.C  -std=c++17  (test for warnings, line 33)
PASS: g++.dg/analyzer/pr94028.C  -std=c++17 (test for excess errors)
PASS: g++.dg/analyzer/pr94028.C  -std=c++2a  (test for warnings, line 22)
PASS: g++.dg/analyzer/pr94028.C  -std=c++2a  (test for warnings, line 26)
PASS: g++.dg/analyzer/pr94028.C  -std=c++2a  (test for warnings, line 33)
PASS: g++.dg/analyzer/pr94028.C  -std=c++2a (test for excess errors)
PASS: g++.dg/analyzer/pr94028.C  -std=c++98  (test for warnings, line 22)
PASS: g++.dg/analyzer/pr94028.C  -std=c++98  (test for warnings, line 26)
PASS: g++.dg/analyzer/pr94028.C  -std=c++98  (test for warnings, line 33)
PASS: g++.dg/analyzer/pr94028.C  -std=c++98 (test for excess errors)



-------------- next part --------------
A non-text attachment was scrubbed...
Name: patch.txt
Type: text/x-patch
Size: 916 bytes
Desc: not available
URL: <https://gcc.gnu.org/pipermail/gcc-patches/attachments/20200716/2f60e77e/attachment.bin>