From 1ed7cec714c623758e3418ec69eaf3b3ff03e9f6 Mon Sep 17 00:00:00 2001
From: Kp <kp@valhallalegends.com>
Date: Sat, 3 Jun 2017 17:11:12 +0000
Subject: [PATCH] Tighten __builtin_constant_p check to handle gcc-7

Compile time sanity checks are built on gcc's __builtin_constant_p.
This intrinsic is useful for optimization and for some simple forms of
compile-time checks (such as the infamous open(2) missing mode), but
upstream does not guarantee that it will work reliably for more
complicated checks.  Starting in gcc-7.1, __builtin_constant_p returns
an incorrect result of true for the vm_vec_sub expression

    `(a.x == b.x && a.y == b.y && a.z == b.z)`

even when the blamed sites clearly cannot prove that the inputs are
equal.  The useful result would be to return true if, and only if, the
inputs were provably identical; inputs which might be identical at
runtime, or might not, would return false.  Based on a bug filed with
gcc and the developer comments there, it appears many projects have
assumed this intrinsic is usable in this way, but the gcc developers do
not guarantee that it can be used this way.  Additionally, they believe
affected projects are rare and were wrong to use this intrinsic, so they
have no plans to fix this regression.

For more details, see gcc bug #72785
<https://gcc.gnu.org/bugzilla/show_bug.cgi?id=72785>.  For some more
pointed commentary on this change in gcc, see Linus' kernel commit
474c90156c8dcc2fa815e6716cc9394d7930cb9c
<https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=474c90156c8dcc2fa815e6716cc9394d7930cb9c>.

Update the SConf test to include the reproducer shown by Markus
Trippelsdorf in gcc bug #72785, comment #0.  This reproducer compiles
cleanly on <gcc-7, causing no change in semantics on older compilers.
Affected versions of gcc-7 will miscompile this reproducer into a link
error, causing SConf to record that the compiler does not optimize
__builtin_constant_p.  This is pessimistic, since even affected versions
of gcc-7 can handle some simple uses of __builtin_constant_p correctly.

This is a quick fix to get gcc-7 users working.  Upstream seems
disinclined to revert to the more useful semantics of <gcc-7 or to
introduce an alternative intrinsic with more helpful semantics.  As a
minor enhancement for Rebirth, it would be nice to probe the limits of
gcc-7's handling of __builtin_constant_p so that cases it handles
correctly could be enabled for gcc-7 users, while still blacklisting the
more complicated checks that gcc-7 miscompiles.

Thanks to Markus Trippelsdorf for providing a minimal reproducer to
detect the affected gcc versions.

Reported-by: parkerlreed <https://github.com/dxx-rebirth/dxx-rebirth/issues/337>
---
 SConstruct | 28 +++++++++++++++++++++++-----
 1 file changed, 23 insertions(+), 5 deletions(-)

diff --git a/SConstruct b/SConstruct
index fbd04a7ed..ca151b896 100644
--- a/SConstruct
+++ b/SConstruct
@@ -1358,14 +1358,32 @@ is not a bug in the test or in the compiler.
 help:assume compiler supports compile-time __builtin_constant_p
 """
 		f = '''
+/*
+ * Begin `timekeeping.i` from gcc bug #72785, comment #0.  This block
+ * provokes gcc-7 into generating an impossible code path, which then
+ * fails to link when ____ilog2_NaN is intentionally undefined.
+ */
+int a, b;
+int ____ilog2_NaN();
+static void by(void) {
+	int c = 1;
+	b = a ?: c;
+	__builtin_constant_p(b) ? b ? ____ilog2_NaN() : 0 : 0;
+}
+/*
+ * End `timekeeping.i`.
+ */
 int c(int);
-static int a(int b){
-	return __builtin_constant_p(b) ? 1 : %s;
+static int x(int y){
+	return __builtin_constant_p(y) ? 1 : %s;
 }
 '''
-		main = 'return a(1) + a(2)'
+		main = '''
+	by();
+	return x(1) + x(2);
+'''
 		Define = context.sconf.Define
-		if self.Link(context, text=f % 'c(b)', main=main, msg='whether compiler optimizes __builtin_constant_p', calling_function='optimize_builtin_constant_p'):
+		if self.Link(context, text=f % 'c(y)', main=main, msg='whether compiler optimizes __builtin_constant_p', calling_function='optimize_builtin_constant_p'):
 			Define('DXX_HAVE_BUILTIN_CONSTANT_P')
 			Define('DXX_CONSTANT_TRUE(E)', '(__builtin_constant_p((E)) && (E))')
 			dxx_builtin_constant_p = '__builtin_constant_p(A)'
@@ -3621,7 +3639,7 @@ class DXXCommon(LazyObjectConstructor):
 	def platform_settings(self):
 		# windows or *nix?
 		platform_name = self.user_settings.host_platform
-		message(self, "compiling on %s for %s into %s%s" % (sys.platform, platform_name, self.user_settings.builddir or '.',
+		message(self, "compiling on %r/%r for %r into %s%s" % (sys.platform, os.uname()[4], platform_name, self.user_settings.builddir or '.',
 			(' with prefix list %s' % str(self._argument_prefix_list)) if self._argument_prefix_list else ''))
 		return (
 			self.Win32PlatformSettings if platform_name == 'win32' else (