| Message ID | 000000000000fb79ae059e568398@google.com |
|---|---|
| State | New |
| Headers | show |
Nit: Yeah, I forgot to edit out the ChangeLog diffs. On Tue, Feb 11, 2020 at 5:01 PM Doug Evans <dje@google.com> wrote: > > Hopefully straightforward (and I didn't miss anything ...). > > gdb/ChangeLog > PR rust/25535 > * rust-lang.c (rust_print_enum): Apply embedded_offset to > rust_enum_variant calculation. > > gdb/testsuite/ChangeLog > PR rust/25535 > * gdb.rust/simple.exp: Add test. > * gdb.rust/simple.rs: Add test. > > diff --git a/gdb/ChangeLog b/gdb/ChangeLog > index 91d7cec98af..1d8af99d8d6 100644 > --- a/gdb/ChangeLog > +++ b/gdb/ChangeLog > @@ -1,3 +1,9 @@ > +2020-02-11 Doug Evans <dje@google.com> > + > + PR rust/25535 > + * rust-lang.c (rust_print_enum): Apply embedded_offset to > + rust_enum_variant calculation. > + > 2020-02-11 Christian Biesinger <cbiesinger@google.com> > > * cris-tdep.c (cris_supply_gregset): Change signature to match > diff --git a/gdb/rust-lang.c b/gdb/rust-lang.c > index 9123bf2146d..18dc6a56471 100644 > --- a/gdb/rust-lang.c > +++ b/gdb/rust-lang.c > @@ -481,7 +481,8 @@ rust_print_enum (struct type *type, int embedded_offset, > } > > const gdb_byte *valaddr = value_contents_for_printing (val); > - struct field *variant_field = rust_enum_variant (type, valaddr); > + struct field *variant_field = rust_enum_variant (type, > + valaddr + embedded_offset); > embedded_offset += FIELD_BITPOS (*variant_field) / 8; > struct type *variant_type = FIELD_TYPE (*variant_field); > > diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog > index 06826746d5f..a613c1e6565 100644 > --- a/gdb/testsuite/ChangeLog > +++ b/gdb/testsuite/ChangeLog > @@ -1,3 +1,9 @@ > +2020-02-11 Doug Evans <dje@google.com> > + > + PR rust/25535 > + * gdb.rust/simple.exp: Add test. > + * gdb.rust/simple.rs: Add test. > + > 2020-02-11 Sergio Durigan Junior <sergiodj@redhat.com> > > PR tui/25126 > diff --git a/gdb/testsuite/gdb.rust/simple.exp b/gdb/testsuite/gdb.rust/simple.exp > index aa64e6b08f1..b4fcf274261 100644 > --- a/gdb/testsuite/gdb.rust/simple.exp > +++ b/gdb/testsuite/gdb.rust/simple.exp > @@ -357,6 +357,8 @@ gdb_test_sequence "ptype/o SimpleLayout" "" { > > gdb_test "print u2" " = simple::Union2 {name: \\\[1\\\]}" > > +gdb_test "print nonzero_offset" " = simple::EnumWithNonzeroOffset {a: core::option::Option<u8>::Some\\(1\\), b: core::option::Option<u8>::None}" > + > # PR rust/23626 - this used to crash. Note that the results are > # fairly lax because most existing versions of Rust (those before the > # DW_TAG_variant patches) do not emit what gdb wants here; and there > diff --git a/gdb/testsuite/gdb.rust/simple.rs b/gdb/testsuite/gdb.rust/simple.rs > index 1280e04449b..36448ec45c6 100644 > --- a/gdb/testsuite/gdb.rust/simple.rs > +++ b/gdb/testsuite/gdb.rust/simple.rs > @@ -104,6 +104,12 @@ struct SimpleLayout { > > enum EmptyEnum {} > > +#[derive(Debug)] > +struct EnumWithNonzeroOffset { > + a: Option<u8>, > + b: Option<u8>, > +} > + > fn main () { > let a = (); > let b : [i32; 0] = []; > @@ -186,6 +192,8 @@ fn main () { > > let u2 = Union2 { name: [1] }; > > + let nonzero_offset = EnumWithNonzeroOffset { a: Some(1), b: None }; > + > println!("{}, {}", x.0, x.1); // set breakpoint here > println!("{}", diff2(92, 45)); > empty();
On 2020-02-11 8:01 p.m., Doug Evans via gdb-patches wrote: > Hopefully straightforward (and I didn't miss anything ...). > > gdb/ChangeLog > PR rust/25535 > * rust-lang.c (rust_print_enum): Apply embedded_offset to > rust_enum_variant calculation. > > gdb/testsuite/ChangeLog > PR rust/25535 > * gdb.rust/simple.exp: Add test. > * gdb.rust/simple.rs: Add test. Hi Doug, Tom has this big series that gets rid of the val_print API (and therefore embedded_offset), which I think was on its way in. See this patch here in particular: https://sourceware.org/ml/gdb-patches/2019-12/msg00288.html Could you check if that fixes the bug? Of course, the test you add would still be relevant. Simon
>> gdb/ChangeLog >> PR rust/25535 >> * rust-lang.c (rust_print_enum): Apply embedded_offset to >> rust_enum_variant calculation. >> >> gdb/testsuite/ChangeLog >> PR rust/25535 >> * gdb.rust/simple.exp: Add test. >> * gdb.rust/simple.rs: Add test. Simon> Hi Doug, Simon> Tom has this big series that gets rid of the val_print API (and therefore embedded_offset), Simon> which I think was on its way in. See this patch here in particular: Simon> https://sourceware.org/ml/gdb-patches/2019-12/msg00288.html Simon> Could you check if that fixes the bug? Of course, the test you add would still be relevant. I think it's fine to land this now. I'll try to rebase the val_print thing soon. I've kind of been breaking things a lot so I've been putting it off a little. Tom
On 2020-02-12 2:54 a.m., Tom Tromey wrote: >>> gdb/ChangeLog >>> PR rust/25535 >>> * rust-lang.c (rust_print_enum): Apply embedded_offset to >>> rust_enum_variant calculation. >>> >>> gdb/testsuite/ChangeLog >>> PR rust/25535 >>> * gdb.rust/simple.exp: Add test. >>> * gdb.rust/simple.rs: Add test. > > Simon> Hi Doug, > > Simon> Tom has this big series that gets rid of the val_print API (and therefore embedded_offset), > Simon> which I think was on its way in. See this patch here in particular: > > Simon> https://sourceware.org/ml/gdb-patches/2019-12/msg00288.html > > Simon> Could you check if that fixes the bug? Of course, the test you add would still be relevant. > > I think it's fine to land this now. > > I'll try to rebase the val_print thing soon. I've kind of been breaking > things a lot so I've been putting it off a little. Agreed, we don't lose anything by merging this now. Simon
>>>>> "Doug" == Doug Evans via gdb-patches <gdb-patches@sourceware.org> writes:
Doug> gdb/ChangeLog
Doug> PR rust/25535
Doug> * rust-lang.c (rust_print_enum): Apply embedded_offset to
Doug> rust_enum_variant calculation.
I'm pushing this now, to unblock the val_print patches.
Tom
Thx. On Wed, Feb 19, 2020 at 12:58 PM Tom Tromey <tom@tromey.com> wrote: > >>>>> "Doug" == Doug Evans via gdb-patches <gdb-patches@sourceware.org> > writes: > > Doug> gdb/ChangeLog > Doug> PR rust/25535 > Doug> * rust-lang.c (rust_print_enum): Apply embedded_offset to > Doug> rust_enum_variant calculation. > > I'm pushing this now, to unblock the val_print patches. > > Tom >
diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 91d7cec98af..1d8af99d8d6 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,9 @@ +2020-02-11 Doug Evans <dje@google.com> + + PR rust/25535 + * rust-lang.c (rust_print_enum): Apply embedded_offset to + rust_enum_variant calculation. + 2020-02-11 Christian Biesinger <cbiesinger@google.com> * cris-tdep.c (cris_supply_gregset): Change signature to match diff --git a/gdb/rust-lang.c b/gdb/rust-lang.c index 9123bf2146d..18dc6a56471 100644 --- a/gdb/rust-lang.c +++ b/gdb/rust-lang.c @@ -481,7 +481,8 @@ rust_print_enum (struct type *type, int embedded_offset, } const gdb_byte *valaddr = value_contents_for_printing (val); - struct field *variant_field = rust_enum_variant (type, valaddr); + struct field *variant_field = rust_enum_variant (type, + valaddr + embedded_offset); embedded_offset += FIELD_BITPOS (*variant_field) / 8; struct type *variant_type = FIELD_TYPE (*variant_field); diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 06826746d5f..a613c1e6565 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,9 @@ +2020-02-11 Doug Evans <dje@google.com> + + PR rust/25535 + * gdb.rust/simple.exp: Add test. + * gdb.rust/simple.rs: Add test. + 2020-02-11 Sergio Durigan Junior <sergiodj@redhat.com> PR tui/25126 diff --git a/gdb/testsuite/gdb.rust/simple.exp b/gdb/testsuite/gdb.rust/simple.exp index aa64e6b08f1..b4fcf274261 100644 --- a/gdb/testsuite/gdb.rust/simple.exp +++ b/gdb/testsuite/gdb.rust/simple.exp @@ -357,6 +357,8 @@ gdb_test_sequence "ptype/o SimpleLayout" "" { gdb_test "print u2" " = simple::Union2 {name: \\\[1\\\]}" +gdb_test "print nonzero_offset" " = simple::EnumWithNonzeroOffset {a: core::option::Option<u8>::Some\\(1\\), b: core::option::Option<u8>::None}" + # PR rust/23626 - this used to crash. Note that the results are # fairly lax because most existing versions of Rust (those before the # DW_TAG_variant patches) do not emit what gdb wants here; and there diff --git a/gdb/testsuite/gdb.rust/simple.rs b/gdb/testsuite/gdb.rust/simple.rs index 1280e04449b..36448ec45c6 100644 --- a/gdb/testsuite/gdb.rust/simple.rs +++ b/gdb/testsuite/gdb.rust/simple.rs @@ -104,6 +104,12 @@ struct SimpleLayout { enum EmptyEnum {} +#[derive(Debug)] +struct EnumWithNonzeroOffset { + a: Option<u8>, + b: Option<u8>, +} + fn main () { let a = (); let b : [i32; 0] = []; @@ -186,6 +192,8 @@ fn main () { let u2 = Union2 { name: [1] }; + let nonzero_offset = EnumWithNonzeroOffset { a: Some(1), b: None }; + println!("{}, {}", x.0, x.1); // set breakpoint here println!("{}", diff2(92, 45)); empty();
Hopefully straightforward (and I didn't miss anything ...). gdb/ChangeLog PR rust/25535 * rust-lang.c (rust_print_enum): Apply embedded_offset to rust_enum_variant calculation. gdb/testsuite/ChangeLog PR rust/25535 * gdb.rust/simple.exp: Add test. * gdb.rust/simple.rs: Add test.