LTO allows the compiler to prove variable values that non-LTO mode
cannot prove. The workaround for the gcc-7 __builtin_constant_p
quirk incorrectly caused LTO on older gcc to report __builtin_constant_p
as unsupported. Fix this by adding a stub asm() statement to hide the
value of the input parameter from the compiler.
Fixes: 1ed7cec714 ("Tighten __builtin_constant_p check to handle gcc-7")
Early implementations of integer_sequence used a naive implementation
that required one level of template depth per additional integer in the
sequence. Rebirth uses a private alternate implementation named
make_tree_index_sequence that requires only log(N) steps for an
N-element index_sequence. Recent versions of gcc ship a log(N) version
of integer_sequence. Probe for that version and, if found, use it
instead of the private implementation, on the theory that the compiler
writers did at least as good a job as I did, and possibly better if they
were able to leverage compiler implementation details.
- Add wrappers for PHYSFS_read and its convenience functions. Poison
the memory before calling PHYSFS, so that any uninitialized bytes
(likely caused by a short read) are reported if the caller tries to
access the value anyway.
- Add SConstruct options to enable wrapping, so that users do not need
to enumerate the wrapped functions manually.
- Fix link failure when using LTO+wrappers.
It is not meant to be included in every file. Starting in gcc-7, static
functions declared in valptridx.tcc generate -Wunused-function warnings
if the including file does not instantiate any templates that use the
static function. Avoid these warnings by including the tcc only in a
file that is guaranteed to instantiate templates that use these
functions.
gcc-7 warning -Wformat-truncation would be good to fix, but the compiler
does not offer an acceptable solution yet.
gcc-7 warning -Wimplicit-fallthrough requires more analysis of the
diagnosed sites to identify which, if any, require a resolution other
than suppressing the warning.
gcc-6 issues a "used but never defined" warning for functions which are
declared, never defined, and used only in contexts which are later
deleted by the optimizer. If the function were actually used, the link
would fail. Switch to a complicated mechanism to define the function
while keeping it out of line so that __attribute__((__error__)) is
triggered as intended.
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>
When Rebirth is configured to use OpenGL, it cannot start with an SDL
that lacks OpenGL support. Detect this mismatch at build time. Require
the user to resolve the conflict by disabling Rebirth OpenGL or by
enabling SDL OpenGL.
The Windows implementation of inet_ntop incorrectly omits the `const`
qualifier on the input address. This broke the Windows build, since
Rebirth passes a const-qualified input address, as permitted by POSIX.
Some Windows cross-compilers, such as mingw32, lack a definition of
inet_ntop entirely. For such environments, fall back to inet_ntoa and
disallow building with IPv6.
Rework the formatting and add a Windows-specific test for whether
inet_ntop is available. As inet_ntop is specified by POSIX, assume all
modern non-Windows platforms support it.
SConstruct prefers to refer to linker flags for the main executable as
LINKFLAGS, not LDFLAGS. Rename the internal storage to eliminate a
special case in accessing it. Continue to use the environment variable
$LDFLAGS as an initial value, since many tools expect to pass linker
flags through $LDFLAGS.
zicodxx reported a compiler error when using inheriting constructors
when using an unspecified environment. Investigation shows that gcc-6.x
has a regression that causes spurious -Wuseless-cast warnings where the
compiler automatically synthesizes a cast, then warns the user that the
synthesized cast is useless.
Compensate for this by extending the -Wuseless-cast testcase to provoke
this bug, if present. Affected users will lose -Wuseless-cast, but
their build will succeed.
Reported-by: zicodxx <https://github.com/dxx-rebirth/dxx-rebirth/issues/316>
Popular Windows tools for stack unwinding lack support for the DWARF
debug format. Unwinding the stack without DWARF is unreliable without
frame pointers. Default frame pointers to enabled for Windows so that
these tools work despite their lack of DWARF support.
These flags are passed through to the underlying build, but not checked
by the SConf tests. They are meant for options which will work in the
main build, but fail in the configure environment, such as `-Wl,--wrap`.
Move DXX_VERSION_SEQ from DXXProgram to DXXCommon to make it visible to
DXXArchive.configure_environment(). If pch is enabled, define
DXX_VERSION_SEQ in dxxsconf.h and remove it from the command line. This
fixes a build failure where gcc ignored ownpch.cpp.gch because it was
built without DXX_VERSION_SEQ, but some files were built with
DXX_VERSION_SEQ.
To recognize getaddrinfo as supported, require that the AI_* flags
passed to it be usable:
- AI_NUMERICSERV must be defined as a number or be undefined
- If ipv6=1, then AI_V4MAPPED and AI_ALL must be defined as numbers
When getaddrinfo is used, raise _WIN32_WINNT from 0x501 to 0x600.
Otherwise, lower it to 0x500.
derhass reports that SDL on Raspbian provokes a warning from
-Wuseless-cast. Add a test that will report -Wuseless-cast as unusable
when SDL causes this warning. Developers on Raspbian will not get the
benefit of -Wuseless-cast, but their build will succeed.
Reported-by: derhass <https://github.com/dxx-rebirth/dxx-rebirth/issues/257>
Extended identifiers are convenient when supported, but derhass reports
that the latest stable gcc on Raspbian stable fails the test for
mandatory extended identifier support. Replace the Cxx11RequiredFeature
test for extended identifiers with an SConf test to selectively enable
-fextended-identifiers.
Reported-by: derhass <https://github.com/dxx-rebirth/dxx-rebirth/issues/254>
Fixes: a8a41aa2ff ("Move git version into Python class")
Previously, raspberrypi mode forced DXX_WORDS_NEED_ALIGNMENT on and any
other mode forced it off. Add a specific user setting for
DXX_WORDS_NEED_ALIGNMENT. Set its default as bool(raspberrypi) to
preserve the previous special case.
Rename symbol WORDS_NEED_ALIGNMENT to DXX_WORDS_NEED_ALIGNMENT to show that it is a DXX
symbol, not one inherited from a library.
This is a mostly automated transform, but the changes to SConstruct were
manual.
git grep -wl WORDS_NEED_ALIGNMENT -- '*.h' '*.cpp' | xargs sed -i -e 's/^\s*#ifdef \(WORDS_NEED_ALIGNMENT\)\>/#if DXX_\1/' -e 's/\s*#\(el\)\?if \(.*\)defined(\(WORDS_NEED_ALIGNMENT\))/#\1if \2DXX_\3/' -e 's/^\s*#ifndef \(WORDS_NEED_ALIGNMENT\)\>/#if !DXX_\1/'