no_mangle static symbols have improper debug info

[m4b]

Repro

1. cargo new --bin static

2. cd static

3. add the following code to main.rs:

   #[no_mangle]
   pub static TEST: u64 = 0xdeadbeef;
   
   pub fn main() {
   println!("TEST: {}", TEST);
   }

4. cargo build

5. gdb target/debug/static

6. break static::main

7. r

8. whatis TEST

9. output: type = <data variable, no debug info>

10. frown

When TEST is a repr(C) struct, etc., this makes debugging extraordinarily difficult and laborious.

Actual Use Case

I'm implementing the gdb debugger interface specified in /usr/include/link.h, which requires a single symbol, _r_debug to be present in the _DYNAMIC array.

(The symbol is actually not exported either, roughly GLOBAL and in the _DYNAMIC array, which I get around by using a --dynamic-list flag to the linker.)

I don't get around the lack of debug symbols at all, which vexes me.

[tromey]

This is issue #32574.

Note that gdb supports a second, improved interface to detecting shared library events. I don't remember exactly when this went in but here's one of the series; if you're interested this should help find the final patches:

http://www.cygwin.com/ml/gdb-patches/2012-07/msg00157.html

There's a Rust crate for making stap-style probes.

[Mark-Simulacrum]

I'm going to close in favor of #32574. Thanks for the issue, though.

[m4b]

I believe that we should re-open this issue in light of #46453

[Mark-Simulacrum]

Doesn't #46453 already cover the problem listed here? Or is it different? If it's different I'm happy to reopen.

[m4b]

I think that this is actually an extra bug, in that the no_mangle seems to be ignored and it is given linkage_name, whereas it shouldn't receive any at all.

Even if #46453 is fixed, I believe this issue will persist, because it would still be given a mangled linkage_name which will reference a symbol that won't ever exist, since the name isn't mangled in the symbol table, and hence it shouldn't be given one at all.

Does that make sense?

[m4b]

To illustrate, here is the rust no_mangle static version:

DW_TAG_variable [3]  
  DW_AT_name [DW_FORM_strp]	( .debug_str[0x00000061] = "TEST")
  DW_AT_type [DW_FORM_ref4]	(cu + 0x0049 => {0x00000049})
  DW_AT_external [DW_FORM_flag_present]	(true)
  DW_AT_decl_file [DW_FORM_data1]	("/home/m4b/tmp/bad_debug/test.rs")
  DW_AT_decl_line [DW_FORM_data1]	(8)
  DW_AT_alignment [DW_FORM_udata]	(1)
  DW_AT_location [DW_FORM_exprloc]	(<0x9> 03 c0 65 05 00 00 00 00 00 )
  DW_AT_linkage_name [DW_FORM_strp]	( .debug_str[0x00000066] = "_ZN4test4TESTE")

and the C++ unmangled static:

DW_TAG_variable [9]  
  DW_AT_name [DW_FORM_strp]       ( .debug_str[0x00000053] = "TEST")
  DW_AT_type [DW_FORM_ref4]       (cu + 0x0048 => {0x00000048})
  DW_AT_external [DW_FORM_flag_present]   (true)
  DW_AT_decl_file [DW_FORM_data1] ("/home/m4b/tmp/bad_debug/test.cpp")
  DW_AT_decl_line [DW_FORM_data1] (14)
  DW_AT_location [DW_FORM_exprloc]        (<0x9> 03 40 10 20 00 00 00 00 00 )

Iiuc, everything is perfectly correct, except we just have to not emit the DW_AT_linkage_name in no_mangle cases.

[hanna-kruppe]

I believe that a "principled" fix for #46453 would also automatically solve this issue. There's already code in the compiler that computes the real symbol name of all the functions and statics we emit. Using that code for the DWARF linkage_name would give the right linkage_name to all symbols, no_mangle ones included. That would set a linkage_name identical to the DWARF name, which would differ from C++, but I can't imagine that would cause harm?

[tromey]

That would set a linkage_name identical to the DWARF name, which would differ from C++, but I can't imagine that would cause harm?

It should be fine, but in cases where the two are equal, it is fine to omit the linkage name, and this uses less space. This is what C compilers do.

[hanna-kruppe]

Regardless, that's just a small if. The important part is unifying the symbol/linkage name computation.

[wesleywiser]

Visited during the wg-debugging triage meeting and this seems to have been resolved. @rylev is going to add a regression test.