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 GDB pretty-printer for tuples and pointers #42278

Merged
merged 8 commits into from
Jun 9, 2017

Conversation

gentoo90
Copy link
Contributor

Names of children should not be the same, because GDB uses them to distinguish the children.

Before After
tuples_before tuples_after

main.rs

enum Test {
    Zero,
    One(i32),
    Two(i32, String),
    Three(i32, String, Vec<String>),
}

fn main() {
    let tuple = (1, 2, "Asdfgh");
    let zero = Test::Zero;
    let one = Test::One(10);
    let two = Test::Two(42, "Qwerty".to_owned());
    let three = Test::Three(9000,
                            "Zxcvbn".to_owned(),
                            vec!["lorem".to_owned(), "ipsum".to_owned(), "dolor".to_owned()]);
    println!(""); // breakpoint here
}

launch.json

{
    "version": "0.2.0",
    "configurations": [
        {
            "type": "gdb",
            "request": "launch",
            "gdbpath": "rust-gdb",
            "name": "Launch Program",
            "valuesFormatting": "prettyPrinters", //this requires plugin Native Debug >= 0.20.0
            "target": "./target/debug/test_pretty_printers",
            "cwd": "${workspaceRoot}"
        }
    ]
}

Names of children should not be the same,
because GDB uses them to distinguish the children.
@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@carols10cents
Copy link
Member

Thanks for the PR! We’ll periodically check in on it to make sure that @alexcrichton or someone else from the team reviews it soon.

@carols10cents carols10cents added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 29, 2017
Some pointers values include additional info,
so they can't be parsed with int().
@gentoo90 gentoo90 force-pushed the gdb-pretty-printers branch from 8de9edb to 167e4b0 Compare May 30, 2017 08:17
@gentoo90 gentoo90 changed the title Fix GDB pretty-printer for tuples Fix GDB pretty-printer for tuples and pointers May 30, 2017
@gentoo90
Copy link
Contributor Author

gentoo90 commented May 30, 2017

Fixed exception in pointers parsing

Before After
ptr_before ptr_after

Code:

fn main() {
    let opt_s = Some("AAAA");
    let opt_i = Some(42);
    println!(""); // breakpoint here
}

@alexcrichton
Copy link
Member

r? @michaelwoerister

( knows more about debuginfo and pretty printers than I )

This looks great though!

@gentoo90
Copy link
Contributor Author

str() failed on things like let opt_s = Some("I \u{2665} Unicode");

@michaelwoerister
Copy link
Member

Thanks a lot for the PR, @gentoo90! I would be great if you could add test cases to the src/test/debuginfo suite so those fixes don't break again in the future. Let me know if you need any help with that.

@gentoo90
Copy link
Contributor Author

Hi @michaelwoerister, is there any way to run a single test using the nightly binary build from rustup, without fetching all the submodules or at least without compiling everything in the repository?

@michaelwoerister
Copy link
Member

You could try ./x.py test --stage 0 src/test/debuginfo. That should spare you as much compiling as possible.

@gentoo90
Copy link
Contributor Author

Most of the tests are ignored, accept gdb-pretty-struct-and-enums-pre-gdb-7-7.rs. Is this OK? Is this because I don't have lldb installed?

@michaelwoerister
Copy link
Member

You should not need LLDB for the GDB based tests. I assume this is on Linux? What's the GDB version you are using?

@gentoo90
Copy link
Contributor Author

Yes, gentoo linux, gdb 7.12

@gentoo90
Copy link
Contributor Author

I've found another issue while trying to debug bootstrap. When it's built as a part of cargo workspace, it doesn't have info about its source files. So when I set a breakpoint in src/bootstrap/bin/main.rs, gdb says No source file named /home/user/rust/src/bootstrap/bin/main.rs.

But if I remove src/Cargo.toml and build bootstrap with cargo build, the breakpoint works just fine.

@michaelwoerister
Copy link
Member

Most of the tests are ignored...

I just tested with GDB 7.12 on Ubuntu and tests are not ignored for me. If you remove all *.stamp files from build/x86_64-unknown-linux-gnu/test/debuginfo, are tests still ignored during the first run of x.py test --stage 0 src/test/debuginfo? I get 13 ignored tests in that case, which should be fine.

I've found another issue while trying to debug bootstrap.

Would you mind opening an issue about that?

@gentoo90
Copy link
Contributor Author

gentoo90 commented Jun 2, 2017

A proper way to test tuple field names would be

