Skip to content
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

Fix debuginfo for pointers/references to unsized types #93006

Merged
merged 6 commits into from
Jan 28, 2022

Conversation

michaelwoerister
Copy link
Member

@michaelwoerister michaelwoerister commented Jan 17, 2022

This PR makes the compiler emit fat pointer debuginfo in all cases. Before, we sometimes got thin-pointer debuginfo, making it impossible to fully interpret the pointed to memory in debuggers. The code is actually cleaner now, especially around generation of trait object pointer debuginfo.

Fixes #92718

Blocked on #92729.

@michaelwoerister michaelwoerister added A-debuginfo Area: Debugging information in compiled programs (DWARF, PDB, etc.) S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. labels Jan 17, 2022
@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jan 17, 2022
@rust-highfive
Copy link
Collaborator

r? @oli-obk

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 17, 2022
@michaelwoerister michaelwoerister force-pushed the fix-unsized-ptr-debuginfo branch 2 times, most recently from f57d9de to 3da8b15 Compare January 17, 2022 17:00
@bors
Copy link
Contributor

bors commented Jan 18, 2022

☔ The latest upstream changes (presumably #93009) made this pull request unmergeable. Please resolve the merge conflicts.

@michaelwoerister michaelwoerister removed the S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. label Jan 18, 2022
@michaelwoerister
Copy link
Member Author

This PR makes type names, that are part of a pointer type, fully qualified. For example, &Foo<u8> becomes &my_crate::some_mod::Foo<u8>. This is the correct behavior, but it occurs to me that that might have performance implications.

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jan 19, 2022
@bors
Copy link
Contributor

bors commented Jan 19, 2022

⌛ Trying commit 756a70c with merge 5d8768a3a4a4ad49d5b52f745a6a48bda4ff3bf0...

@bors
Copy link
Contributor

bors commented Jan 19, 2022

☀️ Try build successful - checks-actions
Build commit: 5d8768a3a4a4ad49d5b52f745a6a48bda4ff3bf0 (5d8768a3a4a4ad49d5b52f745a6a48bda4ff3bf0)

@rust-timer
Copy link
Collaborator

Queued 5d8768a3a4a4ad49d5b52f745a6a48bda4ff3bf0 with parent 2f004d2, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (5d8768a3a4a4ad49d5b52f745a6a48bda4ff3bf0): comparison url.

Summary: This benchmark run did not return any relevant changes.

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR led to changes in compiler perf.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf -perf-regression

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jan 19, 2022
@oli-obk
Copy link
Contributor

oli-obk commented Jan 19, 2022

This lgtm from a pure impl perspective, but I think someone who knows llvm should look over it

r? rust-lang/wg-llvm

@oli-obk
Copy link
Contributor

oli-obk commented Jan 19, 2022

r? @rust-lang/wg-llvm

@michaelwoerister
Copy link
Member Author

There doesn't seem to be an LLVM group in https://github.com/rust-lang/highfive/blob/master/highfive/configs/rust-lang/rust.json yet.

r? @davidtwco, who knows debuginfo well (but feel free to re-assign if you don't want to review!)

@rust-highfive rust-highfive assigned davidtwco and unassigned oli-obk Jan 20, 2022
Copy link
Member

@davidtwco davidtwco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me, two small nits but otherwise I think this is good.

compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs Outdated Show resolved Hide resolved
compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs Outdated Show resolved Hide resolved
@michaelwoerister
Copy link
Member Author

Thanks for the reviews! Comments should be addressed now.

@davidtwco

This comment has been minimized.

@bors

This comment has been minimized.

@bors bors added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 27, 2022
…ge doc comment so it is not interpreted as doc-test.
@michaelwoerister
Copy link
Member Author

Fix doc-comment so it's not interpreted as doctest.

@bors r=davidtwco,oli-obk

@bors
Copy link
Contributor

bors commented Jan 27, 2022

📌 Commit d33e317 has been approved by davidtwco,oli-obk

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 27, 2022
@bors
Copy link
Contributor

bors commented Jan 28, 2022

⌛ Testing commit d33e317 with merge bcca153730480e04fc3eae2166fc3b6c07942047...

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Jan 28, 2022

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 28, 2022
…t mark fat pointer fields as artificial.

LLDB does not seem to see fields if they are marked with DW_AT_artificial
which breaks pretty printers that use these fields for decoding fat pointers.
@rust-log-analyzer
Copy link
Collaborator

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

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

running 165 tests
...................................i........

command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/compiletest" "--compile-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib" "--run-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/i686-unknown-linux-gnu/lib" "--rustc-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "--src-base" "/checkout/src/test/mir-opt" "--build-base" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/mir-opt" "--stage-id" "stage2-i686-unknown-linux-gnu" "--suite" "mir-opt" "--mode" "mir-opt" "--target" "i686-unknown-linux-gnu" "--host" "x86_64-unknown-linux-gnu" "--llvm-filecheck" "/usr/lib/llvm-12/bin/FileCheck" "--nodejs" "/usr/bin/node" "--linker" "cc" "--host-rustcflags" "-Crpath -O -Cdebuginfo=0  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--target-rustcflags" "-Crpath -O -Cdebuginfo=0  -Lnative=/checkout/obj/build/i686-unknown-linux-gnu/native/rust-test-helpers" "--docck-python" "/usr/bin/python3" "--lldb-python" "/usr/bin/python3" "--gdb" "/usr/bin/gdb" "--quiet" "--llvm-version" "12.0.0" "--llvm-components" "aarch64 aarch64asmparser aarch64codegen aarch64desc aarch64disassembler aarch64info aarch64utils aggressiveinstcombine all all-targets amdgpu amdgpuasmparser amdgpucodegen amdgpudesc amdgpudisassembler amdgpuinfo amdgpuutils analysis arm armasmparser armcodegen armdesc armdisassembler arminfo armutils asmparser asmprinter avr avrasmparser avrcodegen avrdesc avrdisassembler avrinfo binaryformat bitreader bitstreamreader bitwriter bpf bpfasmparser bpfcodegen bpfdesc bpfdisassembler bpfinfo cfguard codegen core coroutines coverage debuginfocodeview debuginfodwarf debuginfogsym debuginfomsf debuginfopdb demangle dlltooldriver dwarflinker engine executionengine extensions filecheck frontendopenacc frontendopenmp fuzzmutate globalisel hellonew hexagon hexagonasmparser hexagoncodegen hexagondesc hexagondisassembler hexagoninfo instcombine instrumentation interfacestub interpreter ipo irreader jitlink lanai lanaiasmparser lanaicodegen lanaidesc lanaidisassembler lanaiinfo libdriver lineeditor linker lto mc mca mcdisassembler mcjit mcparser mips mipsasmparser mipscodegen mipsdesc mipsdisassembler mipsinfo mirparser msp430 msp430asmparser msp430codegen msp430desc msp430disassembler msp430info native nativecodegen nvptx nvptxcodegen nvptxdesc nvptxinfo objcarcopts object objectyaml option orcjit orcshared orctargetprocess passes perfjitevents powerpc powerpcasmparser powerpccodegen powerpcdesc powerpcdisassembler powerpcinfo profiledata remarks riscv riscvasmparser riscvcodegen riscvdesc riscvdisassembler riscvinfo runtimedyld scalaropts selectiondag sparc sparcasmparser sparccodegen sparcdesc sparcdisassembler sparcinfo support symbolize systemz systemzasmparser systemzcodegen systemzdesc systemzdisassembler systemzinfo tablegen target textapi transformutils vectorize webassembly webassemblyasmparser webassemblycodegen webassemblydesc webassemblydisassembler webassemblyinfo windowsmanifest x86 x86asmparser x86codegen x86desc x86disassembler x86info xcore xcorecodegen xcoredesc xcoredisassembler xcoreinfo xray" "--cc" "" "--cxx" "" "--cflags" "" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" "" "--channel" "nightly" "--color" "always"
expected success, got: signal: 11 (core dumped)

Build completed unsuccessfully in 0:01:48

@michaelwoerister
Copy link
Member Author

@rust-lang/infra This is the second time in this PR that there's a core dump in compiletest after mir-opt testing: https://github.com/rust-lang/rust/runs/4978717078?check_suite_focus=true#step:26:3567
I was not able to reproduce the crash locally. Is this a known issue?

@michaelwoerister
Copy link
Member Author

I was able to reproduce the LLDB test failures on Linux. The last commit removes the DW_AT_artifical attribute from fat pointer fields (as was the case before this PR). That allows LLDB to see the fields again.

@bors r=davidtwco,oli-obk

@bors
Copy link
Contributor

bors commented Jan 28, 2022

📌 Commit c10f9e7 has been approved by davidtwco,oli-obk

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 28, 2022
@bors
Copy link
Contributor

bors commented Jan 28, 2022

⌛ Testing commit c10f9e7 with merge 427eba2...

@bors
Copy link
Contributor

bors commented Jan 28, 2022

☀️ Test successful - checks-actions
Approved by: davidtwco,oli-obk
Pushing 427eba2 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jan 28, 2022
@bors bors merged commit 427eba2 into rust-lang:master Jan 28, 2022
@rustbot rustbot added this to the 1.60.0 milestone Jan 28, 2022
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (427eba2): comparison url.

Summary: This benchmark run did not return any relevant results.

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-debuginfo Area: Debugging information in compiled programs (DWARF, PDB, etc.) merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incomplete/wrong debuginfo for references and pointers to slice-like unsized types
10 participants