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

debuginfo-gdb tests can't handle new gdb that supports rust #36323

Closed
infinity0 opened this issue Sep 7, 2016 · 8 comments
Closed

debuginfo-gdb tests can't handle new gdb that supports rust #36323

infinity0 opened this issue Sep 7, 2016 · 8 comments
Labels
A-debuginfo Area: Debugging information in compiled programs (DWARF, PDB, etc.)

Comments

@infinity0
Copy link
Contributor

Lots of error: line not found in debugger output e.g. see buildlog for 1.13.0~~nightly.20160904, shorter snippet of one failure: https://paste.debian.net/812883/

test result: FAILED. 37 passed; 65 failed; 3 ignored; 0 measured
@Aatch Aatch added A-debuginfo Area: Debugging information in compiled programs (DWARF, PDB, etc.) A-amusing labels Sep 8, 2016
@michaelwoerister
Copy link
Member

@brson @alexcrichton We need test infrastructure that uses a Rust-enabled version of GDB.

cc @tromey @Manishearth

@Manishearth
Copy link
Member

set language rust might be necessary in those tests?

@alexcrichton
Copy link
Member

Discussed during tools triage today, conclusions were:

  • Ideally tests would support all versions of GDB, perhaps even including a gdb/lldb style split of assertions and commands for newer versions of GDB
  • We should have bots testing both versions of GDB
    • This may involve two bots with two versions of GDB
    • It may also involve one bot testing two versions of GDB

@nagisa
Copy link
Member

nagisa commented Nov 3, 2016

This is an example of a failure, which also makes it quite obvious why the tests fail:

error: line not found in debugger output: $4 = {a = 3, b = 4, c = 5, d = 6, e = 7, f = 8, g = 9, h = 10}
<snip>
$4 = function_arg_initialization::BigStruct {a: 3, b: 4, c: 5, d: 6, e: 7, f: 8, g: 9, h: 10}

This is a very nasty failure case because I haven’t seen a successful run of tests on arch for months now.

@TimNN
Copy link
Contributor

TimNN commented Nov 3, 2016

I've got a PR ready, which fixes all those tests (for GDB trunk), #37410.

sophiajt pushed a commit to sophiajt/rust that referenced this issue Nov 4, 2016
Support GDB with native rust support

This PR aims at making the debuginfo tests pass with the newer GDB versions which have native rust support (RGDB)

To that end debuginfo tests now support three GDB prefixes:
- `gdb` applicable to both GDB varieties
- `gdbg` (**G**eneric) only applicable to the old GDB
- `gdbr` (**R**ust) only applicable to the new RGDB

Whether the GDB has rust support is detected based on it's version: GDB >= 7.11.10 is considered to have rust support.

---

**Test updates**

All tests have been updated to the new GDB version. Note that some tests currently require the GDB trunk<sup>1</sup>.

---

**How to move forward with this PR:**

I propose the following steps for moving forward with this PR:
- [x] Validate the general approach of this PR (the `gdb-`, `gdbg-` and `gdbr-` split)
- [x] Validate the approach taken for updating the debuginfo tests (I've checked this since there's (almost) no `set language c` left, which was my main concern)
- [x] Determine how to distinguish between the new and old GDB (and implement that)
- [ ] Add one or more non-gating build bots with the new GDB (blocked on the previous item, can happen after this PR has been merged)
- [ ] If the new bots pass the tests, gate on them
- [x] \(Maybe) update the remaining tests to run without `set syntax c` (in a separate PR)
- [ ] \(Maybe) add tests specifically for the new GDB (in a separate PR / open an issue about this)

I'm not completely sure about the build bot related steps (cc @alexcrichton), the current approach was suggested to prevent any downtime / broken build time between a new GDB gating builder being added and this PR being merged.

---

**Suboptimal RGDB Output**

