Skip to content
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

Suppress debuginfo on naked function arguments #74105

Merged
merged 1 commit into from
Jul 30, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion src/librustc_mir_build/build/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use rustc_hir::lang_items;
use rustc_hir::{GeneratorKind, HirIdMap, Node};
use rustc_index::vec::{Idx, IndexVec};
use rustc_infer::infer::TyCtxtInferExt;
use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags;
use rustc_middle::middle::region;
use rustc_middle::mir::*;
use rustc_middle::ty::subst::Subst;
Expand Down Expand Up @@ -797,12 +798,22 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
argument_scope: region::Scope,
ast_body: &'tcx hir::Expr<'tcx>,
) -> BlockAnd<()> {
let tcx = self.hir.tcx();
let attrs = tcx.codegen_fn_attrs(fn_def_id);
let naked = attrs.flags.contains(CodegenFnAttrFlags::NAKED);

// Allocate locals for the function arguments
for &ArgInfo(ty, _, arg_opt, _) in arguments.iter() {
let source_info =
SourceInfo::outermost(arg_opt.map_or(self.fn_span, |arg| arg.pat.span));
let arg_local = self.local_decls.push(LocalDecl::with_source_info(ty, source_info));

// Emit function argument debuginfo only for non-naked functions.
// See: https://github.com/rust-lang/rust/issues/42779
if naked {
continue;
}
Comment on lines +811 to +815
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is a good idea, wouldn't be safer to just disable all debuginfo generation for #[naked] functions in codegen?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also this PR doesn't add a mir-opt test showing the lack of MIR debuginfo (but which we might not want anyway).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eddyb We don't want to suppress the debuginfo describing the calling contract of the function, just the internal function operations. The lldb can reference this information to show the contents of (for example) the first argument in %rdi. But if you think we need to implement this another way, I'm not particular.


// If this is a simple binding pattern, give debuginfo a nice name.
if let Some(arg) = arg_opt {
if let Some(ident) = arg.pat.simple_ident() {
Expand All @@ -815,7 +826,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
}
}

let tcx = self.hir.tcx();
let tcx_hir = tcx.hir();
let hir_typeck_results = self.hir.typeck_results();

Expand Down
4 changes: 2 additions & 2 deletions src/test/codegen/naked-functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ pub fn naked_empty() {
// CHECK-NEXT: define void @naked_with_args(i{{[0-9]+( %0)?}})
pub fn naked_with_args(a: isize) {
// CHECK-NEXT: {{.+}}:
// CHECK-NEXT: %a = alloca i{{[0-9]+}}
// CHECK-NEXT: %_1 = alloca i{{[0-9]+}}
&a; // keep variable in an alloca
// CHECK: ret void
}
Expand All @@ -39,7 +39,7 @@ pub fn naked_with_return() -> isize {
#[naked]
pub fn naked_with_args_and_return(a: isize) -> isize {
// CHECK-NEXT: {{.+}}:
// CHECK-NEXT: %a = alloca i{{[0-9]+}}
// CHECK-NEXT: %_1 = alloca i{{[0-9]+}}
&a; // keep variable in an alloca
// CHECK: ret i{{[0-9]+}} %{{[0-9]+}}
a
Expand Down
40 changes: 40 additions & 0 deletions src/test/debuginfo/function-arguments-naked.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// min-lldb-version: 310

// We have to ignore android because of this issue:
// https://github.com/rust-lang/rust/issues/74847
// ignore-android

// compile-flags:-g

// === GDB TESTS ===================================================================================

// gdb-command:run

// gdb-command:info args
// gdb-check:No arguments.
// gdb-command:continue

// === LLDB TESTS ==================================================================================

// lldb-command:run

// lldb-command:frame variable
// lldbg-check:(unsigned long) = 111 (unsigned long) = 222
// lldbr-check:(unsigned long) = 111 (unsigned long) = 222
// lldb-command:continue


#![feature(naked_functions)]
#![feature(omit_gdb_pretty_printer_section)]
#![omit_gdb_pretty_printer_section]

fn main() {
naked(111, 222);
}

#[naked]
fn naked(x: usize, y: usize) {
zzz(); // #break
}

fn zzz() { () }