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.

Sign up for GitHub

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

Jump to bottom

Set up standard library path substitution in rust-gdb and gdbgui #109527

Merged
merged 1 commit into from
Apr 11, 2023
Merged

Set up standard library path substitution in rust-gdb and gdbgui #109527

merged 1 commit into from
Apr 11, 2023

Conversation

lnicola
Copy link
Member

@lnicola lnicola commented Mar 23, 2023 •
edited
Loading

Fixes #62945

Only lightly tested (in release mode, where the paths are a bit of a mess) because my gdb appears to crash with internal-error: inside_main_func: Assertion 'block != nullptr' failed. and I don't have gdbgui. Please review carefully my shell syntax.

There's also rust-lldb, but I don't know the equivalent for it.

@rustbot
Copy link
Collaborator

rustbot commented Mar 23, 2023

r? @Mark-Simulacrum

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 23, 2023
@mpaulucci mpaulucci mentioned this pull request Mar 23, 2023
Merged
@cuviper
Copy link
Member

cuviper commented Mar 23, 2023

FWIW, I have something very similar in Fedora - a general patch and a sed to fill in the real build dir:
https://src.fedoraproject.org/rpms/rust/blob/rawhide/f/rustc-1.61.0-rust-gdb-substitute-path.patch
https://src.fedoraproject.org/rpms/rust/blob/rawhide/f/rust.spec#_602

Your /rustc/$RUSTC_COMMIT_HASH is correct for CI builds, and I'll be able to drop my patch and sed my replacement on that instead... but it sure would be nice if we could make that more general to automatically solve both of our cases. Maybe a new option like rustc --sysroot or --print ... for the build path?

Mark-Simulacrum
Mark-Simulacrum reviewed Mar 26, 2023
View reviewed changes
src/etc/rust-gdb
@@ -21,4 +23,6 @@ RUST_GDB="${RUST_GDB:-gdb}"
PYTHONPATH="$PYTHONPATH:$GDB_PYTHON_MODULE_DIRECTORY" exec ${RUST_GDB} \
--directory="$GDB_PYTHON_MODULE_DIRECTORY" \
-iex "add-auto-load-safe-path $GDB_PYTHON_MODULE_DIRECTORY" \
-iex "set substitute-path /rustc/$RUSTC_COMMIT_HASH $RUSTC_SYSROOT/lib/rustlib/src/rust" \
Copy link
Member

Choose a reason for hiding this comment

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

This seems reasonable(ish), but I'm wondering whether this is a problem if the destination does not exist. Maybe it's clearer to see the hash path? Not sure.

Copy link
Member

Choose a reason for hiding this comment

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

IME, gdb only uses the substitution for reading the source file, but still displays the original path in backtraces and such. Even the TUI layout shows the source window labeled with the original path.

Copy link
Member Author

Choose a reason for hiding this comment

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

IIUC, there's no issue if a path doesn't exist, the substitutions just don't work.

@Mark-Simulacrum
Copy link
Member

r? @cuviper - I'm OK r+'ing this but not sure if you wanted to block on your comment

@rustbot rustbot assigned cuviper and unassigned Mark-Simulacrum Apr 2, 2023
@lnicola
Copy link
Member Author

lnicola commented Apr 2, 2023 •
edited
Loading

Sorrry for dropping the ball on this. I can try to add a new --print flag (assuming I figure out how), it sounds reasonable. Will that have to go through a FCP?

@cuviper
Copy link
Member

cuviper commented Apr 10, 2023

I do still think a --print flag would be a good idea, but it doesn't need to block here. A compiler MCP should be relatively lightweight for proposing that.

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Apr 10, 2023

📌 Commit 8dd0ec6 has been approved by cuviper

It is now in the queue for this repository.

@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 Apr 10, 2023
@JohnTitor JohnTitor mentioned this pull request Apr 11, 2023
Closed
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Apr 11, 2023
@JohnTitor
Rollup merge of rust-lang#109527 - lnicola:rust-gdb-substitute-path, …
0d2de94 
…r=cuviper

Set up standard library path substitution in rust-gdb and gdbgui

Fixes rust-lang#62945

---

Only lightly tested (in release mode, where the paths are a bit of a mess) because my `gdb` appears to crash with `internal-error: inside_main_func: Assertion 'block != nullptr' failed.` and I don't have `gdbgui`. Please review carefully my shell syntax.

There's also `rust-lldb`, but I don't know the equivalent for it.
@JohnTitor JohnTitor mentioned this pull request Apr 11, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 11, 2023
@bors
Auto merge of rust-lang#110170 - JohnTitor:rollup-hdramer, r=JohnTitor
5072826 
Rollup of 8 pull requests

Successful merges:

 - rust-lang#109527 (Set up standard library path substitution in rust-gdb and gdbgui)
 - rust-lang#109752 (Stall auto trait assembly in new solver for int/float vars)
 - rust-lang#109860 (Add support for RISC-V relax target feature)
 - rust-lang#109923 (Update `error [E0449]: unnecessary visibility qualifier` to be more clear)
 - rust-lang#110070 (The `wrapping_neg` example for unsigned types shouldn't use `i8`)
 - rust-lang#110146 (fix(doc): do not parse inline when output is json for external crate)
 - rust-lang#110147 (Add regression test for rust-lang#104916)
 - rust-lang#110149 (Update books)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit c617ddf into rust-lang:master Apr 11, 2023
@rustbot rustbot added this to the 1.70.0 milestone Apr 11, 2023
@lnicola lnicola deleted the rust-gdb-substitute-path branch April 12, 2023 07:25
@lnicola lnicola mentioned this pull request Apr 12, 2023
Closed
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cuviper cuviper cuviper left review comments

@Mark-Simulacrum Mark-Simulacrum Mark-Simulacrum left review comments

Assignees

@cuviper cuviper

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.70.0
Development

Successfully merging this pull request may close these issues.

rust-gdb doesn't set up path substitutions?
5 participants
@lnicola @rustbot @cuviper @Mark-Simulacrum @bors