I've found several places where the output of RGDB is not ideal. They are tagged with `// FIXME`, here is an overview:
- [x] Trait references are not syntactically correct: `type_names::&Trait2<...>` (**WontFix**: the issue is minor and from @Manishearth below: "to properly address the trait issue we should wait for trait object support")
- [x] Univariant enums are printed as `<error reading variable>` (**Fixed** in GDB trunk<sup>1<sup>)
- [x] Unions are printed as `<error reading variable>` (**Fixed** in GDB trunk<sup>1</sup>)
- [x] "Nil Enums" (`enum Foo {}`) are printed as `<error reading variable>` (**WontFix**: the are not supposed to exist)
- [x] I have found no alternative for `sizeof(var)` in rust mode, so had to resort to `set language c` (**Fixed** in GDB trunk<sup>1</sup>)
- [x] I have found not way of interpreting a value as a specific enum variant, so had to resort to `set language c` (**Fixed** in GDB trunk<sup>1</sup>)
- [x] Before the initial `run` command, gdb does not realise it should be in rust mode (thus, if one want's to print statics before the run one has to explicitly `set language rust`) (maybe this is intended / expected behaviour, if so, someone please tell me ;) (**"Expected"** / current behaviour of GDB: picks up jemalloc, see rust-lang#37410 (comment))

<sup>1</sup>: Or rather in @Manishearth's trunk, waiting to be merged upstream.

---

cc @alexcrichton, @michaelwoerister, rust-lang#36323
alexcrichton added a commit to alexcrichton/rust that referenced this issue Nov 4, 2016
Support GDB with native rust support

This PR aims at making the debuginfo tests pass with the newer GDB versions which have native rust support (RGDB)

To that end debuginfo tests now support three GDB prefixes:
- `gdb` applicable to both GDB varieties
- `gdbg` (**G**eneric) only applicable to the old GDB
- `gdbr` (**R**ust) only applicable to the new RGDB

Whether the GDB has rust support is detected based on it's version: GDB >= 7.11.10 is considered to have rust support.

---

**Test updates**

All tests have been updated to the new GDB version. Note that some tests currently require the GDB trunk<sup>1</sup>.

---

**How to move forward with this PR:**

I propose the following steps for moving forward with this PR:
- [x] Validate the general approach of this PR (the `gdb-`, `gdbg-` and `gdbr-` split)
- [x] Validate the approach taken for updating the debuginfo tests (I've checked this since there's (almost) no `set language c` left, which was my main concern)
- [x] Determine how to distinguish between the new and old GDB (and implement that)
- [ ] Add one or more non-gating build bots with the new GDB (blocked on the previous item, can happen after this PR has been merged)
- [ ] If the new bots pass the tests, gate on them
- [x] \(Maybe) update the remaining tests to run without `set syntax c` (in a separate PR)
- [ ] \(Maybe) add tests specifically for the new GDB (in a separate PR / open an issue about this)

I'm not completely sure about the build bot related steps (cc @alexcrichton), the current approach was suggested to prevent any downtime / broken build time between a new GDB gating builder being added and this PR being merged.

---

**Suboptimal RGDB Output**

I've found several places where the output of RGDB is not ideal. They are tagged with `// FIXME`, here is an overview:
- [x] Trait references are not syntactically correct: `type_names::&Trait2<...>` (**WontFix**: the issue is minor and from @Manishearth below: "to properly address the trait issue we should wait for trait object support")
- [x] Univariant enums are printed as `<error reading variable>` (**Fixed** in GDB trunk<sup>1<sup>)
- [x] Unions are printed as `<error reading variable>` (**Fixed** in GDB trunk<sup>1</sup>)
- [x] "Nil Enums" (`enum Foo {}`) are printed as `<error reading variable>` (**WontFix**: the are not supposed to exist)
- [x] I have found no alternative for `sizeof(var)` in rust mode, so had to resort to `set language c` (**Fixed** in GDB trunk<sup>1</sup>)
- [x] I have found not way of interpreting a value as a specific enum variant, so had to resort to `set language c` (**Fixed** in GDB trunk<sup>1</sup>)
- [x] Before the initial `run` command, gdb does not realise it should be in rust mode (thus, if one want's to print statics before the run one has to explicitly `set language rust`) (maybe this is intended / expected behaviour, if so, someone please tell me ;) (**"Expected"** / current behaviour of GDB: picks up jemalloc, see rust-lang#37410 (comment))

<sup>1</sup>: Or rather in @Manishearth's trunk, waiting to be merged upstream.

---

cc @alexcrichton, @michaelwoerister, rust-lang#36323
@infinity0
Copy link
Contributor Author

#37410 was closed, does that mean this is now fixed? It doesn't seem to be in 1.14.0, will it be in 1.15.0?

@infinity0
Copy link
Contributor Author

Ping! Can this be closed? A fix was apparently merged in #37597

@michaelwoerister
Copy link
Member

Yes, I think this can be considered fixed.

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.)
Projects
None yet
Development

No branches or pull requests

7 participants