// gdbr-command:interpreter-exec mi2 "-enable-pretty-printing"
// gdbr-check:^done
// gdbr-command:interpreter-exec mi2 "-var-create noPadding8 @ noPadding8"
// gdbr-check:^done,name="noPadding8",numchild="2",value="{...}",type="(i8, u8)",thread-id="1",has_more="0"
// gdbr-command:interpreter-exec mi2 "-var-list-children noPadding8"
// gdbr-check:^done,numchild="2",displayhint="array",children=[child={name="noPadding8.0",exp="0",numchild="0",value="-100",type="i8",thread-id="1"},child={name="noPadding8.1",exp="1",numchild="0",value="100",type="u8",thread-id="1"}],has_more="0"

but -enable-pretty-printing doesn't do anything in batch mode and pretty-printers are not used :(. Looks like a bug in GDB.

Use a class without children() method for printing empty structs.
Presence of this method makes GDB's variable objects interface act like
if the struct had children.
@gentoo90
Copy link
Contributor Author

gentoo90 commented Jun 2, 2017

Added separate pretty-printer for empty structs, which prints the struct type instead of {...} in variable objects mode.

Before After
empty_before empty_after

Testing against variable objects is impossible again without -enable-pretty-printing, and CLI interface has the tests already.

@arielb1
Copy link
Contributor

arielb1 commented Jun 6, 2017

r? @michaelwoerister

@@ -38,6 +38,9 @@
// gdbg-check:$6 = None
// gdbr-check:$6 = core::option::Option::None

// gdb-command: print some_string
// gdbr-check:$7 = Some = {"IAMA optional string!"}
Copy link
Member

Choose a reason for hiding this comment

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

We should try making this gdb-check instead of just gdbr-check, so both kinds of GDB are tested.

@michaelwoerister
Copy link
Member

Ping @gentoo90, regarding #42278 (comment). Could you make these regular gdb-commands and gdb-checks? Otherwise we'll only test versions of GDB that already support Rust natively, while the pretty printers should also work for older GDB versions.

@michaelwoerister michaelwoerister added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 9, 2017
@gentoo90
Copy link
Contributor Author

gentoo90 commented Jun 9, 2017

Sorry for delay.

older GDB versions

ahh, so that's why gdbg-* stuff isn't runned on my system.

@michaelwoerister
Copy link
Member

Sorry for delay.

No worries. Thanks for continuing to work on this!

ahh, so that's why gdbg-* stuff isn't runned on my system.

Yes, that seems like a likely reason. I'm actually not sure what version of GDB is on our test infrastructure.

@michaelwoerister
Copy link
Member

@bors r+

Let's give it a try.

@bors
Copy link
Contributor

bors commented Jun 9, 2017

📌 Commit 63076dd has been approved by michaelwoerister

@bors
Copy link
Contributor

bors commented Jun 9, 2017

⌛ Testing commit 63076dd with merge 3d5b8c6...

bors added a commit that referenced this pull request Jun 9, 2017
Fix GDB pretty-printer for tuples and pointers

Names of children should not be the same, because GDB uses them to distinguish the children.

|Before|After|
|---|---|
|![tuples_before](https://cloud.githubusercontent.com/assets/1297574/26527639/5d6cf10e-43a0-11e7-9498-abfcddb08055.png)|![tuples_after](https://cloud.githubusercontent.com/assets/1297574/26527655/9699233a-43a0-11e7-83c6-f58f713b51a0.png)|

`main.rs`
```rust
enum Test {
    Zero,
    One(i32),
    Two(i32, String),
    Three(i32, String, Vec<String>),
}

fn main() {
    let tuple = (1, 2, "Asdfgh");
    let zero = Test::Zero;
    let one = Test::One(10);
    let two = Test::Two(42, "Qwerty".to_owned());
    let three = Test::Three(9000,
                            "Zxcvbn".to_owned(),
                            vec!["lorem".to_owned(), "ipsum".to_owned(), "dolor".to_owned()]);
    println!(""); // breakpoint here
}
```

`launch.json`
```json
{
    "version": "0.2.0",
    "configurations": [
        {
            "type": "gdb",
            "request": "launch",
            "gdbpath": "rust-gdb",
            "name": "Launch Program",
            "valuesFormatting": "prettyPrinters", //this requires plugin Native Debug >= 0.20.0
            "target": "./target/debug/test_pretty_printers",
            "cwd": "${workspaceRoot}"
        }
    ]
}
```
@bors
Copy link
Contributor

bors commented Jun 9, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: michaelwoerister
Pushing 3d5b8c6 to master...

@bors bors merged commit 63076dd into rust-lang:master Jun 9, 2017
@michaelwoerister
Copy link
Member

🎉
Thanks, @gentoo90!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants