Re: [PATCH][RFC] Instrument function exit with __builtin_unreachable in C++.

[Jason Merrill <jason at redhat dot com>]

On Thu, Oct 5, 2017 at 12:53 PM, Martin Liška <mliska@suse.cz> wrote:
> On 10/05/2017 05:07 PM, Jason Merrill wrote:
>> On Thu, Oct 5, 2017 at 6:31 AM, Martin Liška <mliska@suse.cz> wrote:
>>> As discussed 2 days ago on IRC with Jakub and Jonathan, C++ standard says
>>> that having a non-return
>>> function with missing return statement is undefined behavior. We've got
>>> -fsanitize=return check for
>>> that and we can in such case instrument __builtin_unreachable(). This
>>> patch does that.
>>
>>
>> Great.
>>
>>> And there's still some fallout:
>>>
>>> FAIL: g++.dg/cpp0x/constexpr-diag3.C  -std=c++11  (test for errors, line
>>> 7)
>>> FAIL: g++.dg/cpp0x/constexpr-neg3.C  -std=c++11  (test for errors, line
>>> 12)
>>> FAIL: g++.dg/cpp1y/constexpr-return2.C  -std=c++14  (test for errors,
>>> line 7)
>>> FAIL: g++.dg/cpp1y/constexpr-return2.C  -std=c++14 (test for excess
>>> errors)
>>> FAIL: g++.dg/cpp1y/pr63996.C  -std=c++14  (test for errors, line 9)
>>> FAIL: g++.dg/cpp1y/pr63996.C  -std=c++14 (test for excess errors)
>>>
>>> 1) there are causing:
>>>
>>> ./xg++ -B.
>>> /home/marxin/Programming/gcc/gcc/testsuite/g++.dg/cpp1y/pr63996.C
>>> /home/marxin/Programming/gcc/gcc/testsuite/g++.dg/cpp1y/pr63996.C:9:23:
>>> in constexpr expansion of ‘foo(1)’
>>> /home/marxin/Programming/gcc/gcc/testsuite/g++.dg/cpp1y/pr63996.C:4:1:
>>> error: ‘__builtin_unreachable()’ is not a constant expression
>>>   foo (int i)
>>>   ^~~
>>>
>>> Where we probably should not emit the built-in call. Am I right?
>>
>>
>> Or constexpr.c could give a more friendly diagnostic for
>> __builtin_unreachable.  It's correct to give a diagnostic here for
>> this testcase.
>
>
> Hi.
>
> That's good idea, any suggestion different from:
>
> ./xg++ -B.
> /home/marxin/Programming/gcc2/gcc/testsuite/g++.dg/cpp1y/pr63996.C
> /home/marxin/Programming/gcc2/gcc/testsuite/g++.dg/cpp1y/pr63996.C:9:23:
> in constexpr expansion of ‘foo(1)’
> <built-in>: error: constexpr can't contain call of a non-return function
> ‘__builtin_unreachable’
>
> which is probably misleading as it points to a function call that is
> actually missing in source code.
> Should we distinguish between implicit and explicit __builtin_unreachable?

Probably without your change the constexpr code already diagnoses the
missing return as "constexpr call flows off the end of the function";
that same message seems appropriate.

> So turning on the warning by default for c++, we get about 500 failing test-cases. Uf :/

Yes, we've been sloppy about this in the testsuite.  :(

Jason