-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Initialization of single-use-consts happens before the function prelude #128945
Comments
…=<try> Enable debuginfo tests that were "temporarily disabled" for the past 6 years `tests/debuginfo/drop-locations.rs` No idea why we are setting `-O -Cno-prepopulate-passes`. Nowadays that turns on MIR opts, no idea what it did in 2018. Turning off optimizations makes just loc2 missing. That seems like a bug in debuginfo generation. Turning off all MIR opts and even adding a `drop(s)` on that line doesn't help. Cursed. Keeping this test ignored. `tests/debuginfo/option-like-enum.rs` Debuggers are better at Rust! Also one inexplicable change. No idea why we used to expect that. Deleted. `tests/debuginfo/function-arg-initialization.rs` rust-lang#128945 `tests/debuginfo/macro-stepping.rs` Also broken by SingleUseConsts. I hate tests like this, nobody uses a debugger on code that's just assigning consts to unused variables. Test should be rewritten with realistic code. `tests/debuginfo/basic-types-metadata.rs` Debuggers are better at Rust! Diverging functions don't have -> ! in their whatis output anymore? Not sure if that is a bug. --- More failures on apple! `tests/debuginfo/by-value-non-immediate-argument.rs` `tests/debuginfo/function-arg-initialization.rs` `tests/debuginfo/function-prologue-stepping-regular.rs` I think all of these indicate that lldb on aarch64-apple does not understand indirect by-value non-immediate arguments. Is that known? `tests/debuginfo/method-on-enum.rs` `tests/debuginfo/option-like-enum.rs` `tests/debuginfo/struct-in-enum.rs` lldb on aarch64-apple can't print enums until at least llvm 18. We use an old lldb on apple. try-job: test-various try-job: x86_64-msvc try-job: x86_64-mingw try-job: i686-msvc try-job: i686-mingw try-job: armhf-gnu try-job: dist-various-1
…=<try> Enable debuginfo tests that have been "temporarily disabled" for the past 6 years `tests/debuginfo/drop-locations.rs` No idea why we are setting `-O -Cno-prepopulate-passes`. Nowadays that turns on MIR opts, no idea what it did in 2018. Turning off optimizations makes just loc2 missing. That seems like a bug in debuginfo generation. Turning off all MIR opts and even adding a `drop(s)` on that line doesn't help. Cursed. Keeping this test ignored. `tests/debuginfo/option-like-enum.rs` Debuggers are better at Rust! Also one inexplicable change. No idea why we used to expect that. Deleted. `tests/debuginfo/function-arg-initialization.rs` rust-lang#128945 `tests/debuginfo/macro-stepping.rs` Also broken by SingleUseConsts. I hate tests like this, nobody uses a debugger on code that's just assigning consts to unused variables. Test should be rewritten with realistic code. `tests/debuginfo/basic-types-metadata.rs` Debuggers are better at Rust! Diverging functions don't have -> ! in their whatis output anymore? Not sure if that is a bug. --- More failures on apple! `tests/debuginfo/by-value-non-immediate-argument.rs` `tests/debuginfo/function-arg-initialization.rs` `tests/debuginfo/function-prologue-stepping-regular.rs` I think all of these indicate that lldb on aarch64-apple does not understand indirect by-value non-immediate arguments. Is that known? `tests/debuginfo/method-on-enum.rs` `tests/debuginfo/option-like-enum.rs` `tests/debuginfo/struct-in-enum.rs` lldb on aarch64-apple can't print enums until at least llvm 18. We use an old lldb on apple. --- aarch64-gnu fails on `tests/debuginfo/by-value-non-immediate-argument.rs` as well. Looks like either a codegen bug or a gdb bug. --- The prettyprinters that we need for `tests/debuginfo/pretty-std.rs` are not loaded on windows-gnu. It's not entirely clear why, but I suspect that the escaping that was added in rust-lang#17161 is interfering somehow. --- try-job: x86_64-msvc try-job: x86_64-mingw try-job: i686-msvc try-job: i686-mingw
…=<try> Enable debuginfo tests that have been "temporarily disabled" for the past 6 years `tests/debuginfo/drop-locations.rs` No idea why we are setting `-O -Cno-prepopulate-passes`. Nowadays that turns on MIR opts, no idea what it did in 2018. Turning off optimizations makes just loc2 missing. That seems like a bug in debuginfo generation. Turning off all MIR opts and even adding a `drop(s)` on that line doesn't help. Cursed. Keeping this test ignored. `tests/debuginfo/option-like-enum.rs` Debuggers are better at Rust! Also one inexplicable change. No idea why we used to expect that. Deleted. `tests/debuginfo/function-arg-initialization.rs` rust-lang#128945 `tests/debuginfo/macro-stepping.rs` Also broken by SingleUseConsts. I hate tests like this, nobody uses a debugger on code that's just assigning consts to unused variables. Test should be rewritten with realistic code. `tests/debuginfo/basic-types-metadata.rs` Debuggers are better at Rust! Diverging functions don't have -> ! in their whatis output anymore? Not sure if that is a bug. --- More failures on apple! `tests/debuginfo/by-value-non-immediate-argument.rs` `tests/debuginfo/function-arg-initialization.rs` `tests/debuginfo/function-prologue-stepping-regular.rs` I think all of these indicate that lldb on aarch64-apple does not understand indirect by-value non-immediate arguments. Is that known? `tests/debuginfo/method-on-enum.rs` `tests/debuginfo/option-like-enum.rs` `tests/debuginfo/struct-in-enum.rs` lldb on aarch64-apple can't print enums until at least llvm 18. We use an old lldb on apple. --- aarch64-gnu fails on `tests/debuginfo/by-value-non-immediate-argument.rs` as well. Looks like either a codegen bug or a gdb bug. --- The prettyprinters that we need for `tests/debuginfo/pretty-std.rs` are not loaded on windows-gnu. It's not entirely clear why, but I suspect that the escaping that was added in rust-lang#17161 is interfering somehow. --- try-job: x86_64-msvc try-job: x86_64-mingw try-job: i686-msvc try-job: i686-mingw
…=compiler-errors Enable debuginfo tests that have been "temporarily disabled" for the past 6 years The PR history is a bit of a mess because I had to test this a lot with try-jobs, so I'll try to summarize the non-obvious changes here. A number of tests now have `min-lldb-version: 1800`. Those tests should have gotten an lldb version jump either in rust-lang#124781 or long ago. Note that all such tests with that lldb version requirement do not run in Apple CI. `tests/debuginfo/drop-locations.rs` is staying disabled for now because gdb doesn't know to stop on the drop calls produced by a `}`: rust-lang#128971 `tests/debuginfo/function-arg-initialization.rs` now has `-Zmir-enable-passes=-SingleUseConsts`; without that we initialize the const before the function prelude: rust-lang#128945 `tests/debuginfo/by-value-non-immediate-argument.rs` fails because we don't generate a function prelude for unused non-immediate arguments, even with all optimizations disabled, and this seems to confuse debuggers on aarch64: rust-lang#128973 `tests/debuginfo/pretty-std.rs` is staying disabled on windows-gnu because our test harness doesn't know how to load our pretty-printers on that target: rust-lang#128981 `tests/debuginfo/method-on-enum.rs` and `tests/debuginfo/option-like-enum.rs` encounter some kind of gdb bug on i686-pc-windows-gnu. I don't know enough about that situation to write a good issue. I plan on doing more work on this test suite. There's clearly a lot more basic cleanup work to do here.
…=<try> Enable debuginfo tests that have been "temporarily disabled" for the past 6 years The PR history is a bit of a mess because I had to test this a lot with try-jobs, so I'll try to summarize the non-obvious changes here. A number of tests now have `min-lldb-version: 1800`. Those tests should have gotten an lldb version jump either in rust-lang#124781 or long ago. Note that all such tests with that lldb version requirement do not run in Apple CI. `tests/debuginfo/drop-locations.rs` is staying disabled for now because gdb doesn't know to stop on the drop calls produced by a `}`: rust-lang#128971 `tests/debuginfo/function-arg-initialization.rs` now has `-Zmir-enable-passes=-SingleUseConsts`; without that we initialize the const before the function prelude: rust-lang#128945 `tests/debuginfo/by-value-non-immediate-argument.rs` fails because we don't generate a function prelude for unused non-immediate arguments, even with all optimizations disabled, and this seems to confuse debuggers on aarch64: rust-lang#128973 `tests/debuginfo/pretty-std.rs` is staying disabled on windows-gnu because our test harness doesn't know how to load our pretty-printers on that target: rust-lang#128981 `tests/debuginfo/method-on-enum.rs` and `tests/debuginfo/option-like-enum.rs` encounter some kind of gdb bug on i686-pc-windows-gnu. I don't know enough about that situation to write a good issue. I plan on doing more work on this test suite. There's clearly a lot more basic cleanup work to do here. try-job: i686-gnu try-job: aarch64-gnu try-job: x86_64-msvc try-job: i686-mingw
…=compiler-errors Enable debuginfo tests that have been "temporarily disabled" for the past 6 years The PR history is a bit of a mess because I had to test this a lot with try-jobs, so I'll try to summarize the non-obvious changes here. A number of tests now have `min-lldb-version: 1800`. Those tests should have gotten an lldb version jump either in rust-lang#124781 or long ago. Note that all such tests with that lldb version requirement do not run in Apple CI. `tests/debuginfo/drop-locations.rs` is staying disabled for now because gdb doesn't know to stop on the drop calls produced by a `}`: rust-lang#128971 `tests/debuginfo/function-arg-initialization.rs` now has `-Zmir-enable-passes=-SingleUseConsts`; without that we initialize the const before the function prelude: rust-lang#128945 `tests/debuginfo/by-value-non-immediate-argument.rs` fails because we don't generate a function prelude for unused non-immediate arguments, even with all optimizations disabled, and this seems to confuse debuggers on aarch64: rust-lang#128973 `tests/debuginfo/pretty-std.rs` is staying disabled on windows-gnu because our test harness doesn't know how to load our pretty-printers on that target: rust-lang#128981 `tests/debuginfo/method-on-enum.rs` and `tests/debuginfo/option-like-enum.rs` encounter some kind of gdb bug on i686-pc-windows-gnu. I don't know enough about that situation to write a good issue. I plan on doing more work on this test suite. There's clearly a lot more basic cleanup work to do here.
…=<try> Enable debuginfo tests that have been "temporarily disabled" for the past 6 years The PR history is a bit of a mess because I had to test this a lot with try-jobs, so I'll try to summarize the non-obvious changes here. A number of tests now have `min-lldb-version: 1800`. Those tests should have gotten an lldb version jump either in rust-lang#124781 or long ago. Note that all such tests with that lldb version requirement do not run in Apple CI. `tests/debuginfo/drop-locations.rs` is staying disabled for now because gdb doesn't know to stop on the drop calls produced by a `}`: rust-lang#128971 `tests/debuginfo/function-arg-initialization.rs` now has `-Zmir-enable-passes=-SingleUseConsts`; without that we initialize the const before the function prelude: rust-lang#128945 `tests/debuginfo/by-value-non-immediate-argument.rs` fails because we don't generate a function prelude for unused non-immediate arguments, even with all optimizations disabled, and this seems to confuse debuggers on aarch64: rust-lang#128973 `tests/debuginfo/pretty-std.rs` is staying disabled on windows-gnu because our test harness doesn't know how to load our pretty-printers on that target: rust-lang#128981 `tests/debuginfo/method-on-enum.rs` and `tests/debuginfo/option-like-enum.rs` encounter some kind of gdb bug on i686-pc-windows-gnu. I don't know enough about that situation to write a good issue. I plan on doing more work on this test suite. There's clearly a lot more basic cleanup work to do here. try-job: x86_64-gnu-llvm-17 try-job: x86_64-gnu-tools try-job: aarch64-gnu try-job: arm-android try-job: armhf-gnu try-job: i686-gnu try-job: i686-gnu-nopt try-job: mingw-check try-job: test-various try-job: x86_64-fuchsia try-job: x86_64-rust-for-linux try-job: x86_64-gnu try-job: x86_64-gnu-stable try-job: x86_64-gnu-aux try-job: x86_64-gnu-debug try-job: x86_64-gnu-llvm-18 try-job: x86_64-gnu-nopt try-job: x86_64-apple-1 try-job: aarch64-apple try-job: x86_64-msvc try-job: i686-msvc try-job: i686-mingw try-job: x86_64-mingw
…=<try> Enable debuginfo tests that have been "temporarily disabled" for the past 6 years The PR history is a bit of a mess because I had to test this a lot with try-jobs, so I'll try to summarize the non-obvious changes here. A number of tests now have `min-lldb-version: 1800`. Those tests should have gotten an lldb version jump either in rust-lang#124781 or long ago. Note that all such tests with that lldb version requirement do not run in Apple CI. `tests/debuginfo/drop-locations.rs` is staying disabled for now because gdb doesn't know to stop on the drop calls produced by a `}`: rust-lang#128971 `tests/debuginfo/function-arg-initialization.rs` now has `-Zmir-enable-passes=-SingleUseConsts`; without that we initialize the const before the function prelude: rust-lang#128945 `tests/debuginfo/by-value-non-immediate-argument.rs` fails because we don't generate a function prelude for unused non-immediate arguments, even with all optimizations disabled, and this seems to confuse debuggers on aarch64: rust-lang#128973 `tests/debuginfo/pretty-std.rs` is staying disabled on windows-gnu because our test harness doesn't know how to load our pretty-printers on that target: rust-lang#128981 `tests/debuginfo/method-on-enum.rs` and `tests/debuginfo/option-like-enum.rs` encounter some kind of gdb bug on i686-pc-windows-gnu. I don't know enough about that situation to write a good issue. I plan on doing more work on this test suite. There's clearly a lot more basic cleanup work to do here. try-job: aarch64-gnu try-job: arm-android try-job: armhf-gnu try-job: i686-gnu try-job: i686-gnu-nopt try-job: test-various try-job: x86_64-fuchsia try-job: x86_64-rust-for-linux try-job: x86_64-gnu try-job: x86_64-gnu-stable
…=<try> Enable debuginfo tests that have been "temporarily disabled" for the past 6 years The PR history is a bit of a mess because I had to test this a lot with try-jobs, so I'll try to summarize the non-obvious changes here. A number of tests now have `min-lldb-version: 1800`. Those tests should have gotten an lldb version jump either in rust-lang#124781 or long ago. Note that all such tests with that lldb version requirement do not run in Apple CI. `tests/debuginfo/drop-locations.rs` is staying disabled for now because gdb doesn't know to stop on the drop calls produced by a `}`: rust-lang#128971 `tests/debuginfo/function-arg-initialization.rs` now has `-Zmir-enable-passes=-SingleUseConsts`; without that we initialize the const before the function prelude: rust-lang#128945 `tests/debuginfo/by-value-non-immediate-argument.rs` fails because we don't generate a function prelude for unused non-immediate arguments, even with all optimizations disabled, and this seems to confuse debuggers on aarch64: rust-lang#128973 `tests/debuginfo/pretty-std.rs` is staying disabled on windows-gnu because our test harness doesn't know how to load our pretty-printers on that target: rust-lang#128981 `tests/debuginfo/method-on-enum.rs` and `tests/debuginfo/option-like-enum.rs` encounter some kind of gdb bug on i686-pc-windows-gnu. I don't know enough about that situation to write a good issue. I plan on doing more work on this test suite. There's clearly a lot more basic cleanup work to do here. try-job: aarch64-apple try-job: aarch64-gnu try-job: dist-i586-gnu-i586-i686-musl try-job: i686-gnu try-job: i686-gnu-nopt try-job: i686-mingw try-job: i686-msvc try-job: test-various try-job: x86_64-apple-1
…=<try> Enable debuginfo tests that have been "temporarily disabled" for the past 6 years The PR history is a bit of a mess because I had to test this a lot with try-jobs, so I'll try to summarize the non-obvious changes here. A number of tests now have `min-lldb-version: 1800`. Those tests should have gotten an lldb version jump either in rust-lang#124781 or long ago. Note that all such tests with that lldb version requirement do not run in Apple CI. `tests/debuginfo/drop-locations.rs` is staying disabled for now because gdb doesn't know to stop on the drop calls produced by a `}`: rust-lang#128971 `tests/debuginfo/function-arg-initialization.rs` now has `-Zmir-enable-passes=-SingleUseConsts`; without that we initialize the const before the function prelude: rust-lang#128945 `tests/debuginfo/by-value-non-immediate-argument.rs` fails because we don't generate a function prelude for unused non-immediate arguments, even with all optimizations disabled, and this seems to confuse debuggers on aarch64: rust-lang#128973 `tests/debuginfo/pretty-std.rs` is staying disabled on windows-gnu because our test harness doesn't know how to load our pretty-printers on that target: rust-lang#128981 `tests/debuginfo/method-on-enum.rs` and `tests/debuginfo/option-like-enum.rs` encounter some kind of gdb bug on i686-pc-windows-gnu. I don't know enough about that situation to write a good issue. I plan on doing more work on this test suite. There's clearly a lot more basic cleanup work to do here. try-job: x86_64-gnu try-job: x86_64-gnu-distcheck try-job: x86_64-gnu-llvm-17 try-job: x86_64-gnu-llvm-18 try-job: x86_64-gnu-nopt try-job: x86_64-gnu-stable try-job: x86_64-mingw try-job: x86_64-msvc
…=compiler-errors Enable debuginfo tests that have been "temporarily disabled" for the past 6 years The PR history is a bit of a mess because I had to test this a lot with try-jobs, so I'll try to summarize the non-obvious changes here. A number of tests now have `min-lldb-version: 1800`. Those tests should have gotten an lldb version jump either in rust-lang#124781 or long ago. Note that all such tests with that lldb version requirement do not run in Apple CI. `tests/debuginfo/drop-locations.rs` is staying disabled for now because gdb doesn't know to stop on the drop calls produced by a `}`: rust-lang#128971 `tests/debuginfo/function-arg-initialization.rs` now has `-Zmir-enable-passes=-SingleUseConsts`; without that we initialize the const before the function prelude: rust-lang#128945 `tests/debuginfo/by-value-non-immediate-argument.rs` fails because we don't generate a function prelude for unused non-immediate arguments, even with all optimizations disabled, and this seems to confuse debuggers on aarch64: rust-lang#128973 `tests/debuginfo/pretty-std.rs` is staying disabled on windows-gnu because our test harness doesn't know how to load our pretty-printers on that target: rust-lang#128981 `tests/debuginfo/method-on-enum.rs` and `tests/debuginfo/option-like-enum.rs` encounter some kind of gdb bug on i686-pc-windows-gnu. I don't know enough about that situation to write a good issue. I plan on doing more work on this test suite. There's clearly a lot more basic cleanup work to do here.
…-errors Enable debuginfo tests that have been "temporarily disabled" for the past 6 years The PR history is a bit of a mess because I had to test this a lot with try-jobs, so I'll try to summarize the non-obvious changes here. A number of tests now have `min-lldb-version: 1800`. Those tests should have gotten an lldb version jump either in rust-lang/rust#124781 or long ago. Note that all such tests with that lldb version requirement do not run in Apple CI. `tests/debuginfo/drop-locations.rs` is staying disabled for now because gdb doesn't know to stop on the drop calls produced by a `}`: rust-lang/rust#128971 `tests/debuginfo/function-arg-initialization.rs` now has `-Zmir-enable-passes=-SingleUseConsts`; without that we initialize the const before the function prelude: rust-lang/rust#128945 `tests/debuginfo/by-value-non-immediate-argument.rs` fails because we don't generate a function prelude for unused non-immediate arguments, even with all optimizations disabled, and this seems to confuse debuggers on aarch64: rust-lang/rust#128973 `tests/debuginfo/pretty-std.rs` is staying disabled on windows-gnu because our test harness doesn't know how to load our pretty-printers on that target: rust-lang/rust#128981 `tests/debuginfo/method-on-enum.rs` and `tests/debuginfo/option-like-enum.rs` encounter some kind of gdb bug on i686-pc-windows-gnu. I don't know enough about that situation to write a good issue. I plan on doing more work on this test suite. There's clearly a lot more basic cleanup work to do here.
…-errors Enable debuginfo tests that have been "temporarily disabled" for the past 6 years The PR history is a bit of a mess because I had to test this a lot with try-jobs, so I'll try to summarize the non-obvious changes here. A number of tests now have `min-lldb-version: 1800`. Those tests should have gotten an lldb version jump either in rust-lang/rust#124781 or long ago. Note that all such tests with that lldb version requirement do not run in Apple CI. `tests/debuginfo/drop-locations.rs` is staying disabled for now because gdb doesn't know to stop on the drop calls produced by a `}`: rust-lang/rust#128971 `tests/debuginfo/function-arg-initialization.rs` now has `-Zmir-enable-passes=-SingleUseConsts`; without that we initialize the const before the function prelude: rust-lang/rust#128945 `tests/debuginfo/by-value-non-immediate-argument.rs` fails because we don't generate a function prelude for unused non-immediate arguments, even with all optimizations disabled, and this seems to confuse debuggers on aarch64: rust-lang/rust#128973 `tests/debuginfo/pretty-std.rs` is staying disabled on windows-gnu because our test harness doesn't know how to load our pretty-printers on that target: rust-lang/rust#128981 `tests/debuginfo/method-on-enum.rs` and `tests/debuginfo/option-like-enum.rs` encounter some kind of gdb bug on i686-pc-windows-gnu. I don't know enough about that situation to write a good issue. I plan on doing more work on this test suite. There's clearly a lot more basic cleanup work to do here.
…finitely. Because constants are currently emitted *before* the prologue, leaving the debug location on the IRBuilder spills onto other instructions in the prologue and messes up both line numbers as well as the point LLVM chooses to be the prologue end. Example LLVM IR (irrelevant IR elided): Before: define internal { i64, i64 } @_ZN3tmp3Foo18var_return_opt_try17he02116165b0fc08cE(ptr align 8 %self) !dbg !347 { start: %self.dbg.spill = alloca [8 x i8], align 8 %_0 = alloca [16 x i8], align 8 %residual.dbg.spill = alloca [0 x i8], align 1 #dbg_declare(ptr %residual.dbg.spill, !353, !DIExpression(), !357) store ptr %self, ptr %self.dbg.spill, align 8, !dbg !357 #dbg_declare(ptr %self.dbg.spill, !350, !DIExpression(), !358) After: define internal { i64, i64 } @_ZN3tmp3Foo18var_return_opt_try17h00b17d08874ddd90E(ptr align 8 %self) !dbg !347 { start: %self.dbg.spill = alloca [8 x i8], align 8 %_0 = alloca [16 x i8], align 8 %residual.dbg.spill = alloca [0 x i8], align 1 #dbg_declare(ptr %residual.dbg.spill, !353, !DIExpression(), !357) store ptr %self, ptr %self.dbg.spill, align 8 #dbg_declare(ptr %self.dbg.spill, !350, !DIExpression(), !358) Note in particular how !357 from %residual.dbg.spill's dbg_declare no longer falls through onto the store to %self.dbg.spill. This fixes argument values at entry when the constant is a ZST (e.g. <Option as Try>::Residual). This fixes rust-lang#130003 (but note that it does *not* fix issues with argument values and non-ZST constants, which emit their own stores that have debug info on them, like rust-lang#128945).
Merely moving the constant initializations to after the prologue won't fix the stepping issue. We either need the ability to control which DILocations are treated as breakpoint locations by LLVM (a behavior which is currently entirely implicit and that cannot be controlled in the IR) or we would need to initialize constants at their point of use. |
@khuey I am very much a debugging/debuginfo/debugger newbie. If you have an amendment or an complete replacement for the issue wording or title (since I don't think you have permission to just modify this issue) please don't hesitate to open a new issue or tell me how to fix the wording above. I agree that what you're describing sounds a lot more like the root cause of this and other debuginfo weirdness I saw in the test suite. |
I don't have a simple suggestion on how to modify this issue because there are at least 3 potential user-visible issues here and they may all have different solutions:
|
@rustbot claim |
…-emission, r=michaelwoerister Don't leave debug locations for constants sitting on the builder indefinitely Because constants are currently emitted *before* the prologue, leaving the debug location on the IRBuilder spills onto other instructions in the prologue and messes up both line numbers as well as the point LLVM chooses to be the prologue end. Example LLVM IR (irrelevant IR elided): Before: ``` define internal { i64, i64 } `@_ZN3tmp3Foo18var_return_opt_try17he02116165b0fc08cE(ptr` align 8 %self) !dbg !347 { start: %self.dbg.spill = alloca [8 x i8], align 8 %_0 = alloca [16 x i8], align 8 %residual.dbg.spill = alloca [0 x i8], align 1 #dbg_declare(ptr %residual.dbg.spill, !353, !DIExpression(), !357) store ptr %self, ptr %self.dbg.spill, align 8, !dbg !357 #dbg_declare(ptr %self.dbg.spill, !350, !DIExpression(), !358) ``` After: ``` define internal { i64, i64 } `@_ZN3tmp3Foo18var_return_opt_try17h00b17d08874ddd90E(ptr` align 8 %self) !dbg !347 { start: %self.dbg.spill = alloca [8 x i8], align 8 %_0 = alloca [16 x i8], align 8 %residual.dbg.spill = alloca [0 x i8], align 1 #dbg_declare(ptr %residual.dbg.spill, !353, !DIExpression(), !357) store ptr %self, ptr %self.dbg.spill, align 8 #dbg_declare(ptr %self.dbg.spill, !350, !DIExpression(), !358) ``` Note in particular how !357 from %residual.dbg.spill's dbg_declare no longer falls through onto the store to %self.dbg.spill. This fixes argument values at entry when the constant is a ZST (e.g. `<Option as Try>::Residual`). This fixes rust-lang#130003 (but note that it does *not* fix issues with argument values and non-ZST constants, which emit their own stores that have debug info on them, like rust-lang#128945). r? `@michaelwoerister`
…-emission, r=michaelwoerister Don't leave debug locations for constants sitting on the builder indefinitely Because constants are currently emitted *before* the prologue, leaving the debug location on the IRBuilder spills onto other instructions in the prologue and messes up both line numbers as well as the point LLVM chooses to be the prologue end. Example LLVM IR (irrelevant IR elided): Before: ``` define internal { i64, i64 } ``@_ZN3tmp3Foo18var_return_opt_try17he02116165b0fc08cE(ptr`` align 8 %self) !dbg !347 { start: %self.dbg.spill = alloca [8 x i8], align 8 %_0 = alloca [16 x i8], align 8 %residual.dbg.spill = alloca [0 x i8], align 1 #dbg_declare(ptr %residual.dbg.spill, !353, !DIExpression(), !357) store ptr %self, ptr %self.dbg.spill, align 8, !dbg !357 #dbg_declare(ptr %self.dbg.spill, !350, !DIExpression(), !358) ``` After: ``` define internal { i64, i64 } ``@_ZN3tmp3Foo18var_return_opt_try17h00b17d08874ddd90E(ptr`` align 8 %self) !dbg !347 { start: %self.dbg.spill = alloca [8 x i8], align 8 %_0 = alloca [16 x i8], align 8 %residual.dbg.spill = alloca [0 x i8], align 1 #dbg_declare(ptr %residual.dbg.spill, !353, !DIExpression(), !357) store ptr %self, ptr %self.dbg.spill, align 8 #dbg_declare(ptr %self.dbg.spill, !350, !DIExpression(), !358) ``` Note in particular how !357 from %residual.dbg.spill's dbg_declare no longer falls through onto the store to %self.dbg.spill. This fixes argument values at entry when the constant is a ZST (e.g. `<Option as Try>::Residual`). This fixes rust-lang#130003 (but note that it does *not* fix issues with argument values and non-ZST constants, which emit their own stores that have debug info on them, like rust-lang#128945). r? ``@michaelwoerister``
…-emission, r=michaelwoerister Don't leave debug locations for constants sitting on the builder indefinitely Because constants are currently emitted *before* the prologue, leaving the debug location on the IRBuilder spills onto other instructions in the prologue and messes up both line numbers as well as the point LLVM chooses to be the prologue end. Example LLVM IR (irrelevant IR elided): Before: ``` define internal { i64, i64 } ```@_ZN3tmp3Foo18var_return_opt_try17he02116165b0fc08cE(ptr``` align 8 %self) !dbg !347 { start: %self.dbg.spill = alloca [8 x i8], align 8 %_0 = alloca [16 x i8], align 8 %residual.dbg.spill = alloca [0 x i8], align 1 #dbg_declare(ptr %residual.dbg.spill, !353, !DIExpression(), !357) store ptr %self, ptr %self.dbg.spill, align 8, !dbg !357 #dbg_declare(ptr %self.dbg.spill, !350, !DIExpression(), !358) ``` After: ``` define internal { i64, i64 } ```@_ZN3tmp3Foo18var_return_opt_try17h00b17d08874ddd90E(ptr``` align 8 %self) !dbg !347 { start: %self.dbg.spill = alloca [8 x i8], align 8 %_0 = alloca [16 x i8], align 8 %residual.dbg.spill = alloca [0 x i8], align 1 #dbg_declare(ptr %residual.dbg.spill, !353, !DIExpression(), !357) store ptr %self, ptr %self.dbg.spill, align 8 #dbg_declare(ptr %self.dbg.spill, !350, !DIExpression(), !358) ``` Note in particular how !357 from %residual.dbg.spill's dbg_declare no longer falls through onto the store to %self.dbg.spill. This fixes argument values at entry when the constant is a ZST (e.g. `<Option as Try>::Residual`). This fixes rust-lang#130003 (but note that it does *not* fix issues with argument values and non-ZST constants, which emit their own stores that have debug info on them, like rust-lang#128945). r? ```@michaelwoerister```
…mission, r=michaelwoerister Don't leave debug locations for constants sitting on the builder indefinitely Because constants are currently emitted *before* the prologue, leaving the debug location on the IRBuilder spills onto other instructions in the prologue and messes up both line numbers as well as the point LLVM chooses to be the prologue end. Example LLVM IR (irrelevant IR elided): Before: ``` define internal { i64, i64 } `@_ZN3tmp3Foo18var_return_opt_try17he02116165b0fc08cE(ptr` align 8 %self) !dbg !347 { start: %self.dbg.spill = alloca [8 x i8], align 8 %_0 = alloca [16 x i8], align 8 %residual.dbg.spill = alloca [0 x i8], align 1 #dbg_declare(ptr %residual.dbg.spill, !353, !DIExpression(), !357) store ptr %self, ptr %self.dbg.spill, align 8, !dbg !357 #dbg_declare(ptr %self.dbg.spill, !350, !DIExpression(), !358) ``` After: ``` define internal { i64, i64 } `@_ZN3tmp3Foo18var_return_opt_try17h00b17d08874ddd90E(ptr` align 8 %self) !dbg !347 { start: %self.dbg.spill = alloca [8 x i8], align 8 %_0 = alloca [16 x i8], align 8 %residual.dbg.spill = alloca [0 x i8], align 1 #dbg_declare(ptr %residual.dbg.spill, !353, !DIExpression(), !357) store ptr %self, ptr %self.dbg.spill, align 8 #dbg_declare(ptr %self.dbg.spill, !350, !DIExpression(), !358) ``` Note in particular how !357 from %residual.dbg.spill's dbg_declare no longer falls through onto the store to %self.dbg.spill. This fixes argument values at entry when the constant is a ZST (e.g. `<Option as Try>::Residual`). This fixes rust-lang#130003 (but note that it does *not* fix issues with argument values and non-ZST constants, which emit their own stores that have debug info on them, like rust-lang#128945). r? `@michaelwoerister`
Currently constants are "pulled forward" and have their stack spills emitted first. This confuses LLVM as to where to place breakpoints at function entry, and results in argument values being wrong in the debugger. It's straightforward to avoid emitting the stack spills for constants until arguments/etc have been introduced in debug_introduce_locals, so do that. Example LLVM IR (irrelevant IR elided): Before: define internal void @_ZN11rust_1289457binding17h2c78f956ba4bd2c3E(i64 %a, i64 %b, double %c) unnamed_addr #0 !dbg !178 { start: %c.dbg.spill = alloca [8 x i8], align 8 %b.dbg.spill = alloca [8 x i8], align 8 %a.dbg.spill = alloca [8 x i8], align 8 %x.dbg.spill = alloca [4 x i8], align 4 store i32 0, ptr %x.dbg.spill, align 4, !dbg !192 ; LLVM places breakpoint here. #dbg_declare(ptr %x.dbg.spill, !190, !DIExpression(), !192) store i64 %a, ptr %a.dbg.spill, align 8 #dbg_declare(ptr %a.dbg.spill, !187, !DIExpression(), !193) store i64 %b, ptr %b.dbg.spill, align 8 #dbg_declare(ptr %b.dbg.spill, !188, !DIExpression(), !194) store double %c, ptr %c.dbg.spill, align 8 #dbg_declare(ptr %c.dbg.spill, !189, !DIExpression(), !195) ret void, !dbg !196 } After: define internal void @_ZN11rust_1289457binding17h2c78f956ba4bd2c3E(i64 %a, i64 %b, double %c) unnamed_addr #0 !dbg !178 { start: %x.dbg.spill = alloca [4 x i8], align 4 %c.dbg.spill = alloca [8 x i8], align 8 %b.dbg.spill = alloca [8 x i8], align 8 %a.dbg.spill = alloca [8 x i8], align 8 store i64 %a, ptr %a.dbg.spill, align 8 #dbg_declare(ptr %a.dbg.spill, !187, !DIExpression(), !192) store i64 %b, ptr %b.dbg.spill, align 8 #dbg_declare(ptr %b.dbg.spill, !188, !DIExpression(), !193) store double %c, ptr %c.dbg.spill, align 8 #dbg_declare(ptr %c.dbg.spill, !189, !DIExpression(), !194) store i32 0, ptr %x.dbg.spill, align 4, !dbg !195 ; LLVM places breakpoint here. #dbg_declare(ptr %x.dbg.spill, !190, !DIExpression(), !195) ret void, !dbg !196 } Note in particular the position of the "LLVM places breakpoint here" comment relative to the stack spills for the function arguments. LLVM assumes that the first instruction with with a debug location is the end of the prologue. As LLVM does not currently offer front ends any direct control over the placement of the prologue end reordering the IR is the only mechanism available to fix argument values at function entry in the presence of MIR optimizations like SingleUseConsts. Fixes rust-lang#128945
…avidtwco Reorder stack spills so that constants come later. Currently constants are "pulled forward" and have their stack spills emitted first. This confuses LLVM as to where to place breakpoints at function entry, and results in argument values being wrong in the debugger. It's straightforward to avoid emitting the stack spills for constants until arguments/etc have been introduced in debug_introduce_locals, so do that. Example LLVM IR (irrelevant IR elided): Before: ``` define internal void `@_ZN11rust_1289457binding17h2c78f956ba4bd2c3E(i64` %a, i64 %b, double %c) unnamed_addr #0 !dbg !178 { start: %c.dbg.spill = alloca [8 x i8], align 8 %b.dbg.spill = alloca [8 x i8], align 8 %a.dbg.spill = alloca [8 x i8], align 8 %x.dbg.spill = alloca [4 x i8], align 4 store i32 0, ptr %x.dbg.spill, align 4, !dbg !192 ; LLVM places breakpoint here. #dbg_declare(ptr %x.dbg.spill, !190, !DIExpression(), !192) store i64 %a, ptr %a.dbg.spill, align 8 #dbg_declare(ptr %a.dbg.spill, !187, !DIExpression(), !193) store i64 %b, ptr %b.dbg.spill, align 8 #dbg_declare(ptr %b.dbg.spill, !188, !DIExpression(), !194) store double %c, ptr %c.dbg.spill, align 8 #dbg_declare(ptr %c.dbg.spill, !189, !DIExpression(), !195) ret void, !dbg !196 } ``` After: ``` define internal void `@_ZN11rust_1289457binding17h2c78f956ba4bd2c3E(i64` %a, i64 %b, double %c) unnamed_addr #0 !dbg !178 { start: %x.dbg.spill = alloca [4 x i8], align 4 %c.dbg.spill = alloca [8 x i8], align 8 %b.dbg.spill = alloca [8 x i8], align 8 %a.dbg.spill = alloca [8 x i8], align 8 store i64 %a, ptr %a.dbg.spill, align 8 #dbg_declare(ptr %a.dbg.spill, !187, !DIExpression(), !192) store i64 %b, ptr %b.dbg.spill, align 8 #dbg_declare(ptr %b.dbg.spill, !188, !DIExpression(), !193) store double %c, ptr %c.dbg.spill, align 8 #dbg_declare(ptr %c.dbg.spill, !189, !DIExpression(), !194) store i32 0, ptr %x.dbg.spill, align 4, !dbg !195 ; LLVM places breakpoint here. #dbg_declare(ptr %x.dbg.spill, !190, !DIExpression(), !195) ret void, !dbg !196 } ``` Note in particular the position of the "LLVM places breakpoint here" comment relative to the stack spills for the function arguments. LLVM assumes that the first instruction with with a debug location is the end of the prologue. As LLVM does not currently offer front ends any direct control over the placement of the prologue end reordering the IR is the only mechanism available to fix argument values at function entry in the presence of MIR optimizations like SingleUseConsts. Fixes rust-lang#128945 r? `@michaelwoerister`
…idtwco Reorder stack spills so that constants come later. Currently constants are "pulled forward" and have their stack spills emitted first. This confuses LLVM as to where to place breakpoints at function entry, and results in argument values being wrong in the debugger. It's straightforward to avoid emitting the stack spills for constants until arguments/etc have been introduced in debug_introduce_locals, so do that. Example LLVM IR (irrelevant IR elided): Before: ``` define internal void `@_ZN11rust_1289457binding17h2c78f956ba4bd2c3E(i64` %a, i64 %b, double %c) unnamed_addr #0 !dbg !178 { start: %c.dbg.spill = alloca [8 x i8], align 8 %b.dbg.spill = alloca [8 x i8], align 8 %a.dbg.spill = alloca [8 x i8], align 8 %x.dbg.spill = alloca [4 x i8], align 4 store i32 0, ptr %x.dbg.spill, align 4, !dbg !192 ; LLVM places breakpoint here. #dbg_declare(ptr %x.dbg.spill, !190, !DIExpression(), !192) store i64 %a, ptr %a.dbg.spill, align 8 #dbg_declare(ptr %a.dbg.spill, !187, !DIExpression(), !193) store i64 %b, ptr %b.dbg.spill, align 8 #dbg_declare(ptr %b.dbg.spill, !188, !DIExpression(), !194) store double %c, ptr %c.dbg.spill, align 8 #dbg_declare(ptr %c.dbg.spill, !189, !DIExpression(), !195) ret void, !dbg !196 } ``` After: ``` define internal void `@_ZN11rust_1289457binding17h2c78f956ba4bd2c3E(i64` %a, i64 %b, double %c) unnamed_addr #0 !dbg !178 { start: %x.dbg.spill = alloca [4 x i8], align 4 %c.dbg.spill = alloca [8 x i8], align 8 %b.dbg.spill = alloca [8 x i8], align 8 %a.dbg.spill = alloca [8 x i8], align 8 store i64 %a, ptr %a.dbg.spill, align 8 #dbg_declare(ptr %a.dbg.spill, !187, !DIExpression(), !192) store i64 %b, ptr %b.dbg.spill, align 8 #dbg_declare(ptr %b.dbg.spill, !188, !DIExpression(), !193) store double %c, ptr %c.dbg.spill, align 8 #dbg_declare(ptr %c.dbg.spill, !189, !DIExpression(), !194) store i32 0, ptr %x.dbg.spill, align 4, !dbg !195 ; LLVM places breakpoint here. #dbg_declare(ptr %x.dbg.spill, !190, !DIExpression(), !195) ret void, !dbg !196 } ``` Note in particular the position of the "LLVM places breakpoint here" comment relative to the stack spills for the function arguments. LLVM assumes that the first instruction with with a debug location is the end of the prologue. As LLVM does not currently offer front ends any direct control over the placement of the prologue end reordering the IR is the only mechanism available to fix argument values at function entry in the presence of MIR optimizations like SingleUseConsts. Fixes rust-lang#128945 r? `@michaelwoerister`
…finitely. Because constants are currently emitted *before* the prologue, leaving the debug location on the IRBuilder spills onto other instructions in the prologue and messes up both line numbers as well as the point LLVM chooses to be the prologue end. Example LLVM IR (irrelevant IR elided): Before: define internal { i64, i64 } @_ZN3tmp3Foo18var_return_opt_try17he02116165b0fc08cE(ptr align 8 %self) !dbg !347 { start: %self.dbg.spill = alloca [8 x i8], align 8 %_0 = alloca [16 x i8], align 8 %residual.dbg.spill = alloca [0 x i8], align 1 #dbg_declare(ptr %residual.dbg.spill, !353, !DIExpression(), !357) store ptr %self, ptr %self.dbg.spill, align 8, !dbg !357 #dbg_declare(ptr %self.dbg.spill, !350, !DIExpression(), !358) After: define internal { i64, i64 } @_ZN3tmp3Foo18var_return_opt_try17h00b17d08874ddd90E(ptr align 8 %self) !dbg !347 { start: %self.dbg.spill = alloca [8 x i8], align 8 %_0 = alloca [16 x i8], align 8 %residual.dbg.spill = alloca [0 x i8], align 1 #dbg_declare(ptr %residual.dbg.spill, !353, !DIExpression(), !357) store ptr %self, ptr %self.dbg.spill, align 8 #dbg_declare(ptr %self.dbg.spill, !350, !DIExpression(), !358) Note in particular how !357 from %residual.dbg.spill's dbg_declare no longer falls through onto the store to %self.dbg.spill. This fixes argument values at entry when the constant is a ZST (e.g. <Option as Try>::Residual). This fixes rust-lang#130003 (but note that it does *not* fix issues with argument values and non-ZST constants, which emit their own stores that have debug info on them, like rust-lang#128945).
I ran into this as part of #128913
Compile this example program extracted from
tests/debuginfo/function-arg-initialization.rs
with
rustc test.rs -g
then load it into gdb withgdb test
and behold:The bug here is that a breakpoint set inside a function can land on an instruction before the function prelude.
As far as I can tell, this is only possible with single-use-consts. You can get the right codegen in current rustc by setting
-Zmir-enable-passes=-SingleUseConsts
. But I think blaming that MIR pass would be mistake because that MIR pass is just a reimplementation of the same concept that used to be calledConstDebugInfo
.Naive attempts to bisect this using the breakpoint method above will all land on #107404. But I think that is a wrong bisection, because that PR just fixed an existing MIR pass so that we produce the right span information in our debuginfo.
I think the root cause or the PR that first introduced the bug is #73210. You can see in this godbolt demo that rustc 1.50.0 is the version where we start initializing
x
before the function prelude: https://godbolt.org/z/r5W3M4sG9. It's likely that nobody noticed or appreciated how codegen has changed because the debuginfo test for this scenario had been disabled.This problem also afflicts
tests/debuginfo/macro-stepping.rs
, because its strange combination of single-use-consts on the same line as function calls makes stepping through the function hit all those lines before the prelude when the consts are initialized, then again when the functions are called.The text was updated successfully, but these errors were encountered: