-
Notifications
You must be signed in to change notification settings - Fork 14.5k
[BOLT] Force frame pointers off for runtimes #148009
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
[BOLT] Force frame pointers off for runtimes #148009
Conversation
@llvm/pr-subscribers-bolt Author: Peter Waller (peterwaller-arm) ChangesDistributions are making the choice to turn frame pointers on by default. Nixpkgs recently turned them on, and the method they use to do so implies that everything is built with them on by default. Assuming that a well behaved distribution doing this puts See also: #147569 Full diff: https://github.com/llvm/llvm-project/pull/148009.diff 1 Files Affected:
diff --git a/bolt/runtime/CMakeLists.txt b/bolt/runtime/CMakeLists.txt
index 87cc44812da11..4c496c2b800ef 100644
--- a/bolt/runtime/CMakeLists.txt
+++ b/bolt/runtime/CMakeLists.txt
@@ -35,7 +35,10 @@ set(BOLT_RT_FLAGS
-fno-exceptions
-fno-rtti
-fno-stack-protector
- -fPIC)
+ -fPIC
+ -mgeneral-regs-only
+ -fomit-frame-pointer # Runtime currently assumes omitted frame pointers. llvm/llvm-project#147569
+)
if (CMAKE_SYSTEM_PROCESSOR MATCHES "x86_64")
set(BOLT_RT_FLAGS ${BOLT_RT_FLAGS}
-mno-sse
|
llvm-project/bolt/runtime/instr.cpp Line 1666 in dbb79c3
I currently suspect the issue is arising from
Mentioning it in case anyone has an idea what's happening there, it looks like it relates to |
@peterwaller-arm It looks like GCC just doesn't support naked functions on AArch64 at all, see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77882. |
27767c4
to
8d5cb4d
Compare
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.
Looks reasonable to me, but a bolt maintainer should approve as well.
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.
LGTM. Thanks!
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.
Thanks Peter!
Distributions are making the choice to turn frame pointers on by default. Nixpkgs recently turned them on, and the method they use to do so implies that everything is built with them on by default. NixOS/nixpkgs#399014 Assuming that a well behaved distribution doing this puts '-fno-omit-frame-pointer' at the beginning of the compiler invocation, we can still re-enable it by supplying -fomit-frame-pointer thereafter. See also: llvm#147569 Fixes: llvm#148595
8d5cb4d
to
01424e2
Compare
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/153/builds/37967 Here is the relevant piece of the build log for the reference
|
Distributions are making the choice to turn frame pointers on by default. Nixpkgs recently turned them on, and the method they use to do so implies that everything is built with them on by default.
NixOS/nixpkgs#399014
Assuming that a well behaved distribution doing this puts
-fno-omit-frame-pointer
at the beginning of the compiler invocation, we can still re-enable omission by supplying-fomit-frame-pointer
during compilation.This fixes some segfaults from stack corruption in binaries rewritten by bolt with
llvm-bolt -instrument
.See also: #147569
Fixes: #148595