On 10/05/2017 05:07 PM, Jason Merrill wrote:
On Thu, Oct 5, 2017 at 6:31 AM, Martin Liška <mli...@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?
FAIL: g++.dg/torture/pr34850.C -O1 (test for warnings, line 15)
FAIL: g++.dg/torture/pr34850.C -O2 (test for warnings, line 15)
FAIL: g++.dg/torture/pr34850.C -O2 -flto -fno-use-linker-plugin
-flto-partition=none (test for warnings, line 15)
FAIL: g++.dg/torture/pr34850.C -O2 -flto -fuse-linker-plugin
-fno-fat-lto-objects (test for warnings, line 15)
FAIL: g++.dg/torture/pr34850.C -O3 -g (test for warnings, line 15)
FAIL: g++.dg/torture/pr34850.C -Os (test for warnings, line 15)
2) this test is somehow hard to fix as it requires some unreachability.
Can't we add a return statement to memset?
Does not help :)
Also, this testcase seems to indicate a danger from this patch, like
we've seen before with bounds checking: if we have a checking path
that complains about invalid arguments and then has undefined
behavior, we optimize away the diagnostic. Can we warn in such cases?
Good question, can you please provide more info how that happens? Do we have an
example for that?
We probably want to provide a way to turn off this behavior.
If we're going to enable this by default, we probably also want
-Wreturn-type on by default.
Agree.
Thanks,
Martin
Jason
