Take DW_TAG_namespace's name into account when reading DW_AT_name

[bjorn3]

For example alloc::boxed::Box::new will have a DW_AT_name of new, but it is nested in several DW_TAG_namespace which are named alloc, boxed and Box respectively.

[philipc]

Finding the DW_TAG_namespace entries would be very expensive, and DW_AT_name is only a fallback. We should be using DW_AT_linkage_name instead:

< 4><0x00000039>          DW_TAG_subprogram
                            DW_AT_linkage_name          _ZN5alloc5boxed12Box$LT$T$GT$3new17h8819ec3570e2d1e8E
                            DW_AT_name                  new<u32>
                            DW_AT_decl_file             0x00000002 /rustc/4fb7144ed159f94491249e86d5bbd033b5d60550/src/liballoc/boxed.rs
                            DW_AT_decl_line             0x000000ae
                            DW_AT_type                  0x00000068<.debug_info+0x0000033a>
                            DW_AT_inline                DW_INL_inlined

[bjorn3]

The linkage name is missing the type parameters.

[philipc]

Is this issue also proposing that we use DW_AT_name instead of DW_AT_linkage_name?

[bjorn3]

I guess it could be another method.

[philipc]

Is this still of interest to you?

Rough proof of concept at philipc@c532758
This changes it to return both the linkage name and the name/namespace.

Performance comparison below. There might be room for some optimization.

 name                                        before ns/iter  after ns/iter  diff ns/iter  diff %  speedup 
 context_new_and_query_location_rc           1,576,415       1,802,732           226,317  14.36%   x 0.87 
 context_new_and_query_location_slice        607,612         610,777               3,165   0.52%   x 0.99 
 context_new_and_query_with_functions_rc     1,732,377       1,784,938            52,561   3.03%   x 0.97 
 context_new_and_query_with_functions_slice  759,694         762,421               2,727   0.36%   x 1.00 
 context_new_parse_functions_rc              9,991,413       10,133,597          142,184   1.42%   x 0.99 
 context_new_parse_functions_slice           9,150,412       9,214,042            63,630   0.70%   x 0.99 
 context_new_parse_inlined_functions_rc      33,198,829      39,720,621        6,521,792  19.64%   x 0.84 
 context_new_parse_inlined_functions_slice   28,527,872      33,520,880        4,993,008  17.50%   x 0.85 
 context_new_parse_lines_rc                  6,731,890       6,812,627            80,737   1.20%   x 0.99 
 context_new_parse_lines_slice               4,934,562       5,022,465            87,903   1.78%   x 0.98 
 context_new_rc                              1,475,518       1,465,765            -9,753  -0.66%   x 1.01 
 context_new_slice                           539,449         506,929             -32,520  -6.03%   x 1.06 
 context_query_location_rc                   1,114,929       1,115,536               607   0.05%   x 1.00 
 context_query_location_slice                1,158,963       1,178,368            19,405   1.67%   x 0.98 
 context_query_with_functions_rc             2,593,325       2,809,019           215,694   8.32%   x 0.92 
 context_query_with_functions_slice          2,486,511       2,550,311            63,800   2.57%   x 0.97 

[bjorn3]

The project I needed this for is no longer being worked on, so I don't need this anymore myself. Maybe others still have a use case for it?

The linkage name is missing the type parameters.

-Csymbol-mangling-version=v0 solves this problem by including them in the mangled symbol name.

[mstange]

I haven't gotten any reports asking for adding the argument types. I've asked a few people who use samply to profile Rust code and the sentiment was along the lines of: "Having the arguments would be nice, I suppose, but I haven't noticed their absence"

[jyn514]

This is observable when using -C debuginfo=line-tables-only, in which case DW_AT_linkage_name is not present. Given that DW_AT_name is used as a fallback only when linkage_name isn't present, maybe it makes sense to take the perf hit?

Note also that using the linkage name is incorrect when the linkage name differs from the DW_AT_name. For example, std::panicking::begin_panic_handler is linked as rust_begin_unwind.

$ CARGO_PROFILE_RELEASE_DEBUG=line-tables-only RUST_BACKTRACE=short cargo run --release
stack backtrace:
   0: rust_begin_unwind
             at /rustc/049355708383ab1b9a1046559b9d4230bdb3a5bc/library/std/src/panicking.rs:695:5
   1: core::panicking::panic_fmt
             at /rustc/049355708383ab1b9a1046559b9d4230bdb3a5bc/library/core/src/panicking.rs:75:14
   2: main
             at ./src/main.rs:6:5

; CARGO_PROFILE_RELEASE_DEBUG=1 RUST_BACKTRACE=short c r --release
stack backtrace:
   0: rust_begin_unwind
             at /rustc/049355708383ab1b9a1046559b9d4230bdb3a5bc/library/std/src/panicking.rs:695:5
   1: core::panicking::panic_fmt
             at /rustc/049355708383ab1b9a1046559b9d4230bdb3a5bc/library/core/src/panicking.rs:75:14
   2: example::main
             at ./src/main.rs:6:5

[philipc]

This is observable when using -C debuginfo=line-tables-only, in which case DW_AT_linkage_name is not present.

I think it's present for some things, but not everything. My opinion is this is an LLVM oversight. gcc includes it. A workaround is to use the symbol table first, which this crate did for its CLI in #332, but other users need to do that themselves.

I wasn't aware of things like rust_begin_unwind, but that doesn't seem like something that users are normally interested in, and the line number info will still be correct.

[jyn514]

I think it's present for some things, but not everything. My opinion is this is an LLVM oversight. gcc includes it. A workaround is to use the symbol table first, which this crate did for its CLI in #332, but other users need to do that themselves.

hmm, ok. how do you feel about moving this into the addr2line library instead of having every downstream reimplement the symbol table lookup?

[jyn514]

i did have to make the stack trace include at least one inlined function before it replicated the missing DW_AT_linkage_name, so llvm is probably omitting it for inlined functions.

[philipc]

That matches my memory of what LLVM is doing.

The primary users of this crate use Context, and by design Context only handles DWARF. Symbol table support is dependent on the object file loader, and this crate does not want to restrict that.

We do provide symbol table support in Loader, but we don't provide a method that combines both a symbol table lookup and a DWARF lookup. Maybe we could add a method for this, but it wouldn't help the primary users.

[workingjubilee]

It seems to me addr2line does not support ergonomically doing anything else but using Context, so was that mere observation or truly a statement of desire?

[philipc]

My statement was explaining why this would not have the desired outcome of helping every downstream user:

how do you feel about moving this into the addr2line library instead of having every downstream reimplement the symbol table lookup?

[jyn514]

I wasn't aware of things like rust_begin_unwind, but that doesn't seem like something that users are normally interested in, and the line number info will still be correct.

here is another case where users would be interested and the line number is not correct: default implementations of a trait.

< 4><0x00000810>          DW_TAG_subprogram
                            DW_AT_low_pc                0x002c75c0
                            DW_AT_high_pc               <offset-from-lowpc> 319 <highpc: 0x002c76ff>
                            DW_AT_frame_base            len 0x0001: 0x57:
                                DW_OP_reg7
                            DW_AT_linkage_name          _ZN3std2io19default_read_to_end28_$u7b$$u7b$closure$u7d$$u7d$17hcc4683a285cc2869E
                            DW_AT_name                  {closure#0}<ruzstd::streaming_decoder::StreamingDecoder<&mut &[u8], ruzstd::frame_decoder::FrameDecoder>>
                            DW_AT_decl_file             0x00000003 /home/jyn/.local/lib/rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/io/mod.rs

here the linkage name points to std::io::default_read_to_end::{{closure}}, but "abstractly" this is <ruzstd::streaming_decoder::StreamingDecoder as std::io::Read>::read_to_end.

[philipc]

It's not clear to me what's going on from the information you have given, and I'm not convinced it is anything specific to default implementations.

You said that abstractly this is <ruzstd::streaming_decoder::StreamingDecoder as std::io::Read>::read_to_end, but that's not what DW_AT_name gives either, so how does using DW_AT_name fix it? You haven't shown the parent namespace names, but I suspect they don't include read_to_end either.

Secondly, the default implementation of read_to_end is a function that calls default_read_to_end, and I suspect that the address range 0x002c75c0-0x002c76ff really is code that is defined by the function default_read_to_end, so returning that name and line number is correct (and I expect that read_to_end will appear somewhere else in the inline function information). What's missing is that default_read_to_end has a generic parameter which is not included in the linkage name, and that's included in DW_AT_name (that is, the problem is identical to the motivation given in #181 (comment)).