Skip to content

Don't let .debug_gdb_scripts become loadable into memory. #41627

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

Don't let .debug_gdb_scripts become loadable into memory. #41627

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bc6f725
Don't let .debug_gdb_scripts become loadable into memory.
eddyb Apr 29, 2017
f91ec4f
rustc_trans: use llvm.used instead of volatile loads for the gdb sect…
eddyb Apr 30, 2017
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
18 changes: 10 additions & 8 deletions src/librustc_trans/base.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -701,7 +701,7 @@ pub fn maybe_create_entry_wrapper(ccx: &CrateContext) {

let bld = Builder::new_block(ccx, llfn, "top");

debuginfo::gdb::insert_reference_to_gdb_debug_scripts_section_global(ccx, &bld);
debuginfo::gdb::insert_reference_to_gdb_debug_scripts_section_global(ccx);

let (start_fn, args) = if use_start_lang_item {
let start_def_id = ccx.tcx().require_lang_item(StartFnLangItem);
Expand Down Expand Up @@ -785,17 +785,19 @@ fn write_metadata<'a, 'gcx>(tcx: TyCtxt<'a, 'gcx, 'gcx>,
tcx.sess.cstore.metadata_section_name(&tcx.sess.target.target);
let name = CString::new(section_name).unwrap();
llvm::LLVMSetSection(llglobal, name.as_ptr());

// Also generate a .section directive to force no
// flags, at least for ELF outputs, so that the
// metadata doesn't get loaded into memory.
let directive = format!(".section {}", section_name);
let directive = CString::new(directive).unwrap();
llvm::LLVMSetModuleInlineAsm(metadata_llmod, directive.as_ptr())
make_section_non_loadable(metadata_llmod, section_name);
}
return (metadata_llcx, metadata_llmod, metadata);
}

/// Generate a .section directive to force no flags (e.g. for ELF outputs)
/// so that the contents of that section don't get loaded into memory.
pub unsafe fn make_section_non_loadable(llmod: ModuleRef, section: &str) {
let directive = format!(".section {}", section);
let directive = CString::new(directive).unwrap();
llvm::LLVMSetModuleInlineAsm(llmod, directive.as_ptr())
}

/// Find any symbols that are defined in one compilation unit, but not declared
/// in any other compilation unit. Give these symbols internal linkage.
fn internalize_symbols<'a, 'tcx>(sess: &Session,
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_trans/consts.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ pub fn trans_static<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>,

if attr::contains_name(attrs, "used") {
// This static will be stored in the llvm.used variable which is an array of i8*
let cast = llvm::LLVMConstPointerCast(g, Type::i8p(ccx).to_ref());
let cast = ptrcast(g, Type::i8p(ccx));
ccx.used_statics().borrow_mut().push(cast);
}

Expand Down
28 changes: 13 additions & 15 deletions src/librustc_trans/debuginfo/gdb.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,9 @@

use llvm;

use common::{C_bytes, CrateContext, C_i32};
use builder::Builder;
use common::{C_bytes, CrateContext};
use base;
use consts;
use declare;
use type_::Type;
use session::config::NoDebugInfo;
Expand All @@ -22,19 +23,13 @@ use std::ptr;
use syntax::attr;


/// Inserts a side-effect free instruction sequence that makes sure that the
/// .debug_gdb_scripts global is referenced, so it isn't removed by the linker.
pub fn insert_reference_to_gdb_debug_scripts_section_global(ccx: &CrateContext, builder: &Builder) {
/// Inserts the .debug_gdb_scripts global into the set of symbols
/// to be placed in `llvm.used`, so it isn't removed by the linker.
Copy link
Member

Choose a reason for hiding this comment

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

"by the linker" - you mean the compiler, right? The linker doesn't know or care about llvm.used stuff but likely it has some special case to preserve the .debug_gdb_scripts section.

Copy link
Member

Choose a reason for hiding this comment

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

No, by the linker. I thought that too, and started writing a comment, but turns out that it's not true; LLVM has a separate !llvm.compiler.used for what you say, and !llvm.used is defined to keep the symbol through the linking process as well.

Copy link
Member

@whitequark whitequark Apr 30, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Upon further investigation, on ELF, !llvm.used doesn't actually change anything in the object file, so LangRef is blatantly lying. On MachO, it uses a .no_dead_strip directive. On ELF...

  case MCSA_NoDeadStrip:
    // Ignore for now.
    break;

Copy link
Member

Choose a reason for hiding this comment

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

To add to this, the default linker script on x86_64 Linux has no mention of .debug_gdb_scripts (or a wildcard over all .debug* sections), and the logic of --gc-sections does not appear to special-case any of that either (it only really special-cases .eh_frame, in general; the rest is handled by traversing the graph). So this is interesting.

Copy link
Member Author

Choose a reason for hiding this comment

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

FWIW I only modified the part of the comment that was obviously wrong to me. Is it possible --gc-sections will strip this section, making it useless?

Copy link
Member

Choose a reason for hiding this comment

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

I have confirmed that the linker, called with --gc-sections, will, in fact, remove .debug_gdb_scripts (whether or not !llvm.used is used, of course). Not only that, but it will also gladly remove this section even when used exactly as directed by the gcc manual, and building with gcc and binutils ld!

My conclusion is that, I suppose not completely unexpectedly, this feature is poorly thought out and is not widely supported by toolchains, and Rust's original solution is gross but undeniably effective. Ideally, linker scripts (on ELF platforms) would be updated to do something like...

SECTIONS {
  .debug_gdb_scripts 0 (NOLOAD) : {
    KEEP(*(.debug_gdb_scripts))
  }
}

... but unfortunately, this doesn't work! Without the directive above, the section shows in objdump as:

  1 .debug_gdb_scripts 000000aa  ffff0000  ffff0000  00020000  2**0
                  CONTENTS, ALLOC, LOAD, READONLY, DATA

which is of course not what we want because it's LOAD. With (NOLOAD), however, it's another story entirely:

  1 .debug_gdb_scripts 000000aa  ffff0000  ffff0000  00020000  2**0
                  ALLOC, READONLY

So in a direct contradiction with the ld manual, which claims that:

The linker will process the section normally, but will mark it so that a program loader will not load it into memory. [...] The contents of the [...] section will appear in the linker output file as usual.

the contents of the section does not, in fact, appear in the output file at all.

I'd like to be proven wrong but I am afraid it's not possible to implement this feature.

Copy link
Member

Choose a reason for hiding this comment

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

Actually, I have proven myself wrong. If I use the extremely obscure output section type of INFO in the linker script, then exactly the thing that I expect happens!

.debug_gdb_scripts 0 (INFO) : {
  KEEP(*(.debug_gdb_scripts))
}

  1 .debug_gdb_scripts 000000aa  00000000  00000000  00011848  2**0
                  CONTENTS, READONLY, DEBUGGING

I do not believe it is viable to integrate this into upstream rustc and so this PR and its parent issue may be closed.

Copy link
Member

Choose a reason for hiding this comment

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

So incidentally I was curious where did the DEBUGGING bit appear, since that's not something gcc (or gas) can do, and I looked at the binutils source code. It looks like all .debug* sections should have their SEC_ALLOC bit removed (and SEC_DEBUGGING bit added)...

  if ((flags & SEC_ALLOC) == 0)
    {
      /* The debugging sections appear to be recognized only by name,
	 not any sort of flag.  Their SEC_ALLOC bits are cleared.  */
      if (name [0] == '.')
	{
	  const char *p;
	  int n;
	  if (name[1] == 'd')
	    p = ".debug", n = 6;
	  // [snip]
	  else
	    p = NULL, n = 0;
	  if (p != NULL && strncmp (name, p, n) == 0)
	    flags |= SEC_DEBUGGING;
	}

... but this is evidently not happening for .debug_gdb_scripts for some reason that I have no doubt is deeply unsettling in more than one way. There's really a large amount of logic in binutils that touches SEC_DEBUGGING, most of which is almost but not exactly identical platform code, and the more I look into it, the less I want to be aware of it. Or be conscious.

pub fn insert_reference_to_gdb_debug_scripts_section_global(ccx: &CrateContext) {
if needs_gdb_debug_scripts_section(ccx) {
let gdb_debug_scripts_section_global = get_or_insert_gdb_debug_scripts_section_global(ccx);
// Load just the first byte as that's all that's necessary to force
// LLVM to keep around the reference to the global.
let indices = [C_i32(ccx, 0), C_i32(ccx, 0)];
let element = builder.inbounds_gep(gdb_debug_scripts_section_global, &indices);
let volative_load_instruction = builder.volatile_load(element);
unsafe {
llvm::LLVMSetAlignment(volative_load_instruction, 1);
}
let cast = consts::ptrcast(gdb_debug_scripts_section_global, Type::i8p(ccx));
ccx.used_statics().borrow_mut().push(cast);
}
}

Expand All @@ -51,7 +46,9 @@ pub fn get_or_insert_gdb_debug_scripts_section_global(ccx: &CrateContext)
};

if section_var == ptr::null_mut() {
let section_name = b".debug_gdb_scripts\0";
let c_section_name = ".debug_gdb_scripts\0";
let section_name = &c_section_name[..c_section_name.len()-1];

let section_contents = b"\x01gdb_load_rust_pretty_printers.py\0";

unsafe {
Expand All @@ -62,7 +59,8 @@ pub fn get_or_insert_gdb_debug_scripts_section_global(ccx: &CrateContext)
llvm_type).unwrap_or_else(||{
bug!("symbol `{}` is already defined", section_var_name)
});
llvm::LLVMSetSection(section_var, section_name.as_ptr() as *const _);
llvm::LLVMSetSection(section_var, c_section_name.as_ptr() as *const _);
base::make_section_non_loadable(ccx.llmod(), section_name);
llvm::LLVMSetInitializer(section_var, C_bytes(ccx, section_contents));
llvm::LLVMSetGlobalConstant(section_var, llvm::True);
llvm::LLVMSetUnnamedAddr(section_var, llvm::True);
Expand Down