-
Notifications
You must be signed in to change notification settings - Fork 38.7k
build: disable -stack-clash-protection on Windows #19318
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This does not currently work properly, and will cause GCC to terminate while compiling bitcoind. Related upstream issue: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90458
|
|
||
| dnl stack-clash-protection does not work on Windows | ||
| dnl https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90458 | ||
| AX_CHECK_COMPILE_FLAG([-fno-stack-clash-protection],[HARDENED_CXXFLAGS="$HARDENED_CXXFLAGS -fno-stack-clash-protection"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't it be preferable in this case to not add it in the first place, instead of append -fno-X here? Or is it enabled by some umbrella option?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's likely we'll end up enabling this, and other flags (for other platforms) in #18921. Progress there has been slow, so I wanted to split this out and just disable it now. It doesn't work, and is causing build failures when testing 18921 and related PRs. I haven't seen it being turned on by an umbrella type option/compiler default yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What I mean is making the AC_CHECK_COMPILE_FLAG(…) to enable it conditional on non-windows.
Otherwise you get a gcc overall command line like
fstack-clash-protection … … fno-stack-clash-protection
which probably workd but is confusing to see.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with @wumpus. It sounds like this combo would fail to compile even the most trivial c++ program, so I can't imagine anyone shipping a mingw compiler with this on by default. So proactively turning it of seems like overkill.
Instead, how about for #18921, we add a your minimal test-case as the last (input) param to AX_CHECK_COMPILE_FLAG. That way we don't have to special-case it, and it may just work on mingw at some point in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've merged this into #18921.
|
Wrong @wumpus. |
This does not currently work properly, and will cause GCC to terminate while compiling for Windows. This affects
x86_64-w64-mingw32-g++8 through 10. Related upstream issue: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90458.I've PR'd this separate to #18921, as this doesn't have to wait for us to decide if we want to opt-in to using those flags on other platforms, and is causing build failures.
While doing more testing for #18921, I (and the bot) saw build issues that would terminate GCC:
CXX bitcoind-bitcoind.o during RTL pass: final In file included from ./logging.h:10, from ./util/system.h:21, from ./init.h:11, from bitcoind.cpp:13: ./tinyformat.h: In static member function 'static int tinyformat::detail::FormatArg::toIntImpl(const void*) [with T = std::__cxx11::basic_string<char>]': ./tinyformat.h:550:9: internal compiler error: in seh_emit_stackalloc, at config/i386/winnt.c:1043 } ^ 0x7fa20389609a __libc_start_main ../csu/libc-start.c:308 Please submit a full bug report, with preprocessed source if appropriate. Please include the complete backtrace with any bug report. See <https://gcc.gnu.org/bugs/> for instructions. {standard input}: Assembler messages: {standard input}: Error: open SEH entry at end of file (missing .seh_endproc)Here's a fairly minimal test case, that will cause the issue using
-fstack-clash-protection -O1: