Rework MIR inlining debuginfo so function parameters show up in debuggers.

[khuey]

Line numbers of multiply-inlined functions were fixed in #114643 by using a single DISubprogram. That, however, triggered assertions because parameters weren't deduplicated. The "solution" to that in #115417 was to insert a DILexicalScope below the DISubprogram and parent all of the parameters to that scope. That fixed the assertion, but debuggers (including gdb and lldb) don't recognize variables that are not parented to the subprogram itself as parameters, even if they are emitted with DW_TAG_formal_parameter.

Consider the program:

use std::env;

#[inline(always)]
fn square(n: i32) -> i32 {
    n * n
}

#[inline(never)]
fn square_no_inline(n: i32) -> i32 {
    n * n
}

fn main() {
    let x = square(env::vars().count() as i32);
    let y = square_no_inline(env::vars().count() as i32);
    println!("{x} == {y}");
}

When making a release build with debug=2 and rustc 1.82.0-nightly (8b38707 2024-08-07)

(gdb) r
Starting program: /ephemeral/tmp/target/release/tmp [Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".

Breakpoint 1, tmp::square () at src/main.rs:5
5	    n * n
(gdb) info args
No arguments.
(gdb) info locals
n = 31
(gdb) c
Continuing.

Breakpoint 2, tmp::square_no_inline (n=31) at src/main.rs:10
10	    n * n
(gdb) info args
n = 31
(gdb) info locals
No locals.

This issue is particularly annoying because it removes arguments from stack traces.

The DWARF for the inlined function looks like this:

< 2><0x00002132 GOFF=0x00002132>      DW_TAG_subprogram
                                        DW_AT_linkage_name          _ZN3tmp6square17hc507052ff3d2a488E
                                        DW_AT_name                  square
                                        DW_AT_decl_file             0x0000000f /ephemeral/tmp/src/main.rs
                                        DW_AT_decl_line             0x00000004
                                        DW_AT_type                  0x00001a56<.debug_info+0x00001a56>
                                        DW_AT_inline                DW_INL_inlined
< 3><0x00002142 GOFF=0x00002142>        DW_TAG_lexical_block
< 4><0x00002143 GOFF=0x00002143>          DW_TAG_formal_parameter
                                            DW_AT_name                  n
                                            DW_AT_decl_file             0x0000000f /ephemeral/tmp/src/main.rs
                                            DW_AT_decl_line             0x00000004
                                            DW_AT_type                  0x00001a56<.debug_info+0x00001a56>
< 4><0x0000214e GOFF=0x0000214e>          DW_TAG_null
< 3><0x0000214f GOFF=0x0000214f>        DW_TAG_null

That DW_TAG_lexical_block inhibits every debugger I've tested from recognizing 'n' as a parameter.

This patch removes the additional lexical scope. Parameters can be easily deduplicated by a tuple of their scope and the argument index, at the trivial cost of taking a Hash + Eq bound on DIScope.

[rustbot]

r? @Nadrieril

rustbot has assigned @Nadrieril.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[khuey]

r? @dpaoliello

[rustbot]

Failed to set assignee to dpaoliello: invalid assignee

Note: Only org members with at least the repository "read" role, users with write permissions, or people who have commented on the PR may be assigned.

[khuey]

r? @wesleywiser

[wesleywiser]

Thanks @khuey! The example you gave in the PR description was super helpful in understanding the issue and your fix 🥇

@bors r+

[bors]

📌 Commit 0e4136b has been approved by wesleywiser

It is now in the queue for this repository.

[matthiaskrgr]

@bors r-
#129022 (comment)

[khuey]

Looks like arguments with MSVC don't have the arg field set on the DILocalVariable. Not sure whether that's a bug, but I'll adjust the FileCheck annotations for now.

[khuey]

Would be good if someone could make bors try that.

[cjgillot]

Cc @saethlin as you've been working on debuginfo recently.

[saethlin]

Would be good if someone could make bors try that.

I can make that happen for you once mingw-check-tidy passes.

[saethlin]

Fuchsia infra problem maybe? https://rust-lang.zulipchat.com/#narrow/stream/242791-t-infra/topic/.E2.9C.94.20Are.20fuchsia.20CI.20jobs.20broken.3F/near/462428061

@bors retry

[bors]

⌛ Testing commit 1c5e3c9 with merge ffd6a66...

[rust-log-analyzer]

The job x86_64-mingw failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

[107/128] Building CXX object MachO/CMakeFiles/lldMachO.dir/UnwindInfoSection.cpp.obj
[108/128] Building CXX object MachO/CMakeFiles/lldMachO.dir/Writer.cpp.obj
[109/128] Building CXX object MinGW/CMakeFiles/lldMinGW.dir/Driver.cpp.obj
[110/128] Linking CXX static library lib\liblldMinGW.a
FAILED: lib/liblldMinGW.a 
C:\Windows\system32\cmd.exe /C "cd . && "C:\Program Files\CMake\bin\cmake.exe" -E rm -f lib\liblldMinGW.a && C:\a\rust\rust\mingw64\bin\ar.exe qc lib\liblldMinGW.a  MinGW/CMakeFiles/lldMinGW.dir/Driver.cpp.obj && C:\a\rust\rust\mingw64\bin\ranlib.exe lib\liblldMinGW.a && cd ."
C:\a\rust\rust\mingw64\bin\ar.exe: could not create temporary file whilst writing archive: no more archived files
[111/128] Linking CXX static library lib\liblldMachO.a
[112/128] Building CXX object wasm/CMakeFiles/lldWasm.dir/Driver.cpp.obj
[113/128] Building CXX object wasm/CMakeFiles/lldWasm.dir/InputFiles.cpp.obj
[114/128] Building CXX object wasm/CMakeFiles/lldWasm.dir/InputChunks.cpp.obj

[bors]

💔 Test failed - checks-actions

[khuey]

That also looks like it's unrelated to this patch.

[jieyouxu]

2024-08-15T00:27:34.6712215Z FAILED: lib/liblldMinGW.a 
2024-08-15T00:27:35.0246237Z C:\Windows\system32\cmd.exe /C "cd . && "C:\Program Files\CMake\bin\cmake.exe" -E rm -f lib\liblldMinGW.a && C:\a\rust\rust\mingw64\bin\ar.exe qc lib\liblldMinGW.a  MinGW/CMakeFiles/lldMinGW.dir/Driver.cpp.obj && C:\a\rust\rust\mingw64\bin\ranlib.exe lib\liblldMinGW.a && cd ."
2024-08-15T00:27:35.0248405Z C:\a\rust\rust\mingw64\bin\ar.exe: could not create temporary file whilst writing archive: no more archived files

@bors retry (looks unrelated to this PR)

[bors]

⌛ Testing commit 1c5e3c9 with merge 3139ff0...

[bors]

☀️ Test successful - checks-actions
Approved by: wesleywiser
Pushing 3139ff0 to master...

[rust-timer]

Finished benchmarking commit (3139ff0): comparison URL.

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	2.3%	[2.3%, 2.3%]	1
Improvements ✅ (primary)	-1.2%	[-1.4%, -1.1%]	6
Improvements ✅ (secondary)	-1.2%	[-1.3%, -1.2%]	2
All ❌✅ (primary)	-1.2%	[-1.4%, -1.1%]	6

Max RSS (memory usage)

Results (primary -2.9%, secondary 3.2%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	3.2%	[2.8%, 3.5%]	3
Improvements ✅ (primary)	-2.9%	[-4.1%, -2.1%]	4
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-2.9%	[-4.1%, -2.1%]	4

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

Results (primary -2.7%, secondary -1.5%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-2.7%	[-6.5%, -0.1%]	80
Improvements ✅ (secondary)	-1.5%	[-3.7%, -0.6%]	21
All ❌✅ (primary)	-2.7%	[-6.5%, -0.1%]	80

Bootstrap: 753.275s -> 751.489s (-0.24%)
Artifact size: 339.20 MiB -> 339.19 MiB (-0.00%)

[Kobzol]

More improvements than regressions. And awesome binary size wins!

@rustbot label: +perf-regression-triaged

[nnethercote]

This makes me wonder if there are any other debuginfo inefficiencies that could be fixed...

[khuey]

This makes me wonder if there are any other debuginfo inefficiencies that could be fixed...

There are a lot.

[cuviper]

FYI, this appears to be the cause of LLVM assertions while building cargo release with debuginfo.
https://rust-lang.zulipchat.com/#narrow/channel/122651-general/topic/Help.20minimizing.20LLVM.20assertion.20reproducer.3F/near/477751168