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. > 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? 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? 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. Jason