-
Notifications
You must be signed in to change notification settings - Fork 13.6k
Preserve the .debug_gdb_scripts section #143679
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
Preserve the .debug_gdb_scripts section #143679
Conversation
Some changes occurred in compiler/rustc_codegen_ssa Some changes occurred in compiler/rustc_codegen_gcc |
97ac05e
to
07de96b
Compare
07de96b
to
d54c3ea
Compare
@bors r+ |
…b-scripts-section, r=bjorn3 Preserve the .debug_gdb_scripts section Make sure that compiler and linker don't optimize the section's contents away by adding the global holding the data to `llvm.used`. This eliminates the need for a volatile load in the main shim; since the LLVM codegen backend is the only implementer of the corresponding trait function, remove it entirely. r? `@bjorn3`
…b-scripts-section, r=bjorn3 Preserve the .debug_gdb_scripts section Make sure that compiler and linker don't optimize the section's contents away by adding the global holding the data to `llvm.used`. This eliminates the need for a volatile load in the main shim; since the LLVM codegen backend is the only implementer of the corresponding trait function, remove it entirely. r? ``@bjorn3``
Rollup of 8 pull requests Successful merges: - #140136 (Add an aarch64-msvc build running on ARM64 Windows) - #143642 (stdarch subtree update) - #143679 (Preserve the .debug_gdb_scripts section) - #143707 (Fix `--skip-std-check-if-no-download-rustc`) - #143722 (Make some "safe" llvm ops actually sound) - #143728 (Resolve refactor: extraction of `finalize_module_binding` and `single_import_can_define_name`) - #143742 (Rework borrowing suggestions to use `Expr` instead of just `Span`) - #143744 (Properly track the depth when expanding free alias types) r? `@ghost` `@rustbot` modify labels: rollup
Looks like some GDB tests failed #143755 (comment) |
…ection, r=<try> Preserve the .debug_gdb_scripts section Make sure that compiler and linker don't optimize the section's contents away by adding the global holding the data to `llvm.used`. This eliminates the need for a volatile load in the main shim; since the LLVM codegen backend is the only implementer of the corresponding trait function, remove it entirely. r? `@bjorn3` try-job: dist-i586-gnu-i586-i686-musl
💔 Test failed
|
Interesting, I'm going to have a look. The test passes on my |
You might be able to get away with passing If that doesn't repro it for some reason you can replicate the CI setup pretty much exactly by running the docker tests https://rustc-dev-guide.rust-lang.org/tests/docker.html, with the downside that this takes about as long as CI does. |
Hmm, the full log of the CI job led me to believe that it's I noticed though that my GDB is version 15, whereas the test log shows GDB 12; maybe that's the reason for the discrepancy. In general, the |
You're right, it's i586. I must have been reading the wrong line.
I assume there probably isn't any reason we need to keep a very old gdb version if that makes a difference |
…ection, r=bjorn3 Preserve the .debug_gdb_scripts section Make sure that compiler and linker don't optimize the section's contents away by adding the global holding the data to `llvm.used`. This eliminates the need for a volatile load in the main shim; since the LLVM codegen backend is the only implementer of the corresponding trait function, remove it entirely. Pretty printers in dylib dependencies are now emitted by the main crate instead of the dylib; apart from matching how rlibs are handled, this approach has the advantage that `omit_gdb_pretty_printer_section` keeps working with dylib dependencies. r? `@bjorn3`
This comment has been minimized.
This comment has been minimized.
💔 Test failed - checks-actions |
There's a problem with old linkers again: We're emitting the volatile load in the main shim, but pretty printers are now collected per crate, meaning that the load only preserves the main crate's pretty printers. As far as I can see, this affects only a single test (which uses pretty printers from an rlib). Maybe it's okay to disable it, expecting that people generally have more recent linkers than the CI setup? |
Instead of collecting pretty printers transitively when building executables/staticlibs/cdylibs, let the debugger find each crate's pretty printers via its .debug_gdb_scripts section. This covers the case where libraries defining custom pretty printers are loaded dynamically.
cc2d4b0
to
b4d923c
Compare
@bors r+ |
…ection, r=bjorn3 Preserve the .debug_gdb_scripts section Make sure that compiler and linker don't optimize the section's contents away by adding the global holding the data to `llvm.used`. This eliminates the need for a volatile load in the main shim; since the LLVM codegen backend is the only implementer of the corresponding trait function, remove it entirely. Pretty printers in dylib dependencies are now emitted by the main crate instead of the dylib; apart from matching how rlibs are handled, this approach has the advantage that `omit_gdb_pretty_printer_section` keeps working with dylib dependencies. r? `@bjorn3`
…b-scripts-section, r=bjorn3 Preserve the .debug_gdb_scripts section Make sure that compiler and linker don't optimize the section's contents away by adding the global holding the data to `llvm.used`. This eliminates the need for a volatile load in the main shim; since the LLVM codegen backend is the only implementer of the corresponding trait function, remove it entirely. Pretty printers in dylib dependencies are now emitted by the main crate instead of the dylib; apart from matching how rlibs are handled, this approach has the advantage that `omit_gdb_pretty_printer_section` keeps working with dylib dependencies. r? `@bjorn3`
Yielding to enclosing rollup: @bors retry |
Rollup of 21 pull requests Successful merges: - #137831 (Tweak auto trait errors) - #138689 (add nvptx_target_feature) - #140267 (implement continue_ok and break_ok for ControlFlow) - #143679 (Preserve the .debug_gdb_scripts section) - #143857 (Port #[macro_export] to the new attribute parsing infrastructure) - #143929 (Mark all deprecation lints in name resolution as deny-by-default and report-in-deps) - #144133 (Stabilize const TypeId::of) - #144369 (Upgrade semicolon_in_expressions_from_macros from warn to deny) - #144473 (Address libunwind.a inconsistency issues in the bootstrap program) - #144498 (Add --print target-spec-json-schema) - #144552 (Rehome 33 `tests/ui/issues/` tests to other subdirectories under `tests/ui/`) - #144659 (bootstrap: refactor mingw dist and fix gnullvm) - #144676 (Add documentation for unstable_feature_bound) - #144794 (Port `#[coroutine]` to the new attribute system) - #144835 (Anonymize binders in tail call sig) - #144836 (Change visibility of Args new function) - #144861 (Stabilize `panic_payload_as_str` feature) - #144910 (Add regression tests for seemingly fixed issues) - #144913 ([rustdoc] Fix wrong `i` tooltip icon) - #144917 (Enforce tail call type is related to body return type in borrowck) - #144924 (compiletest: add hint for when a ui test produces no errors) r? `@ghost` `@rustbot` modify labels: rollup
☀️ Test successful - checks-actions |
What is this?This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.Comparing dc0bae1 (parent) -> 8fb40f7 (this PR) Test differencesShow 2 test diffsStage 2
Job group index Test dashboardRun cargo run --manifest-path src/ci/citool/Cargo.toml -- \
test-dashboard 8fb40f798a23adf608182ce5f4eb151fdc8e0da5 --output-dir test-dashboard And then open Job duration changes
How to interpret the job duration changes?Job durations can vary a lot, based on the actual runner instance |
Finished benchmarking commit (8fb40f7): comparison URL. Overall result: ❌✅ regressions and improvements - please read the text belowOur benchmarks found a performance regression caused by this PR. Next Steps:
@rustbot label: +perf-regression Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary 2.6%, secondary 3.0%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary 99.0%, secondary 0.9%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeResults (primary 0.2%, secondary 0.3%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 463.057s -> 464.179s (0.24%) |
Opened #145014 to temporarily revert this PR. |
I think the reason for the perf regression is that something is causing codegen units to be codegened again when that previously didn't happen. I see that the |
Make sure that compiler and linker don't optimize the section's contents
away by adding the global holding the data to
llvm.used
. Thiseliminates the need for a volatile load in the main shim; since the LLVM
codegen backend is the only implementer of the corresponding trait
function, remove it entirely.
Pretty printers in dylib dependencies are now emitted by the main crate
instead of the dylib; apart from matching how rlibs are handled, this
approach has the advantage that
omit_gdb_pretty_printer_section
keepsworking with dylib dependencies.
r? @bjorn3