From 5b800c231f45fcd823a3e958bb942cd620ceb3e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Steinbrink?= Date: Wed, 3 Jan 2018 10:29:27 +0100 Subject: [PATCH 1/4] Don't force-enable frame pointers when generating debug info We apparently used to generate bad/incomplete debug info causing debuggers not to find symbols of stack allocated variables. This was somehow worked around by having frame pointers. With the current codegen, this seems no longer necessary, so we can remove the code that force-enables frame pointers whenever debug info is requested. Since certain situations, like profiling code profit from having frame pointers, we add a -Cforce-frame-pointers flag to always enable frame pointers. Fixes #11906 --- src/librustc/session/config.rs | 6 ++++++ src/librustc/session/mod.rs | 2 +- src/librustc_trans/attributes.rs | 2 -- src/test/codegen/force-frame-pointers.rs | 16 ++++++++++++++++ 4 files changed, 23 insertions(+), 3 deletions(-) create mode 100644 src/test/codegen/force-frame-pointers.rs diff --git a/src/librustc/session/config.rs b/src/librustc/session/config.rs index 06922d986b3ce..023be78922207 100644 --- a/src/librustc/session/config.rs +++ b/src/librustc/session/config.rs @@ -1053,6 +1053,8 @@ options! {CodegenOptions, CodegenSetter, basic_codegen_options, 2 = full debug info with variable and type information"), opt_level: Option = (None, parse_opt_string, [TRACKED], "optimize with possible levels 0-3, s, or z"), + force_frame_pointers: bool = (false, parse_bool, [TRACKED], + "force frame pointers to be used"), debug_assertions: Option = (None, parse_opt_bool, [TRACKED], "explicitly enable the cfg(debug_assertions) directive"), inline_threshold: Option = (None, parse_opt_uint, [TRACKED], @@ -2965,6 +2967,10 @@ mod tests { opts.cg.debuginfo = Some(0xba5eba11); assert!(reference.dep_tracking_hash() != opts.dep_tracking_hash()); + opts = reference.clone(); + opts.cg.force_frame_pointers = true; + assert!(reference.dep_tracking_hash() != opts.dep_tracking_hash()); + opts = reference.clone(); opts.cg.debug_assertions = Some(true); assert!(reference.dep_tracking_hash() != opts.dep_tracking_hash()); diff --git a/src/librustc/session/mod.rs b/src/librustc/session/mod.rs index 37a6b2e79f7db..45b7e2d17404a 100644 --- a/src/librustc/session/mod.rs +++ b/src/librustc/session/mod.rs @@ -20,7 +20,7 @@ use lint::builtin::BuiltinLintDiagnostics; use middle::allocator::AllocatorKind; use middle::dependency_format; use session::search_paths::PathKind; -use session::config::{DebugInfoLevel, OutputType}; +use session::config::{OutputType}; use ty::tls; use util::nodemap::{FxHashSet}; use util::common::{duration_to_secs_str, ErrorReported}; diff --git a/src/librustc_trans/attributes.rs b/src/librustc_trans/attributes.rs index f455c19cc0bbe..5baed57092d0f 100644 --- a/src/librustc_trans/attributes.rs +++ b/src/librustc_trans/attributes.rs @@ -69,8 +69,6 @@ pub fn naked(val: ValueRef, is_naked: bool) { } pub fn set_frame_pointer_elimination(cx: &CodegenCx, llfn: ValueRef) { - // FIXME: #11906: Omitting frame pointers breaks retrieving the value of a - // parameter. if cx.sess().must_not_eliminate_frame_pointers() { llvm::AddFunctionAttrStringValue( llfn, llvm::AttributePlace::Function, diff --git a/src/test/codegen/force-frame-pointers.rs b/src/test/codegen/force-frame-pointers.rs new file mode 100644 index 0000000000000..d40406a0476f2 --- /dev/null +++ b/src/test/codegen/force-frame-pointers.rs @@ -0,0 +1,16 @@ +// Copyright 2018 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. +// +// compile-flags: -C no-prepopulate-passes -C force-frame-pointers + +#![crate_type="lib"] + +// CHECK: attributes #{{.*}} "no-frame-pointer-elim"="true" +pub fn foo() {} From 09d2db4e963c6696f7c22386b6791e419ad75cfb Mon Sep 17 00:00:00 2001 From: Simonas Kazlauskas Date: Tue, 6 Mar 2018 20:27:19 +0200 Subject: [PATCH 2/4] Rework force-frame-pointer This reworks the force-frame-pointer PR to explicitly only consider the value of the flag if it is provided, and use a target default otherwise. Something that was tried but not kept was renaming the flag to `frame-pointer`, because for flag `frame-pointer=no`, there is no guarante, that LLVM will elide *all* the frame pointers; oposite of what the literal reading of the flag would suggest. --- src/librustc/session/config.rs | 6 +++--- src/librustc/session/mod.rs | 7 +++++-- src/test/codegen/force-frame-pointers.rs | 2 +- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/librustc/session/config.rs b/src/librustc/session/config.rs index 023be78922207..59b40e9e2dc56 100644 --- a/src/librustc/session/config.rs +++ b/src/librustc/session/config.rs @@ -1053,8 +1053,8 @@ options! {CodegenOptions, CodegenSetter, basic_codegen_options, 2 = full debug info with variable and type information"), opt_level: Option = (None, parse_opt_string, [TRACKED], "optimize with possible levels 0-3, s, or z"), - force_frame_pointers: bool = (false, parse_bool, [TRACKED], - "force frame pointers to be used"), + force_frame_pointers: Option = (None, parse_opt_bool, [TRACKED], + "force use of the frame pointers"), debug_assertions: Option = (None, parse_opt_bool, [TRACKED], "explicitly enable the cfg(debug_assertions) directive"), inline_threshold: Option = (None, parse_opt_uint, [TRACKED], @@ -2968,7 +2968,7 @@ mod tests { assert!(reference.dep_tracking_hash() != opts.dep_tracking_hash()); opts = reference.clone(); - opts.cg.force_frame_pointers = true; + opts.cg.force_frame_pointers = Some(false); assert!(reference.dep_tracking_hash() != opts.dep_tracking_hash()); opts = reference.clone(); diff --git a/src/librustc/session/mod.rs b/src/librustc/session/mod.rs index 45b7e2d17404a..2ab72ba20bf4f 100644 --- a/src/librustc/session/mod.rs +++ b/src/librustc/session/mod.rs @@ -658,8 +658,11 @@ impl Session { } pub fn must_not_eliminate_frame_pointers(&self) -> bool { - self.opts.debuginfo != DebugInfoLevel::NoDebugInfo - || !self.target.target.options.eliminate_frame_pointer + if let Some(x) = self.opts.cg.force_frame_pointers { + x + } else { + !self.target.target.options.eliminate_frame_pointer + } } /// Returns the symbol name for the registrar function, diff --git a/src/test/codegen/force-frame-pointers.rs b/src/test/codegen/force-frame-pointers.rs index d40406a0476f2..f70e366719882 100644 --- a/src/test/codegen/force-frame-pointers.rs +++ b/src/test/codegen/force-frame-pointers.rs @@ -8,7 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. // -// compile-flags: -C no-prepopulate-passes -C force-frame-pointers +// compile-flags: -C no-prepopulate-passes -C force-frame-pointers=y #![crate_type="lib"] From 969449f236bf8fe47b5bc90d4f02a3db00a80643 Mon Sep 17 00:00:00 2001 From: Simonas Kazlauskas Date: Sat, 24 Mar 2018 21:54:21 +0200 Subject: [PATCH 3/4] =?UTF-8?q?Don=E2=80=99t=20eliminate=20frame=20pointer?= =?UTF-8?q?s=20on=20apple=20by=20default?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/librustc_target/spec/apple_ios_base.rs | 1 + src/librustc_target/spec/i686_apple_darwin.rs | 1 + 2 files changed, 2 insertions(+) diff --git a/src/librustc_target/spec/apple_ios_base.rs b/src/librustc_target/spec/apple_ios_base.rs index acbbab313fe59..46bb01e7c420d 100644 --- a/src/librustc_target/spec/apple_ios_base.rs +++ b/src/librustc_target/spec/apple_ios_base.rs @@ -98,6 +98,7 @@ pub fn opts(arch: Arch) -> Result { executables: true, pre_link_args, has_elf_tls: false, + eliminate_frame_pointer: false, // The following line is a workaround for jemalloc 4.5 being broken on // ios. jemalloc 5.0 is supposed to fix this. // see https://github.com/rust-lang/rust/issues/45262 diff --git a/src/librustc_target/spec/i686_apple_darwin.rs b/src/librustc_target/spec/i686_apple_darwin.rs index 06ea1e4649b42..d17789dfcc07f 100644 --- a/src/librustc_target/spec/i686_apple_darwin.rs +++ b/src/librustc_target/spec/i686_apple_darwin.rs @@ -16,6 +16,7 @@ pub fn target() -> TargetResult { base.max_atomic_width = Some(64); base.pre_link_args.insert(LinkerFlavor::Gcc, vec!["-m32".to_string()]); base.stack_probes = true; + base.eliminate_frame_pointer = false; Ok(Target { llvm_target: "i686-apple-darwin".to_string(), From 7ec045219040876730693543e6cca53120a3236b Mon Sep 17 00:00:00 2001 From: Simonas Kazlauskas Date: Wed, 18 Apr 2018 20:48:34 +0300 Subject: [PATCH 4/4] Force frame pointers for the backtrace test --- src/test/run-pass/backtrace-debuginfo.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/run-pass/backtrace-debuginfo.rs b/src/test/run-pass/backtrace-debuginfo.rs index 2b82a8943636e..7bcb4e5ec2d2e 100644 --- a/src/test/run-pass/backtrace-debuginfo.rs +++ b/src/test/run-pass/backtrace-debuginfo.rs @@ -16,6 +16,7 @@ // "enable" to 0 instead. // compile-flags:-g -Cllvm-args=-enable-tail-merge=0 -Cllvm-args=-opt-bisect-limit=0 +// compile-flags:-Cforce-frame-pointers=yes // ignore-pretty issue #37195 // ignore-cloudabi spawning processes is not supported // ignore-emscripten spawning processes is not supported