* [RFA v2 0/4] variants and variant parts
@ 2018-02-22 20:30 Tom Tromey
2018-02-22 20:30 ` [RFA v2 4/4] Handle DW_TAG_variant_part and DW_TAG_variant Tom Tromey
` (3 more replies)
0 siblings, 4 replies; 18+ messages in thread
From: Tom Tromey @ 2018-02-22 20:30 UTC (permalink / raw)
To: gdb-patches
This is the second version of my variant part series.
I believe this one addresses all of Joel's comments.
Additionally it:
* Adds a patch to fix some bugs in unpack_bits_as_long
* Updates the new DW_TAG_variant test case to test sign extension
* Fixes a bug I found in the Rust code
Regression tested by the buildbot.
Tom
^ permalink raw reply [flat|nested] 18+ messages in thread* [RFA v2 4/4] Handle DW_TAG_variant_part and DW_TAG_variant 2018-02-22 20:30 [RFA v2 0/4] variants and variant parts Tom Tromey @ 2018-02-22 20:30 ` Tom Tromey 2018-02-26 6:56 ` Joel Brobecker 2018-02-22 20:30 ` [RFA v2 1/4] Sign-extend non-bit-fields in unpack_bits_as_long Tom Tromey ` (2 subsequent siblings) 3 siblings, 1 reply; 18+ messages in thread From: Tom Tromey @ 2018-02-22 20:30 UTC (permalink / raw) To: gdb-patches; +Cc: Tom Tromey This changes dwarf2read to understand DW_TAG_variant_part and DW_TAG_variant. Note that DW_AT_discr_list is not handled. I did not need this for Rust. I imagine this should not be too hard to add later, should someone need it. Meanwhile I have gdb emit a complaint if it is seen. There is a lurking issue concerning the placement of the discriminant in the DWARF. For Rust, I ended up following the letter of the standard and having the discriminant be a child of the DW_TAG_variant_part. However, GCC's Ada support does not do this. Pierre-Marie filed this with the DWARF committee: http://dwarfstd.org/ShowIssue.php?issue=180123.1 However as that is read-only, if you have comments you might consider adding them to the GCC bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83935 Finally, there is a DWARF extension lurking in here. In Rust, a univariant enum will not have a discriminant. However, in order to unify the representation of all data-carrying enums, I've made LLVM (and my forthcoming rustc patch) emit a univariant enum using a DW_TAG_variant with a single variant part and without DW_AT_discr. The lack of this DW_AT_discr is the extension. I will submit an issue on dwarfstd.org about this. 2018-02-22 Tom Tromey <tom@tromey.com> * dwarf2read.c (struct variant_field): New. (struct nextfield) <variant>: New field. (dwarf2_add_field): Handle DW_TAG_variant_part. (dwarf2_attach_fields_to_type): Attach a discriminant_info to a discriminated union. (read_structure_type): Handle DW_TAG_variant_part. (handle_struct_member_die): New function, extracted from process_structure_scope. Handle DW_TAG_variant. (process_structure_scope): Handle discriminated unions. Call handle_struct_member_die. 2018-02-22 Tom Tromey <tom@tromey.com> * gdb.dwarf2/variant.c: New file. * gdb.dwarf2/variant.exp: New file. --- gdb/ChangeLog | 13 ++ gdb/dwarf2read.c | 218 ++++++++++++++++++++++++++++------ gdb/testsuite/ChangeLog | 5 + gdb/testsuite/gdb.dwarf2/variant.c | 47 ++++++++ gdb/testsuite/gdb.dwarf2/variant.exp | 224 +++++++++++++++++++++++++++++++++++ 5 files changed, 470 insertions(+), 37 deletions(-) create mode 100644 gdb/testsuite/gdb.dwarf2/variant.c create mode 100644 gdb/testsuite/gdb.dwarf2/variant.exp diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 74efd6383a..221556ab2b 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,5 +1,18 @@ 2018-02-22 Tom Tromey <tom@tromey.com> + * dwarf2read.c (struct variant_field): New. + (struct nextfield) <variant>: New field. + (dwarf2_add_field): Handle DW_TAG_variant_part. + (dwarf2_attach_fields_to_type): Attach a discriminant_info to a + discriminated union. + (read_structure_type): Handle DW_TAG_variant_part. + (handle_struct_member_die): New function, extracted from + process_structure_scope. Handle DW_TAG_variant. + (process_structure_scope): Handle discriminated unions. Call + handle_struct_member_die. + +2018-02-22 Tom Tromey <tom@tromey.com> + * rust-lang.h (rust_last_path_segment): Declare. * rust-lang.c (rust_last_path_segment): Now public. Change contract. diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c index c6e76caf1d..3ca6685657 100644 --- a/gdb/dwarf2read.c +++ b/gdb/dwarf2read.c @@ -1653,11 +1653,30 @@ struct dwarf_block and friends. */ static int bits_per_byte = 8; +/* When reading a variant or variant part, we track a bit more + information about the field, and store it in an object of this + type. */ + +struct variant_field +{ + /* If we see a DW_TAG_variant, then this will be the discriminant + value. */ + ULONGEST discriminant_value; + /* If we see a DW_TAG_variant, then this will be set if this is the + default branch. */ + bool default_branch; + /* While reading a DW_TAG_variant_part, this will be set if this + field is the discriminant. */ + bool is_discriminant; +}; + struct nextfield { struct nextfield *next; int accessibility; int virtuality; + /* Extra information to describe a variant or variant part. */ + struct variant_field variant; struct field field; }; @@ -15598,6 +15617,20 @@ dwarf2_add_field (struct field_info *fip, struct die_info *die, FIELD_NAME (*fp) = type_name_no_tag (fp->type); fip->nbaseclasses++; } + else if (die->tag == DW_TAG_variant_part) + { + /* process_structure_scope will treat this DIE as a union. */ + process_structure_scope (die, cu); + + /* The variant part is relative to the start of the enclosing + structure. */ + SET_FIELD_BITPOS (*fp, 0); + fp->type = get_die_type (die, cu); + fp->artificial = 1; + fp->name = "<<variant>>"; + } + else + gdb_assert_not_reached ("missing case in dwarf2_add_field"); } /* Can the type given by DIE define another type? */ @@ -15723,6 +15756,27 @@ dwarf2_attach_fields_to_type (struct field_info *fip, struct type *type, TYPE_N_BASECLASSES (type) = fip->nbaseclasses; } + if (TYPE_FLAG_DISCRIMINATED_UNION (type)) + { + struct discriminant_info *di = alloc_discriminant_info (type, -1, -1); + + int index = nfields - 1; + struct nextfield *field = fip->fields; + + while (index >= 0) + { + if (field->variant.is_discriminant) + di->discriminant_index = index; + else if (field->variant.default_branch) + di->default_index = index; + else + di->discriminants[index] = field->variant.discriminant_value; + + --index; + field = field->next; + } + } + /* Copy the saved-up fields into the field vector. Start from the head of the list, adding to the tail of the field array, so that they end up in the same order in the array in which they were added to the list. */ @@ -16184,6 +16238,11 @@ read_structure_type (struct die_info *die, struct dwarf2_cu *cu) { TYPE_CODE (type) = TYPE_CODE_UNION; } + else if (die->tag == DW_TAG_variant_part) + { + TYPE_CODE (type) = TYPE_CODE_UNION; + TYPE_FLAG_DISCRIMINATED_UNION (type) = 1; + } else { TYPE_CODE (type) = TYPE_CODE_STRUCT; @@ -16243,6 +16302,92 @@ read_structure_type (struct die_info *die, struct dwarf2_cu *cu) return type; } +/* A helper for process_structure_scope that handles a single member + DIE. */ + +static void +handle_struct_member_die (struct die_info *child_die, struct type *type, + struct field_info *fi, + std::vector<struct symbol *> *template_args, + struct dwarf2_cu *cu) +{ + if (child_die->tag == DW_TAG_member + || child_die->tag == DW_TAG_variable + || child_die->tag == DW_TAG_variant_part) + { + /* NOTE: carlton/2002-11-05: A C++ static data member + should be a DW_TAG_member that is a declaration, but + all versions of G++ as of this writing (so through at + least 3.2.1) incorrectly generate DW_TAG_variable + tags for them instead. */ + dwarf2_add_field (fi, child_die, cu); + } + else if (child_die->tag == DW_TAG_subprogram) + { + /* Rust doesn't have member functions in the C++ sense. + However, it does emit ordinary functions as children + of a struct DIE. */ + if (cu->language == language_rust) + read_func_scope (child_die, cu); + else + { + /* C++ member function. */ + dwarf2_add_member_fn (fi, child_die, type, cu); + } + } + else if (child_die->tag == DW_TAG_inheritance) + { + /* C++ base class field. */ + dwarf2_add_field (fi, child_die, cu); + } + else if (type_can_define_types (child_die)) + dwarf2_add_type_defn (fi, child_die, cu); + else if (child_die->tag == DW_TAG_template_type_param + || child_die->tag == DW_TAG_template_value_param) + { + struct symbol *arg = new_symbol (child_die, NULL, cu); + + if (arg != NULL) + template_args->push_back (arg); + } + else if (child_die->tag == DW_TAG_variant) + { + /* In a variant we want to get the discriminant and also add a + field for our sole member child. */ + struct attribute *discr = dwarf2_attr (child_die, DW_AT_discr_value, cu); + + for (struct die_info *variant_child = child_die->child; + variant_child != NULL; + variant_child = sibling_die (variant_child)) + { + if (variant_child->tag == DW_TAG_member) + { + handle_struct_member_die (variant_child, type, fi, + template_args, cu); + /* Only handle the one. */ + break; + } + } + + /* We don't handle this but we might as well report it if we see + it. */ + if (dwarf2_attr (child_die, DW_AT_discr_list, cu) != nullptr) + complaint (&symfile_complaints, + _("DW_AT_discr_list is not supported yet" + " - DIE at 0x%x [in module %s]"), + to_underlying (child_die->sect_off), + objfile_name (cu->per_cu->dwarf2_per_objfile->objfile)); + + /* The first field was just added, so we can stash the + discriminant there. */ + gdb_assert (fi->fields != NULL); + if (discr == NULL) + fi->fields->variant.default_branch = true; + else + fi->fields->variant.discriminant_value = DW_UNSND (discr); + } +} + /* Finish creating a structure or union type, including filling in its members and creating a symbol for it. */ @@ -16257,6 +16402,39 @@ process_structure_scope (struct die_info *die, struct dwarf2_cu *cu) if (type == NULL) type = read_structure_type (die, cu); + /* When reading a DW_TAG_variant_part, we need to notice when we + read the discriminant member, so we can record it later in the + discriminant_info. */ + bool is_variant_part = TYPE_FLAG_DISCRIMINATED_UNION (type); + sect_offset discr_offset; + + if (is_variant_part) + { + struct attribute *discr = dwarf2_attr (die, DW_AT_discr, cu); + if (discr == NULL) + { + /* Maybe it's a univariant form, an extension we support. + In this case arrange not to check the offset. */ + is_variant_part = false; + } + else if (attr_form_is_ref (discr)) + { + struct dwarf2_cu *target_cu = cu; + struct die_info *target_die = follow_die_ref (die, discr, &target_cu); + + discr_offset = target_die->sect_off; + } + else + { + complaint (&symfile_complaints, + _("DW_AT_discr does not have DIE reference form" + " - DIE at 0x%x [in module %s]"), + to_underlying (die->sect_off), + objfile_name (cu->per_cu->dwarf2_per_objfile->objfile)); + is_variant_part = false; + } + } + if (die->child != NULL && ! die_is_declaration (die, cu)) { struct field_info fi; @@ -16269,44 +16447,10 @@ process_structure_scope (struct die_info *die, struct dwarf2_cu *cu) while (child_die && child_die->tag) { - if (child_die->tag == DW_TAG_member - || child_die->tag == DW_TAG_variable) - { - /* NOTE: carlton/2002-11-05: A C++ static data member - should be a DW_TAG_member that is a declaration, but - all versions of G++ as of this writing (so through at - least 3.2.1) incorrectly generate DW_TAG_variable - tags for them instead. */ - dwarf2_add_field (&fi, child_die, cu); - } - else if (child_die->tag == DW_TAG_subprogram) - { - /* Rust doesn't have member functions in the C++ sense. - However, it does emit ordinary functions as children - of a struct DIE. */ - if (cu->language == language_rust) - read_func_scope (child_die, cu); - else - { - /* C++ member function. */ - dwarf2_add_member_fn (&fi, child_die, type, cu); - } - } - else if (child_die->tag == DW_TAG_inheritance) - { - /* C++ base class field. */ - dwarf2_add_field (&fi, child_die, cu); - } - else if (type_can_define_types (child_die)) - dwarf2_add_type_defn (&fi, child_die, cu); - else if (child_die->tag == DW_TAG_template_type_param - || child_die->tag == DW_TAG_template_value_param) - { - struct symbol *arg = new_symbol (child_die, NULL, cu); + handle_struct_member_die (child_die, type, &fi, &template_args, cu); - if (arg != NULL) - template_args.push_back (arg); - } + if (is_variant_part && discr_offset == child_die->sect_off) + fi.fields->variant.is_discriminant = true; child_die = sibling_die (child_die); } diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 9510dcff39..74d80e2a4d 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,5 +1,10 @@ 2018-02-22 Tom Tromey <tom@tromey.com> + * gdb.dwarf2/variant.c: New file. + * gdb.dwarf2/variant.exp: New file. + +2018-02-22 Tom Tromey <tom@tromey.com> + * gdb.rust/simple.exp: Accept more possible results in enum test. 2018-02-21 John Baldwin <jhb@FreeBSD.org> diff --git a/gdb/testsuite/gdb.dwarf2/variant.c b/gdb/testsuite/gdb.dwarf2/variant.c new file mode 100644 index 0000000000..e145e785ed --- /dev/null +++ b/gdb/testsuite/gdb.dwarf2/variant.c @@ -0,0 +1,47 @@ +/* Copyright 2018 Free Software Foundation, Inc. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see <http://www.gnu.org/licenses/>. */ + +unsigned char buffer[8]; +unsigned char buffer2[8]; + +static void +func (void) +{ +} + +int +main (void) +{ + /* Write the expected values into the buffer. */ + unsigned int x = 23; + if (*(char *) &x) + { + /* Little endian. */ + buffer[0] = 23; + buffer[4] = 23; + buffer2[0] = 255; + buffer2[4] = 23; + } + else + { + buffer[3] = 23; + buffer[7] = 23; + buffer2[0] = 255; + buffer2[7] = 23; + } + + func (); + return 0; +} diff --git a/gdb/testsuite/gdb.dwarf2/variant.exp b/gdb/testsuite/gdb.dwarf2/variant.exp new file mode 100644 index 0000000000..b61659fc22 --- /dev/null +++ b/gdb/testsuite/gdb.dwarf2/variant.exp @@ -0,0 +1,224 @@ +# Copyright 2018 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. + +# Test DW_TAG_variant_part and DW_TAG_variant. + +load_lib dwarf.exp + +# This test can only be run on targets which support DWARF-2 and use +# gas. +if {![dwarf2_support]} { + return 0 +} + +standard_testfile .c variant.S + +# Make some DWARF for the test. +set asm_file [standard_output_file $srcfile2] +Dwarf::assemble $asm_file { + upvar cu_lang cu_lang + + declare_labels uinteger_label float_label int8_label + declare_labels discr_1_label discr_2_label discr_3_label + declare_labels one_label two_label + + # Creating a CU with 4-byte addresses lets this test link on + # both 32- and 64-bit machines. + cu { addr_size 4 } { + compile_unit { + {name file1.txt} + {language @DW_LANG_Rust} + } { + uinteger_label: DW_TAG_base_type { + {DW_AT_byte_size 4 DW_FORM_sdata} + {DW_AT_encoding @DW_ATE_unsigned} + {DW_AT_name {unsigned integer}} + } + + int8_label: DW_TAG_base_type { + {DW_AT_byte_size 1 DW_FORM_sdata} + {DW_AT_encoding @DW_ATE_signed} + {DW_AT_name i8} + } + + float_label: base_type { + {name float} + {encoding @DW_ATE_float} + {byte_size 4 DW_FORM_sdata} + } + + one_label: structure_type { + {name One} + {byte_size 4 DW_FORM_sdata} + } { + member { + {name __0} + {type :$uinteger_label} + {data_member_location 0 data1} + } + } + + two_label: structure_type { + {name Two} + {byte_size 4 DW_FORM_sdata} + } { + member { + {name __0} + {type :$float_label} + {data_member_location 0 data1} + } + } + + structure_type { + {name Simple} + {byte_size 8 DW_FORM_sdata} + } { + variant_part { + {discr :$discr_1_label DW_FORM_ref4} + } { + discr_1_label: member { + {type :$uinteger_label} + {data_member_location 0 data1} + } + + variant { + {discr_value 23 udata} + } { + member { + {type :$one_label} + {data_member_location 4 data1} + } + } + + variant { + {discr_value 1 udata} + } { + member { + {type :$two_label} + {data_member_location 4 data1} + } + } + } + } + + structure_type { + {name Defaulted} + {byte_size 8 DW_FORM_sdata} + } { + variant_part { + {discr :$discr_2_label DW_FORM_ref4} + } { + discr_2_label: member { + {type :$uinteger_label} + {data_member_location 0 data1} + } + + variant { + } { + member { + {type :$one_label} + {data_member_location 4 data1} + } + } + + variant { + {discr_value 1 udata} + } { + member { + {type :$two_label} + {data_member_location 4 data1} + } + } + } + } + + structure_type { + {name Univariant} + {byte_size 8 DW_FORM_sdata} + } { + variant_part { + } { + variant { + } { + member { + {type :$one_label} + {data_member_location 4 data1} + } + } + } + } + + # Rust won't emit a negative discriminant like this, but + # we want to test the code path anyway. + structure_type { + {name Negative} + {byte_size 8 DW_FORM_sdata} + } { + variant_part { + {discr :$discr_3_label DW_FORM_ref4} + } { + discr_3_label: member { + {type :$int8_label} + {data_member_location 0 data1} + } + + variant { + {discr_value -1 sdata} + } { + member { + {type :$one_label} + {data_member_location 4 data1} + } + } + + # Make this the default value so we'll see an + # incorrect result if we mishandle signed + # discriminants. + variant { + } { + member { + {type :$two_label} + {data_member_location 4 data1} + } + } + } + } + } + } +} + +if { [prepare_for_testing "failed to prepare" ${testfile} \ + [list $srcfile $asm_file] debug] } { + return -1 +} + +if ![runto func] { + return -1 +} + +# Get the values into history so we can use it from Rust. +gdb_test "print (void *) buffer" "\\\$1 = .void .. $hex .buffer." +gdb_test "print (void *) buffer2" "\\\$2 = .void .. $hex .buffer2." + +gdb_test "set language rust" +gdb_test "print *(\$1 as *mut Simple)" " = One\\(23\\)" \ + "print as Simple" +gdb_test "print *(\$1 as *mut Defaulted)" " = One\\(23\\)" \ + "print as Defaulted" +gdb_test "print *(\$1 as *mut Univariant)" " = One\\(23\\)" \ + "print as Univariant" + +gdb_test "print *(\$2 as *mut Negative)" " = One\\(23\\)" \ + "print as Negative" -- 2.13.6 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFA v2 4/4] Handle DW_TAG_variant_part and DW_TAG_variant 2018-02-22 20:30 ` [RFA v2 4/4] Handle DW_TAG_variant_part and DW_TAG_variant Tom Tromey @ 2018-02-26 6:56 ` Joel Brobecker 2018-02-26 16:16 ` Tom Tromey 0 siblings, 1 reply; 18+ messages in thread From: Joel Brobecker @ 2018-02-26 6:56 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches > 2018-02-22 Tom Tromey <tom@tromey.com> > > * dwarf2read.c (struct variant_field): New. > (struct nextfield) <variant>: New field. > (dwarf2_add_field): Handle DW_TAG_variant_part. > (dwarf2_attach_fields_to_type): Attach a discriminant_info to a > discriminated union. > (read_structure_type): Handle DW_TAG_variant_part. > (handle_struct_member_die): New function, extracted from > process_structure_scope. Handle DW_TAG_variant. > (process_structure_scope): Handle discriminated unions. Call > handle_struct_member_die. > > 2018-02-22 Tom Tromey <tom@tromey.com> > > * gdb.dwarf2/variant.c: New file. > * gdb.dwarf2/variant.exp: New file. Looks good to me, Tom. Thanks! -- Joel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFA v2 4/4] Handle DW_TAG_variant_part and DW_TAG_variant 2018-02-26 6:56 ` Joel Brobecker @ 2018-02-26 16:16 ` Tom Tromey 0 siblings, 0 replies; 18+ messages in thread From: Tom Tromey @ 2018-02-26 16:16 UTC (permalink / raw) To: Joel Brobecker; +Cc: Tom Tromey, gdb-patches >>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes: >> 2018-02-22 Tom Tromey <tom@tromey.com> >> >> * dwarf2read.c (struct variant_field): New. >> (struct nextfield) <variant>: New field. >> (dwarf2_add_field): Handle DW_TAG_variant_part. >> (dwarf2_attach_fields_to_type): Attach a discriminant_info to a >> discriminated union. Joel> Looks good to me, Tom. Thanks! Thanks for the reviews. I've updated the patches. Patch #4 needed a minor adjustment to the new complaints to account for the recent 64-bit sect_offset change. Tom ^ permalink raw reply [flat|nested] 18+ messages in thread
* [RFA v2 1/4] Sign-extend non-bit-fields in unpack_bits_as_long 2018-02-22 20:30 [RFA v2 0/4] variants and variant parts Tom Tromey 2018-02-22 20:30 ` [RFA v2 4/4] Handle DW_TAG_variant_part and DW_TAG_variant Tom Tromey @ 2018-02-22 20:30 ` Tom Tromey 2018-02-26 6:45 ` Joel Brobecker 2018-02-22 20:30 ` [RFA v2 3/4] Convert Rust to use discriminated unions Tom Tromey 2018-02-22 20:30 ` [RFA v2 2/4] Initial support for variant parts Tom Tromey 3 siblings, 1 reply; 18+ messages in thread From: Tom Tromey @ 2018-02-22 20:30 UTC (permalink / raw) To: gdb-patches; +Cc: Tom Tromey unpack_bits_as_long is documented as sign-extending its result when the type is signed. However, it was only doing sign-extension in the case where the field was a bitfield -- that is, not when the "bitsize" parameter was 0, indicating the size should be taken from the type. Also, unpack_bits_as_long was incorrectly computing the shift for big-endian architectures for the non-bitfield case. This patch fixes these bugs in a straightforward way. A new selftest is included. 2018-02-22 Tom Tromey <tom@tromey.com> * Makefile.in (SUBDIR_UNITTESTS_SRCS): Add unittests/unpack-selftests.c. * unittests/unpack-selftests.c: New file. * value.c (unpack_bits_as_long): Fix bugs in non-bitfield cases. --- gdb/ChangeLog | 7 +++++ gdb/Makefile.in | 1 + gdb/unittests/unpack-selftests.c | 61 ++++++++++++++++++++++++++++++++++++++++ gdb/value.c | 7 +++-- 4 files changed, 74 insertions(+), 2 deletions(-) create mode 100644 gdb/unittests/unpack-selftests.c diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 1644ac364f..734bbdf832 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,10 @@ +2018-02-22 Tom Tromey <tom@tromey.com> + + * Makefile.in (SUBDIR_UNITTESTS_SRCS): Add + unittests/unpack-selftests.c. + * unittests/unpack-selftests.c: New file. + * value.c (unpack_bits_as_long): Fix bugs in non-bitfield cases. + 2018-02-21 John Baldwin <jhb@FreeBSD.org> * arch/aarch64.c: Include "common-defs.h". diff --git a/gdb/Makefile.in b/gdb/Makefile.in index 3a105d75c6..1c58b9270d 100644 --- a/gdb/Makefile.in +++ b/gdb/Makefile.in @@ -427,6 +427,7 @@ SUBDIR_UNITTESTS_SRCS = \ unittests/scoped_fd-selftests.c \ unittests/scoped_mmap-selftests.c \ unittests/scoped_restore-selftests.c \ + unittests/unpack-selftests.c \ unittests/xml-utils-selftests.c SUBDIR_UNITTESTS_OBS = $(patsubst %.c,%.o,$(SUBDIR_UNITTESTS_SRCS)) diff --git a/gdb/unittests/unpack-selftests.c b/gdb/unittests/unpack-selftests.c new file mode 100644 index 0000000000..89f3e09c4e --- /dev/null +++ b/gdb/unittests/unpack-selftests.c @@ -0,0 +1,61 @@ +/* Self tests for unpack_field_as_long + + Copyright (C) 2018 Free Software Foundation, Inc. + + This file is part of GDB. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see <http://www.gnu.org/licenses/>. */ + +#include "defs.h" +#include "selftest.h" +#include "selftest-arch.h" +#include "value.h" +#include "gdbtypes.h" +#include "arch-utils.h" + +namespace selftests { +namespace unpack { + +static void +unpack_field_as_long_tests (struct gdbarch *arch) +{ + gdb_byte buffer[8]; + const struct builtin_type *bt = builtin_type (arch); + struct type *struct_type = arch_composite_type (arch, "<<selftest>>", + TYPE_CODE_STRUCT); + + append_composite_type_field (struct_type, "field0", bt->builtin_int8); + append_composite_type_field_aligned (struct_type, "field1", + bt->builtin_uint32, 4); + + memset (buffer, 0, sizeof (buffer)); + buffer[0] = 255; + if (gdbarch_byte_order (arch) == BFD_ENDIAN_BIG) + buffer[7] = 23; + else + buffer[4] = 23; + + SELF_CHECK (unpack_field_as_long (struct_type, buffer, 0) == -1); + SELF_CHECK (unpack_field_as_long (struct_type, buffer, 1) == 23); +} + +} +} + +void +_initialize_unpack_selftests () +{ + selftests::register_test_foreach_arch + ("unpack_field_as_long", selftests::unpack::unpack_field_as_long_tests); +} diff --git a/gdb/value.c b/gdb/value.c index 9cd9a2fcc7..0d85fe3774 100644 --- a/gdb/value.c +++ b/gdb/value.c @@ -3244,7 +3244,10 @@ unpack_bits_as_long (struct type *field_type, const gdb_byte *valaddr, if (bitsize) bytes_read = ((bitpos % 8) + bitsize + 7) / 8; else - bytes_read = TYPE_LENGTH (field_type); + { + bytes_read = TYPE_LENGTH (field_type); + bitsize = 8 * bytes_read; + } read_offset = bitpos / 8; @@ -3262,7 +3265,7 @@ unpack_bits_as_long (struct type *field_type, const gdb_byte *valaddr, /* If the field does not entirely fill a LONGEST, then zero the sign bits. If the field is signed, and is negative, then sign extend. */ - if ((bitsize > 0) && (bitsize < 8 * (int) sizeof (val))) + if (bitsize < 8 * (int) sizeof (val)) { valmask = (((ULONGEST) 1) << bitsize) - 1; val &= valmask; -- 2.13.6 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFA v2 1/4] Sign-extend non-bit-fields in unpack_bits_as_long 2018-02-22 20:30 ` [RFA v2 1/4] Sign-extend non-bit-fields in unpack_bits_as_long Tom Tromey @ 2018-02-26 6:45 ` Joel Brobecker 0 siblings, 0 replies; 18+ messages in thread From: Joel Brobecker @ 2018-02-26 6:45 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches Hi Tom, On Thu, Feb 22, 2018 at 01:30:15PM -0700, Tom Tromey wrote: > unpack_bits_as_long is documented as sign-extending its result when > the type is signed. However, it was only doing sign-extension in the > case where the field was a bitfield -- that is, not when the "bitsize" > parameter was 0, indicating the size should be taken from the type. > > Also, unpack_bits_as_long was incorrectly computing the shift for > big-endian architectures for the non-bitfield case. > > This patch fixes these bugs in a straightforward way. A new selftest > is included. > > 2018-02-22 Tom Tromey <tom@tromey.com> > > * Makefile.in (SUBDIR_UNITTESTS_SRCS): Add > unittests/unpack-selftests.c. > * unittests/unpack-selftests.c: New file. > * value.c (unpack_bits_as_long): Fix bugs in non-bitfield cases. Looks good to me. Just one thing: I think we need to adjust unpack_bits_as_long's documentation a bit to match the reality of how this function is called: /* Unpack a bitfield of the specified FIELD_TYPE, from the object at VALADDR, and store the result in *RESULT. The bitfield starts at BITPOS bits and contains BITSIZE bits. I checked the callers, and none of them really make a distinction between bitsize = 0 and bitsize != 0. And since it's fairly straightforward to handlet this case directly in unpack_bits_as_long, I agree this is best. Pre-approved with this change. Thanks for adding a self-test! -- Joel ^ permalink raw reply [flat|nested] 18+ messages in thread
* [RFA v2 3/4] Convert Rust to use discriminated unions 2018-02-22 20:30 [RFA v2 0/4] variants and variant parts Tom Tromey 2018-02-22 20:30 ` [RFA v2 4/4] Handle DW_TAG_variant_part and DW_TAG_variant Tom Tromey 2018-02-22 20:30 ` [RFA v2 1/4] Sign-extend non-bit-fields in unpack_bits_as_long Tom Tromey @ 2018-02-22 20:30 ` Tom Tromey 2018-02-26 6:50 ` Joel Brobecker ` (2 more replies) 2018-02-22 20:30 ` [RFA v2 2/4] Initial support for variant parts Tom Tromey 3 siblings, 3 replies; 18+ messages in thread From: Tom Tromey @ 2018-02-22 20:30 UTC (permalink / raw) To: gdb-patches; +Cc: Tom Tromey A Rust enum is, essentially, a discriminated union. Currently the Rust language support handles Rust enums locally, in rust-lang.c. However, because I am changing the Rust compiler to use DW_TAG_variant* to represent enums, it seemed better to have a single internal representation for Rust enums in gdb. This patch implements this idea by moving the current Rust enum handling code to dwarf2read. This allows the simplification of some parts of rust-lang.c as well. 2018-02-22 Tom Tromey <tom@tromey.com> * rust-lang.h (rust_last_path_segment): Declare. * rust-lang.c (rust_last_path_segment): Now public. Change contract. (struct disr_info): Remove. (RUST_ENUM_PREFIX, RUST_ENCODED_ENUM_REAL) (RUST_ENCODED_ENUM_HIDDEN, rust_union_is_untagged) (rust_get_disr_info, rust_tuple_variant_type_p): Remove. (rust_enum_p, rust_enum_variant): New function. (rust_underscore_fields): Remove "offset" parameter. (rust_print_enum): New function. (rust_val_print) <TYPE_CODE_UNION>: Remove enum code. <TYPE_CODE_STRUCT>: Call rust_print_enum when appropriate. (rust_print_struct_def): Add "for_rust_enum" parameter. Handle enums. (rust_internal_print_type): New function, from rust_print_type. Remove enum code. (rust_print_type): Call rust_internal_print_type. (rust_evaluate_subexp) <STRUCTOP_ANONYMOUS, STRUCTOP_STRUCT>: Update enum handling. * dwarf2read.c (struct dwarf2_cu) <rust_unions>: New field. (rust_fully_qualify, alloc_discriminant_info, quirk_rust_enum) (rust_union_quirks): New functions. (process_full_comp_unit, process_full_type_unit): Call rust_union_quirks. (process_structure_scope): Update rust_unions if necessary. 2018-02-22 Tom Tromey <tom@tromey.com> * gdb.rust/simple.exp: Accept more possible results in enum test. --- gdb/ChangeLog | 28 ++ gdb/dwarf2read.c | 313 +++++++++++++++ gdb/rust-lang.c | 796 ++++++++++++++------------------------ gdb/rust-lang.h | 5 + gdb/testsuite/ChangeLog | 4 + gdb/testsuite/gdb.rust/simple.exp | 4 +- 6 files changed, 640 insertions(+), 510 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 2b9d886b76..74efd6383a 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,5 +1,33 @@ 2018-02-22 Tom Tromey <tom@tromey.com> + * rust-lang.h (rust_last_path_segment): Declare. + * rust-lang.c (rust_last_path_segment): Now public. Change + contract. + (struct disr_info): Remove. + (RUST_ENUM_PREFIX, RUST_ENCODED_ENUM_REAL) + (RUST_ENCODED_ENUM_HIDDEN, rust_union_is_untagged) + (rust_get_disr_info, rust_tuple_variant_type_p): Remove. + (rust_enum_p, rust_enum_variant): New function. + (rust_underscore_fields): Remove "offset" parameter. + (rust_print_enum): New function. + (rust_val_print) <TYPE_CODE_UNION>: Remove enum code. + <TYPE_CODE_STRUCT>: Call rust_print_enum when appropriate. + (rust_print_struct_def): Add "for_rust_enum" parameter. Handle + enums. + (rust_internal_print_type): New function, from rust_print_type. + Remove enum code. + (rust_print_type): Call rust_internal_print_type. + (rust_evaluate_subexp) <STRUCTOP_ANONYMOUS, STRUCTOP_STRUCT>: + Update enum handling. + * dwarf2read.c (struct dwarf2_cu) <rust_unions>: New field. + (rust_fully_qualify, alloc_discriminant_info, quirk_rust_enum) + (rust_union_quirks): New functions. + (process_full_comp_unit, process_full_type_unit): Call + rust_union_quirks. + (process_structure_scope): Update rust_unions if necessary. + +2018-02-22 Tom Tromey <tom@tromey.com> + * value.h (value_union_variant): Declare. * valops.c (value_union_variant): New function. * gdbtypes.h (TYPE_FLAG_DISCRIMINATED_UNION): New macro. diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c index 6cdb96335f..c6e76caf1d 100644 --- a/gdb/dwarf2read.c +++ b/gdb/dwarf2read.c @@ -86,6 +86,7 @@ #include <cmath> #include <set> #include <forward_list> +#include "rust-lang.h" /* When == 1, print basic high level tracing messages. When > 1, be more verbose. @@ -762,6 +763,14 @@ struct dwarf2_cu whether the DW_AT_ranges attribute came from the skeleton or DWO. */ ULONGEST ranges_base = 0; + /* When reading debug info generated by older versions of rustc, we + have to rewrite some union types to be struct types with a + variant part. This rewriting must be done after the CU is fully + read in, because otherwise at the point of rewriting some struct + type might not have been fully processed. So, we keep a list of + all such types here and process them after expansion. */ + std::vector<struct type *> rust_unions; + /* Mark used when releasing cached dies. */ unsigned int mark : 1; @@ -10274,6 +10283,302 @@ fixup_go_packaging (struct dwarf2_cu *cu) } } +/* Allocate a fully-qualified name consisting of the two parts on the + obstack. */ + +static const char * +rust_fully_qualify (struct obstack *obstack, const char *p1, const char *p2) +{ + return obconcat (obstack, p1, "::", p2, (char *) NULL); +} + +/* A helper that allocates a struct discriminant_info to attach to a + union type. */ + +static struct discriminant_info * +alloc_discriminant_info (struct type *type, int discriminant_index, + int default_index) +{ + gdb_assert (TYPE_CODE (type) == TYPE_CODE_UNION); + gdb_assert (default_index == -1 + || (default_index > 0 && default_index < TYPE_NFIELDS (type))); + + TYPE_FLAG_DISCRIMINATED_UNION (type) = 1; + + struct discriminant_info *disc + = ((struct discriminant_info *) + TYPE_ZALLOC (type, + offsetof (struct discriminant_info, discriminants) + + TYPE_NFIELDS (type) * sizeof (disc->discriminants[0]))); + disc->default_index = default_index; + disc->discriminant_index = discriminant_index; + + struct dynamic_prop prop; + prop.kind = PROP_UNDEFINED; + prop.data.baton = disc; + + add_dyn_prop (DYN_PROP_DISCRIMINATED, prop, type); + + return disc; +} + +/* Some versions of rustc emitted enums in an unusual way. + + Ordinary enums were emitted as unions. The first element of each + structure in the union was named "RUST$ENUM$DISR". This element + held the discriminant. + + These versions of Rust also implemented the "non-zero" + optimization. When the enum had two values, and one is empty and + the other holds a pointer that cannot be zero, the pointer is used + as the discriminant, with a zero value meaning the empty variant. + Here, the union's first member is of the form + RUST$ENCODED$ENUM$<fieldno>$<fieldno>$...$<variantname> + where the fieldnos are the indices of the fields that should be + traversed in order to find the field (which may be several fields deep) + and the variantname is the name of the variant of the case when the + field is zero. + + This function recognizes whether TYPE is of one of these forms, + and, if so, smashes it to be a variant type. */ + +static void +quirk_rust_enum (struct type *type, struct objfile *objfile) +{ + gdb_assert (TYPE_CODE (type) == TYPE_CODE_UNION); + + /* We don't need to deal with empty enums. */ + if (TYPE_NFIELDS (type) == 0) + return; + +#define RUST_ENUM_PREFIX "RUST$ENCODED$ENUM$" + if (TYPE_NFIELDS (type) == 1 + && startswith (TYPE_FIELD_NAME (type, 0), RUST_ENUM_PREFIX)) + { + const char *name = TYPE_FIELD_NAME (type, 0) + strlen (RUST_ENUM_PREFIX); + + /* Decode the field name to find the offset of the + discriminant. */ + ULONGEST bit_offset = 0; + struct type *field_type = TYPE_FIELD_TYPE (type, 0); + while (name[0] >= '0' && name[0] <= '9') + { + char *tail; + unsigned long index = strtoul (name, &tail, 10); + name = tail; + if (*name != '$' + || index >= TYPE_NFIELDS (field_type) + || (TYPE_FIELD_LOC_KIND (field_type, index) + != FIELD_LOC_KIND_BITPOS)) + { + complaint (&symfile_complaints, + _("Could not parse Rust enum encoding string \"%s\"" + "[in module %s]"), + TYPE_FIELD_NAME (type, 0), + objfile_name (objfile)); + return; + } + ++name; + + bit_offset += TYPE_FIELD_BITPOS (field_type, index); + field_type = TYPE_FIELD_TYPE (field_type, index); + } + + /* Make a union to hold the variants. */ + struct type *union_type = alloc_type (objfile); + TYPE_CODE (union_type) = TYPE_CODE_UNION; + TYPE_NFIELDS (union_type) = 3; + TYPE_FIELDS (union_type) + = (struct field *) TYPE_ZALLOC (type, 3 * sizeof (struct field)); + TYPE_LENGTH (union_type) = TYPE_LENGTH (type); + + /* Put the discriminant must at index 0. */ + TYPE_FIELD_TYPE (union_type, 0) = field_type; + TYPE_FIELD_ARTIFICIAL (union_type, 0) = 1; + TYPE_FIELD_NAME (union_type, 0) = "<<discriminant>>"; + SET_FIELD_BITPOS (TYPE_FIELD (union_type, 0), bit_offset); + + /* The order of fields doesn't really matter, so put the real + field at index 1 and the data-less field at index 2. */ + struct discriminant_info *disc + = alloc_discriminant_info (union_type, 0, 1); + TYPE_FIELD (union_type, 1) = TYPE_FIELD (type, 0); + TYPE_FIELD_NAME (union_type, 1) + = rust_last_path_segment (TYPE_NAME (TYPE_FIELD_TYPE (union_type, 1))); + TYPE_NAME (TYPE_FIELD_TYPE (union_type, 1)) + = rust_fully_qualify (&objfile->objfile_obstack, TYPE_NAME (type), + TYPE_FIELD_NAME (union_type, 1)); + + const char *dataless_name + = rust_fully_qualify (&objfile->objfile_obstack, TYPE_NAME (type), + name); + struct type *dataless_type = init_type (objfile, TYPE_CODE_VOID, 0, + dataless_name); + TYPE_FIELD_TYPE (union_type, 2) = dataless_type; + /* NAME points into the original discriminant name, which + already has the correct lifetime. */ + TYPE_FIELD_NAME (union_type, 2) = name; + SET_FIELD_BITPOS (TYPE_FIELD (union_type, 2), 0); + disc->discriminants[2] = 0; + + /* Smash this type to be a structure type. We have to do this + because the type has already been recorded. */ + TYPE_CODE (type) = TYPE_CODE_STRUCT; + TYPE_NFIELDS (type) = 1; + TYPE_FIELDS (type) + = (struct field *) TYPE_ZALLOC (type, sizeof (struct field)); + + /* Install the variant part. */ + TYPE_FIELD_TYPE (type, 0) = union_type; + SET_FIELD_BITPOS (TYPE_FIELD (type, 0), 0); + TYPE_FIELD_NAME (type, 0) = "<<variants>>"; + } + else if (TYPE_NFIELDS (type) == 1) + { + /* We assume that a union with a single field is a univariant + enum. */ + /* Smash this type to be a structure type. We have to do this + because the type has already been recorded. */ + TYPE_CODE (type) = TYPE_CODE_STRUCT; + + /* Make a union to hold the variants. */ + struct type *union_type = alloc_type (objfile); + TYPE_CODE (union_type) = TYPE_CODE_UNION; + TYPE_NFIELDS (union_type) = TYPE_NFIELDS (type); + TYPE_LENGTH (union_type) = TYPE_LENGTH (type); + TYPE_FIELDS (union_type) = TYPE_FIELDS (type); + + struct type *field_type = TYPE_FIELD_TYPE (union_type, 0); + const char *variant_name + = rust_last_path_segment (TYPE_NAME (field_type)); + TYPE_FIELD_NAME (union_type, 0) = variant_name; + TYPE_NAME (field_type) + = rust_fully_qualify (&objfile->objfile_obstack, + TYPE_NAME (field_type), variant_name); + + /* Install the union in the outer struct type. */ + TYPE_NFIELDS (type) = 1; + TYPE_FIELDS (type) + = (struct field *) TYPE_ZALLOC (union_type, sizeof (struct field)); + TYPE_FIELD_TYPE (type, 0) = union_type; + TYPE_FIELD_NAME (type, 0) = "<<variants>>"; + SET_FIELD_BITPOS (TYPE_FIELD (type, 0), 0); + + alloc_discriminant_info (union_type, -1, 0); + } + else + { + struct type *disr_type = nullptr; + for (int i = 0; i < TYPE_NFIELDS (type); ++i) + { + disr_type = TYPE_FIELD_TYPE (type, i); + + if (TYPE_NFIELDS (disr_type) == 0) + { + /* Could be data-less variant, so keep going. */ + } + else if (strcmp (TYPE_FIELD_NAME (disr_type, 0), + "RUST$ENUM$DISR") != 0) + { + /* Not a Rust enum. */ + return; + } + else + { + /* Found one. */ + break; + } + } + + /* If we got here without a discriminant, then it's probably + just a union. */ + if (disr_type == nullptr) + return; + + /* Smash this type to be a structure type. We have to do this + because the type has already been recorded. */ + TYPE_CODE (type) = TYPE_CODE_STRUCT; + + /* Make a union to hold the variants. */ + struct field *disr_field = &TYPE_FIELD (disr_type, 0); + struct type *union_type = alloc_type (objfile); + TYPE_CODE (union_type) = TYPE_CODE_UNION; + TYPE_NFIELDS (union_type) = 1 + TYPE_NFIELDS (type); + TYPE_LENGTH (union_type) = TYPE_LENGTH (type); + TYPE_FIELDS (union_type) + = (struct field *) TYPE_ZALLOC (union_type, + (TYPE_NFIELDS (union_type) + * sizeof (struct field))); + + memcpy (TYPE_FIELDS (union_type) + 1, TYPE_FIELDS (type), + TYPE_NFIELDS (type) * sizeof (struct field)); + + /* Install the discriminant at index 0 in the union. */ + TYPE_FIELD (union_type, 0) = *disr_field; + TYPE_FIELD_ARTIFICIAL (union_type, 0) = 1; + TYPE_FIELD_NAME (union_type, 0) = "<<discriminant>>"; + + /* Install the union in the outer struct type. */ + TYPE_FIELD_TYPE (type, 0) = union_type; + TYPE_FIELD_NAME (type, 0) = "<<variants>>"; + TYPE_NFIELDS (type) = 1; + + /* Set the size and offset of the union type. */ + SET_FIELD_BITPOS (TYPE_FIELD (type, 0), 0); + + /* We need a way to find the correct discriminant given a + variant name. For convenience we build a map here. */ + struct type *enum_type = FIELD_TYPE (*disr_field); + std::unordered_map<std::string, ULONGEST> discriminant_map; + for (int i = 0; i < TYPE_NFIELDS (enum_type); ++i) + { + if (TYPE_FIELD_LOC_KIND (enum_type, i) == FIELD_LOC_KIND_ENUMVAL) + { + const char *name + = rust_last_path_segment (TYPE_FIELD_NAME (enum_type, i)); + discriminant_map[name] = TYPE_FIELD_ENUMVAL (enum_type, i); + } + } + + int n_fields = TYPE_NFIELDS (union_type); + struct discriminant_info *disc + = alloc_discriminant_info (union_type, 0, -1); + /* Skip the discriminant here. */ + for (int i = 1; i < n_fields; ++i) + { + /* Find the final word in the name of this variant's type. + That name can be used to look up the correct + discriminant. */ + const char *variant_name + = rust_last_path_segment (TYPE_NAME (TYPE_FIELD_TYPE (union_type, + i))); + + auto iter = discriminant_map.find (variant_name); + if (iter != discriminant_map.end ()) + disc->discriminants[i] = iter->second; + + /* Remove the discriminant field. */ + struct type *sub_type = TYPE_FIELD_TYPE (union_type, i); + --TYPE_NFIELDS (sub_type); + ++TYPE_FIELDS (sub_type); + TYPE_FIELD_NAME (union_type, i) = variant_name; + TYPE_NAME (sub_type) + = rust_fully_qualify (&objfile->objfile_obstack, + TYPE_NAME (type), variant_name); + } + } +} + +/* Rewrite some Rust unions to be structures with variants parts. */ + +static void +rust_union_quirks (struct dwarf2_cu *cu) +{ + gdb_assert (cu->language == language_rust); + for (struct type *type : cu->rust_unions) + quirk_rust_enum (type, cu->per_cu->dwarf2_per_objfile->objfile); +} + /* Return the symtab for PER_CU. This works properly regardless of whether we're using the index or psymtabs. */ @@ -10458,6 +10763,9 @@ process_full_comp_unit (struct dwarf2_per_cu_data *per_cu, physnames. */ compute_delayed_physnames (cu); + if (cu->language == language_rust) + rust_union_quirks (cu); + /* Some compilers don't define a DW_AT_high_pc attribute for the compilation unit. If the DW_AT_high_pc is missing, synthesize it, by scanning the DIE's below the compilation unit. */ @@ -10560,6 +10868,9 @@ process_full_type_unit (struct dwarf2_per_cu_data *per_cu, physnames. */ compute_delayed_physnames (cu); + if (cu->language == language_rust) + rust_union_quirks (cu); + /* TUs share symbol tables. If this is the first TU to use this symtab, complete the construction of it with end_expandable_symtab. Otherwise, complete the addition of @@ -16138,6 +16449,8 @@ process_structure_scope (struct die_info *die, struct dwarf2_cu *cu) } quirk_gcc_member_function_pointer (type, objfile); + if (cu->language == language_rust && die->tag == DW_TAG_union_type) + cu->rust_unions.push_back (type); /* NOTE: carlton/2004-03-16: GCC 3.4 (or at least one of its snapshots) has been known to create a die giving a declaration diff --git a/gdb/rust-lang.c b/gdb/rust-lang.c index 5ff80b249b..35126da7a6 100644 --- a/gdb/rust-lang.c +++ b/gdb/rust-lang.c @@ -36,17 +36,15 @@ #include <string> #include <vector> -/* Returns the last segment of a Rust path like foo::bar::baz. Will - not handle cases where the last segment contains generics. This - will return NULL if the last segment cannot be found. */ +/* See rust-lang.h. */ -static const char * -rust_last_path_segment (const char * path) +const char * +rust_last_path_segment (const char *path) { const char *result = strrchr (path, ':'); if (result == NULL) - return NULL; + return path; return result + 1; } @@ -63,209 +61,29 @@ rust_crate_for_block (const struct block *block) return std::string (scope, cp_find_first_component (scope)); } -/* Information about the discriminant/variant of an enum */ - -struct disr_info -{ - /* Name of field. */ - std::string name; - /* Field number in union. Negative on error. For an encoded enum, - the "hidden" member will always be field 1, and the "real" member - will always be field 0. */ - int field_no; - /* True if this is an encoded enum that has a single "real" member - and a single "hidden" member. */ - unsigned int is_encoded : 1; -}; - -/* The prefix of a specially-encoded enum. */ +/* Return true if TYPE, which must be a struct type, represents a Rust + enum. */ -#define RUST_ENUM_PREFIX "RUST$ENCODED$ENUM$" - -/* The number of the real field. */ - -#define RUST_ENCODED_ENUM_REAL 0 - -/* The number of the hidden field. */ - -#define RUST_ENCODED_ENUM_HIDDEN 1 - -/* Whether or not a TYPE_CODE_UNION value is an untagged union - as opposed to being a regular Rust enum. */ static bool -rust_union_is_untagged (struct type *type) +rust_enum_p (const struct type *type) { - /* Unions must have at least one field. */ - if (TYPE_NFIELDS (type) == 0) - return false; - /* If the first field is named, but the name has the rust enum prefix, - it is an enum. */ - if (strncmp (TYPE_FIELD_NAME (type, 0), RUST_ENUM_PREFIX, - strlen (RUST_ENUM_PREFIX)) == 0) - return false; - /* Unions only have named fields. */ - for (int i = 0; i < TYPE_NFIELDS (type); ++i) - { - if (strlen (TYPE_FIELD_NAME (type, i)) == 0) - return false; - } - return true; + return (TYPE_CODE (type) == TYPE_CODE_STRUCT + && TYPE_NFIELDS (type) == 1 + && TYPE_FLAG_DISCRIMINATED_UNION (TYPE_FIELD_TYPE (type, 0))); } -/* Utility function to get discriminant info for a given value. */ +/* Given an enum type and contents, find which variant is active. */ -static struct disr_info -rust_get_disr_info (struct type *type, const gdb_byte *valaddr, - int embedded_offset, CORE_ADDR address, - struct value *val) +struct field * +rust_enum_variant (struct type *type, const gdb_byte *contents) { - int i; - struct disr_info ret; - struct type *disr_type; - struct value_print_options opts; - const char *name_segment; - - get_no_prettyformat_print_options (&opts); - - ret.field_no = -1; - ret.is_encoded = 0; - - if (TYPE_NFIELDS (type) == 0) - error (_("Encountered void enum value")); - - /* If an enum has two values where one is empty and the other holds - a pointer that cannot be zero; then the Rust compiler optimizes - away the discriminant and instead uses a zero value in the - pointer field to indicate the empty variant. */ - if (strncmp (TYPE_FIELD_NAME (type, 0), RUST_ENUM_PREFIX, - strlen (RUST_ENUM_PREFIX)) == 0) - { - char *tail, *token, *saveptr = NULL; - unsigned long fieldno; - struct type *member_type; - LONGEST value; - - ret.is_encoded = 1; - - if (TYPE_NFIELDS (type) != 1) - error (_("Only expected one field in %s type"), RUST_ENUM_PREFIX); - - /* Optimized enums have only one field. */ - member_type = TYPE_FIELD_TYPE (type, 0); - - std::string name (TYPE_FIELD_NAME (type, 0)); - tail = &name[0] + strlen (RUST_ENUM_PREFIX); - - /* The location of the value that doubles as a discriminant is - stored in the name of the field, as - RUST$ENCODED$ENUM$<fieldno>$<fieldno>$...$<variantname> - where the fieldnos are the indices of the fields that should be - traversed in order to find the field (which may be several fields deep) - and the variantname is the name of the variant of the case when the - field is zero. */ - for (token = strtok_r (tail, "$", &saveptr); - token != NULL; - token = strtok_r (NULL, "$", &saveptr)) - { - if (sscanf (token, "%lu", &fieldno) != 1) - { - /* We have reached the enum name, which cannot start - with a digit. */ - break; - } - if (fieldno >= TYPE_NFIELDS (member_type)) - error (_("%s refers to field after end of member type"), - RUST_ENUM_PREFIX); - - embedded_offset += TYPE_FIELD_BITPOS (member_type, fieldno) / 8; - member_type = TYPE_FIELD_TYPE (member_type, fieldno); - } - - if (token == NULL) - error (_("Invalid form for %s"), RUST_ENUM_PREFIX); - value = unpack_long (member_type, valaddr + embedded_offset); - - if (value == 0) - { - ret.field_no = RUST_ENCODED_ENUM_HIDDEN; - ret.name = std::string (TYPE_NAME (type)) + "::" + token; - } - else - { - ret.field_no = RUST_ENCODED_ENUM_REAL; - ret.name = (std::string (TYPE_NAME (type)) + "::" - + rust_last_path_segment (TYPE_NAME (TYPE_FIELD_TYPE (type, 0)))); - } - - return ret; - } - - disr_type = TYPE_FIELD_TYPE (type, 0); - - if (TYPE_NFIELDS (disr_type) == 0) - { - /* This is a bounds check and should never be hit unless Rust - has changed its debuginfo format. */ - error (_("Could not find enum discriminant field")); - } - else if (TYPE_NFIELDS (type) == 1) - { - /* Sometimes univariant enums are encoded without a - discriminant. In that case, treating it as an encoded enum - with the first field being the actual type works. */ - const char *field_name = TYPE_NAME (TYPE_FIELD_TYPE (type, 0)); - const char *last = rust_last_path_segment (field_name); - ret.name = std::string (TYPE_NAME (type)) + "::" + last; - ret.field_no = RUST_ENCODED_ENUM_REAL; - ret.is_encoded = 1; - return ret; - } - - if (strcmp (TYPE_FIELD_NAME (disr_type, 0), "RUST$ENUM$DISR") != 0) - error (_("Rust debug format has changed")); - - string_file temp_file; - /* The first value of the first field (or any field) - is the discriminant value. */ - c_val_print (TYPE_FIELD_TYPE (disr_type, 0), - (embedded_offset + TYPE_FIELD_BITPOS (type, 0) / 8 - + TYPE_FIELD_BITPOS (disr_type, 0) / 8), - address, &temp_file, - 0, val, &opts); - - ret.name = std::move (temp_file.string ()); - name_segment = rust_last_path_segment (ret.name.c_str ()); - if (name_segment != NULL) - { - for (i = 0; i < TYPE_NFIELDS (type); ++i) - { - /* Sadly, the discriminant value paths do not match the type - field name paths ('core::option::Option::Some' vs - 'core::option::Some'). However, enum variant names are - unique in the last path segment and the generics are not - part of this path, so we can just compare those. This is - hackish and would be better fixed by improving rustc's - metadata for enums. */ - const char *field_type = TYPE_NAME (TYPE_FIELD_TYPE (type, i)); - - if (field_type != NULL - && strcmp (name_segment, - rust_last_path_segment (field_type)) == 0) - { - ret.field_no = i; - break; - } - } - } + /* In Rust the enum always fills the containing structure. */ + gdb_assert (TYPE_FIELD_BITPOS (type, 0) == 0); - if (ret.field_no == -1 && !ret.name.empty ()) - { - /* Somehow the discriminant wasn't found. */ - error (_("Could not find variant of %s with discriminant %s"), - TYPE_TAG_NAME (type), ret.name.c_str ()); - } + struct type *union_type = TYPE_FIELD_TYPE (type, 0); - return ret; + int fieldno = value_union_variant (union_type, contents); + return &TYPE_FIELD (union_type, fieldno); } /* See rust-lang.h. */ @@ -281,12 +99,11 @@ rust_tuple_type_p (struct type *type) && TYPE_TAG_NAME (type)[0] == '('); } - /* Return true if all non-static fields of a structlike type are in a - sequence like __0, __1, __2. OFFSET lets us skip fields. */ + sequence like __0, __1, __2. */ static bool -rust_underscore_fields (struct type *type, int offset) +rust_underscore_fields (struct type *type) { int i, field_number; @@ -298,17 +115,12 @@ rust_underscore_fields (struct type *type, int offset) { if (!field_is_static (&TYPE_FIELD (type, i))) { - if (offset > 0) - offset--; - else - { - char buf[20]; + char buf[20]; - xsnprintf (buf, sizeof (buf), "__%d", field_number); - if (strcmp (buf, TYPE_FIELD_NAME (type, i)) != 0) - return false; - field_number++; - } + xsnprintf (buf, sizeof (buf), "__%d", field_number); + if (strcmp (buf, TYPE_FIELD_NAME (type, i)) != 0) + return false; + field_number++; } } return true; @@ -322,16 +134,7 @@ rust_tuple_struct_type_p (struct type *type) /* This is just an approximation until DWARF can represent Rust more precisely. We exclude zero-length structs because they may not be tuple structs, and there's no way to tell. */ - return TYPE_NFIELDS (type) > 0 && rust_underscore_fields (type, 0); -} - -/* Return true if a variant TYPE is a tuple variant, false otherwise. */ - -static bool -rust_tuple_variant_type_p (struct type *type) -{ - /* First field is discriminant */ - return rust_underscore_fields (type, 1); + return TYPE_NFIELDS (type) > 0 && rust_underscore_fields (type); } /* Return true if TYPE is a slice type, otherwise false. */ @@ -516,7 +319,7 @@ rust_val_print_str (struct ui_file *stream, struct value *val, options); } -/* rust_print_type branch for structs and untagged unions. */ +/* rust_val_print helper for structs and untagged unions. */ static void val_print_struct (struct type *type, int embedded_offset, @@ -601,6 +404,69 @@ val_print_struct (struct type *type, int embedded_offset, fputs_filtered ("}", stream); } +/* rust_val_print helper for discriminated unions (Rust enums). */ + +static void +rust_print_enum (struct type *type, int embedded_offset, + CORE_ADDR address, struct ui_file *stream, + int recurse, struct value *val, + const struct value_print_options *options) +{ + struct value_print_options opts = *options; + + opts.deref_ref = 0; + + const gdb_byte *valaddr = value_contents_for_printing (val); + struct field *variant_field = rust_enum_variant (type, valaddr); + embedded_offset += FIELD_BITPOS (*variant_field) / 8; + struct type *variant_type = FIELD_TYPE (*variant_field); + + int nfields = TYPE_NFIELDS (variant_type); + + bool is_tuple = rust_tuple_struct_type_p (variant_type); + + fprintf_filtered (stream, "%s", TYPE_NAME (variant_type)); + if (nfields == 0) + { + /* In case of a nullary variant like 'None', just output + the name. */ + return; + } + + /* In case of a non-nullary variant, we output 'Foo(x,y,z)'. */ + if (is_tuple) + fprintf_filtered (stream, "("); + else + { + /* struct variant. */ + fprintf_filtered (stream, "{"); + } + + bool first_field = true; + for (int j = 0; j < TYPE_NFIELDS (variant_type); j++) + { + if (!first_field) + fputs_filtered (", ", stream); + first_field = false; + + if (!is_tuple) + fprintf_filtered (stream, "%s: ", + TYPE_FIELD_NAME (variant_type, j)); + + val_print (TYPE_FIELD_TYPE (variant_type, j), + (embedded_offset + + TYPE_FIELD_BITPOS (variant_type, j) / 8), + address, + stream, recurse + 1, val, &opts, + current_language); + } + + if (is_tuple) + fputs_filtered (")", stream); + else + fputs_filtered ("}", stream); +} + static const struct generic_val_print_decorations rust_decorations = { /* Complex isn't used in Rust, but we provide C-ish values just in @@ -707,93 +573,22 @@ rust_val_print (struct type *type, int embedded_offset, break; case TYPE_CODE_UNION: - { - int j, nfields, first_field, is_tuple, start; - struct type *variant_type; - struct disr_info disr; - struct value_print_options opts; - - /* Untagged unions are printed as if they are structs. - Since the field bit positions overlap in the debuginfo, - the code for printing a union is same as that for a struct, - the only difference is that the input type will have overlapping - fields. */ - if (rust_union_is_untagged (type)) - { - val_print_struct (type, embedded_offset, address, stream, - recurse, val, options); - break; - } - - opts = *options; - opts.deref_ref = 0; - - disr = rust_get_disr_info (type, valaddr, embedded_offset, address, - val); - - if (disr.is_encoded && disr.field_no == RUST_ENCODED_ENUM_HIDDEN) - { - fprintf_filtered (stream, "%s", disr.name.c_str ()); - break; - } - - first_field = 1; - variant_type = TYPE_FIELD_TYPE (type, disr.field_no); - nfields = TYPE_NFIELDS (variant_type); - - is_tuple = (disr.is_encoded - ? rust_tuple_struct_type_p (variant_type) - : rust_tuple_variant_type_p (variant_type)); - start = disr.is_encoded ? 0 : 1; - - if (nfields > start) - { - /* In case of a non-nullary variant, we output 'Foo(x,y,z)'. */ - if (is_tuple) - fprintf_filtered (stream, "%s(", disr.name.c_str ()); - else - { - /* struct variant. */ - fprintf_filtered (stream, "%s{", disr.name.c_str ()); - } - } - else - { - /* In case of a nullary variant like 'None', just output - the name. */ - fprintf_filtered (stream, "%s", disr.name.c_str ()); - break; - } - - for (j = start; j < TYPE_NFIELDS (variant_type); j++) - { - if (!first_field) - fputs_filtered (", ", stream); - first_field = 0; - - if (!is_tuple) - fprintf_filtered (stream, "%s: ", - TYPE_FIELD_NAME (variant_type, j)); - - val_print (TYPE_FIELD_TYPE (variant_type, j), - (embedded_offset - + TYPE_FIELD_BITPOS (type, disr.field_no) / 8 - + TYPE_FIELD_BITPOS (variant_type, j) / 8), - address, - stream, recurse + 1, val, &opts, - current_language); - } - - if (is_tuple) - fputs_filtered (")", stream); - else - fputs_filtered ("}", stream); - } + /* Untagged unions are printed as if they are structs. Since + the field bit positions overlap in the debuginfo, the code + for printing a union is same as that for a struct, the only + difference is that the input type will have overlapping + fields. */ + val_print_struct (type, embedded_offset, address, stream, + recurse, val, options); break; case TYPE_CODE_STRUCT: - val_print_struct (type, embedded_offset, address, stream, - recurse, val, options); + if (rust_enum_p (type)) + rust_print_enum (type, embedded_offset, address, stream, + recurse, val, options); + else + val_print_struct (type, embedded_offset, address, stream, + recurse, val, options); break; default: @@ -807,19 +602,18 @@ rust_val_print (struct type *type, int embedded_offset, \f static void -rust_print_type (struct type *type, const char *varstring, - struct ui_file *stream, int show, int level, - const struct type_print_options *flags); +rust_internal_print_type (struct type *type, const char *varstring, + struct ui_file *stream, int show, int level, + const struct type_print_options *flags, + bool for_rust_enum); /* Print a struct or union typedef. */ static void rust_print_struct_def (struct type *type, const char *varstring, struct ui_file *stream, int show, int level, - const struct type_print_options *flags) + const struct type_print_options *flags, + bool for_rust_enum) { - bool is_tuple_struct; - int i; - /* Print a tuple type simply. */ if (rust_tuple_type_p (type)) { @@ -831,22 +625,57 @@ rust_print_struct_def (struct type *type, const char *varstring, if (TYPE_N_BASECLASSES (type) > 0) c_print_type (type, varstring, stream, show, level, flags); - /* This code path is also used by unions. */ - if (TYPE_CODE (type) == TYPE_CODE_STRUCT) - fputs_filtered ("struct ", stream); - else - fputs_filtered ("union ", stream); + /* Compute properties of TYPE here because, in the enum case, the + rest of the code ends up looking only at the variant part. */ + const char *tagname = TYPE_TAG_NAME (type); + bool is_tuple_struct = rust_tuple_struct_type_p (type); + bool is_tuple = rust_tuple_type_p (type); + bool is_enum = rust_enum_p (type); + bool is_univariant = false; + + int enum_discriminant_index = -1; - if (TYPE_TAG_NAME (type) != NULL) - fputs_filtered (TYPE_TAG_NAME (type), stream); + if (for_rust_enum) + { + /* Already printing an outer enum, so nothing to print here. */ + } + else + { + /* This code path is also used by unions and enums. */ + if (is_enum) + { + fputs_filtered ("enum ", stream); + type = TYPE_FIELD_TYPE (type, 0); + + struct dynamic_prop *discriminant_prop + = get_dyn_prop (DYN_PROP_DISCRIMINATED, type); + struct discriminant_info *info + = (struct discriminant_info *) discriminant_prop->data.baton; + enum_discriminant_index = info->discriminant_index; + } + else if (TYPE_CODE (type) == TYPE_CODE_UNION && TYPE_NFIELDS (type) == 1) + { + /* Probably a univariant enum. */ + fputs_filtered ("enum ", stream); + is_univariant = true; + } + else if (TYPE_CODE (type) == TYPE_CODE_STRUCT) + fputs_filtered ("struct ", stream); + else + fputs_filtered ("union ", stream); - is_tuple_struct = rust_tuple_struct_type_p (type); + if (tagname != NULL) + fputs_filtered (tagname, stream); + } - if (TYPE_NFIELDS (type) == 0 && !rust_tuple_type_p (type)) + if (TYPE_NFIELDS (type) == 0 && !is_tuple) return; - fputs_filtered (is_tuple_struct ? " (\n" : " {\n", stream); + if (for_rust_enum) + fputs_filtered (is_tuple_struct ? "(" : "{", stream); + else + fputs_filtered (is_tuple_struct ? " (\n" : " {\n", stream); - for (i = 0; i < TYPE_NFIELDS (type); ++i) + for (int i = 0; i < TYPE_NFIELDS (type); ++i) { QUIT; if (field_is_static (&TYPE_FIELD (type, i))) @@ -858,17 +687,36 @@ rust_print_struct_def (struct type *type, const char *varstring, /* For a tuple struct we print the type but nothing else. */ - print_spaces_filtered (level + 2, stream); - if (!is_tuple_struct) + if (!for_rust_enum) + print_spaces_filtered (level + 2, stream); + if (is_enum) + { + if (i == enum_discriminant_index) + continue; + fputs_filtered (TYPE_FIELD_NAME (type, i), stream); + } + else if (is_univariant) + { + const char *name + = rust_last_path_segment (TYPE_NAME (TYPE_FIELD_TYPE (type, i))); + fputs_filtered (name, stream); + } + else if (!is_tuple_struct) fprintf_filtered (stream, "%s: ", TYPE_FIELD_NAME (type, i)); - rust_print_type (TYPE_FIELD_TYPE (type, i), NULL, - stream, show - 1, level + 2, - flags); - fputs_filtered (",\n", stream); + rust_internal_print_type (TYPE_FIELD_TYPE (type, i), NULL, + stream, + ((is_enum || is_univariant) ? show : show - 1), + level + 2, flags, is_enum || is_univariant); + if (!for_rust_enum) + fputs_filtered (",\n", stream); + else if (i + 1 < TYPE_NFIELDS (type)) + fputs_filtered (", ", stream); } - fprintfi_filtered (level, stream, is_tuple_struct ? ")" : "}"); + if (!for_rust_enum) + print_spaces_filtered (level, stream); + fputs_filtered (is_tuple_struct ? ")" : "}", stream); } /* la_print_typedef implementation for Rust. */ @@ -887,9 +735,10 @@ rust_print_typedef (struct type *type, /* la_print_type implementation for Rust. */ static void -rust_print_type (struct type *type, const char *varstring, - struct ui_file *stream, int show, int level, - const struct type_print_options *flags) +rust_internal_print_type (struct type *type, const char *varstring, + struct ui_file *stream, int show, int level, + const struct type_print_options *flags, + bool for_rust_enum) { int i; @@ -910,7 +759,11 @@ rust_print_type (struct type *type, const char *varstring, switch (TYPE_CODE (type)) { case TYPE_CODE_VOID: - fputs_filtered ("()", stream); + /* If we have an enum, we've already printed the type's + unqualified name, and there is nothing else to print + here. */ + if (!for_rust_enum) + fputs_filtered ("()", stream); break; case TYPE_CODE_FUNC: @@ -927,15 +780,16 @@ rust_print_type (struct type *type, const char *varstring, QUIT; if (i > 0) fputs_filtered (", ", stream); - rust_print_type (TYPE_FIELD_TYPE (type, i), "", stream, -1, 0, - flags); + rust_internal_print_type (TYPE_FIELD_TYPE (type, i), "", stream, + -1, 0, flags, false); } fputs_filtered (")", stream); /* If it returns unit, we can omit the return type. */ if (TYPE_CODE (TYPE_TARGET_TYPE (type)) != TYPE_CODE_VOID) { fputs_filtered (" -> ", stream); - rust_print_type (TYPE_TARGET_TYPE (type), "", stream, -1, 0, flags); + rust_internal_print_type (TYPE_TARGET_TYPE (type), "", stream, + -1, 0, flags, false); } break; @@ -944,8 +798,8 @@ rust_print_type (struct type *type, const char *varstring, LONGEST low_bound, high_bound; fputs_filtered ("[", stream); - rust_print_type (TYPE_TARGET_TYPE (type), NULL, - stream, show - 1, level, flags); + rust_internal_print_type (TYPE_TARGET_TYPE (type), NULL, + stream, show - 1, level, flags, false); if (TYPE_HIGH_BOUND_KIND (TYPE_INDEX_TYPE (type)) == PROP_LOCEXPR || TYPE_HIGH_BOUND_KIND (TYPE_INDEX_TYPE (type)) == PROP_LOCLIST) @@ -957,8 +811,10 @@ rust_print_type (struct type *type, const char *varstring, } break; + case TYPE_CODE_UNION: case TYPE_CODE_STRUCT: - rust_print_struct_def (type, varstring, stream, show, level, flags); + rust_print_struct_def (type, varstring, stream, show, level, flags, + for_rust_enum); break; case TYPE_CODE_ENUM: @@ -992,93 +848,21 @@ rust_print_type (struct type *type, const char *varstring, } break; - case TYPE_CODE_UNION: - { - /* ADT enums. */ - int i; - /* Skip the discriminant field. */ - int skip_to = 1; - - /* Unions and structs have the same syntax in Rust, - the only difference is that structs are declared with `struct` - and union with `union`. This difference is handled in the struct - printer. */ - if (rust_union_is_untagged (type)) - { - rust_print_struct_def (type, varstring, stream, show, level, flags); - break; - } - - fputs_filtered ("enum ", stream); - if (TYPE_TAG_NAME (type) != NULL) - { - fputs_filtered (TYPE_TAG_NAME (type), stream); - fputs_filtered (" ", stream); - } - fputs_filtered ("{\n", stream); - - if (strncmp (TYPE_FIELD_NAME (type, 0), RUST_ENUM_PREFIX, - strlen (RUST_ENUM_PREFIX)) == 0) - { - const char *zero_field = strrchr (TYPE_FIELD_NAME (type, 0), '$'); - if (zero_field != NULL && strlen (zero_field) > 1) - { - fprintfi_filtered (level + 2, stream, "%s,\n", zero_field + 1); - /* There is no explicit discriminant field, skip nothing. */ - skip_to = 0; - } - } - else if (TYPE_NFIELDS (type) == 1) - skip_to = 0; - - for (i = 0; i < TYPE_NFIELDS (type); ++i) - { - struct type *variant_type = TYPE_FIELD_TYPE (type, i); - const char *name - = rust_last_path_segment (TYPE_NAME (variant_type)); - - fprintfi_filtered (level + 2, stream, "%s", name); - - if (TYPE_NFIELDS (variant_type) > skip_to) - { - int first = 1; - bool is_tuple = (TYPE_NFIELDS (type) == 1 - ? rust_tuple_struct_type_p (variant_type) - : rust_tuple_variant_type_p (variant_type)); - int j; - - fputs_filtered (is_tuple ? "(" : "{", stream); - for (j = skip_to; j < TYPE_NFIELDS (variant_type); j++) - { - if (first) - first = 0; - else - fputs_filtered (", ", stream); - - if (!is_tuple) - fprintf_filtered (stream, "%s: ", - TYPE_FIELD_NAME (variant_type, j)); - - rust_print_type (TYPE_FIELD_TYPE (variant_type, j), NULL, - stream, show - 1, level + 2, - flags); - } - fputs_filtered (is_tuple ? ")" : "}", stream); - } - - fputs_filtered (",\n", stream); - } - - fputs_filtered ("}", stream); - } - break; - default: c_printer: c_print_type (type, varstring, stream, show, level, flags); } } +static void +rust_print_type (struct type *type, const char *varstring, + struct ui_file *stream, int show, int level, + const struct type_print_options *flags) +{ + rust_internal_print_type (type, varstring, stream, show, level, + flags, false); +} + \f /* Compute the alignment of the type T. */ @@ -1800,7 +1584,6 @@ rust_evaluate_subexp (struct type *expect_type, struct expression *exp, struct value *lhs; int pc, field_number, nfields; struct type *type, *variant_type; - struct disr_info disr; pc = (*pos)++; field_number = longest_to_int (exp->elts[pc + 1].longconst); @@ -1808,58 +1591,64 @@ rust_evaluate_subexp (struct type *expect_type, struct expression *exp, lhs = evaluate_subexp (NULL_TYPE, exp, pos, noside); type = value_type (lhs); - /* Untagged unions can't have anonymous field access since - they can only have named fields. */ - if (TYPE_CODE (type) == TYPE_CODE_UNION - && !rust_union_is_untagged (type)) + + /* Treat a univariant union as if it were an enum. */ + if (TYPE_CODE (type) == TYPE_CODE_UNION && TYPE_NFIELDS (type) == 1) { - disr = rust_get_disr_info (type, value_contents (lhs), - value_embedded_offset (lhs), - value_address (lhs), lhs); + lhs = value_primitive_field (lhs, 0, 0, type); + type = value_type (lhs); + } - if (disr.is_encoded && disr.field_no == RUST_ENCODED_ENUM_HIDDEN) - { - variant_type = NULL; - nfields = 0; - } - else + if (TYPE_CODE (type) == TYPE_CODE_STRUCT) + { + struct type *outer_type = NULL; + + if (rust_enum_p (type)) { - variant_type = TYPE_FIELD_TYPE (type, disr.field_no); - nfields = TYPE_NFIELDS (variant_type); + const gdb_byte *valaddr = value_contents (lhs); + struct field *variant_field = rust_enum_variant (type, valaddr); + + struct value *union_value = value_primitive_field (lhs, 0, 0, + type); + + int fieldno = (variant_field + - &TYPE_FIELD (value_type (union_value), 0)); + lhs = value_primitive_field (union_value, 0, fieldno, + value_type (union_value)); + outer_type = type; + type = value_type (lhs); } - if (!disr.is_encoded) - ++field_number; - - if (field_number >= nfields || field_number < 0) - error(_("Cannot access field %d of variant %s, \ -there are only %d fields"), - disr.is_encoded ? field_number : field_number - 1, - disr.name.c_str (), - disr.is_encoded ? nfields : nfields - 1); - - if (!(disr.is_encoded - ? rust_tuple_struct_type_p (variant_type) - : rust_tuple_variant_type_p (variant_type))) - error(_("Variant %s is not a tuple variant"), disr.name.c_str ()); - - result = value_primitive_field (lhs, 0, field_number, - variant_type); - } - else if (TYPE_CODE (type) == TYPE_CODE_STRUCT) - { /* Tuples and tuple structs */ - nfields = TYPE_NFIELDS(type); + nfields = TYPE_NFIELDS (type); if (field_number >= nfields || field_number < 0) - error(_("Cannot access field %d of %s, there are only %d fields"), - field_number, TYPE_TAG_NAME (type), nfields); + { + if (outer_type != NULL) + error(_("Cannot access field %d of variant %s::%s, " + "there are only %d fields"), + field_number, TYPE_TAG_NAME (outer_type), + rust_last_path_segment (TYPE_TAG_NAME (type)), + nfields); + else + error(_("Cannot access field %d of %s, " + "there are only %d fields"), + field_number, TYPE_TAG_NAME (type), nfields); + } /* Tuples are tuple structs too. */ if (!rust_tuple_struct_type_p (type)) - error(_("Attempting to access anonymous field %d of %s, which is \ -not a tuple, tuple struct, or tuple-like variant"), - field_number, TYPE_TAG_NAME (type)); + { + if (outer_type != NULL) + error(_("Variant %s::%s is not a tuple variant"), + TYPE_TAG_NAME (outer_type), + rust_last_path_segment (TYPE_TAG_NAME (type))); + else + error(_("Attempting to access anonymous field %d " + "of %s, which is not a tuple, tuple struct, or " + "tuple-like variant"), + field_number, TYPE_TAG_NAME (type)); + } result = value_primitive_field (lhs, 0, field_number, type); } @@ -1882,53 +1671,44 @@ tuple structs, and tuple-like enum variants")); const char *field_name = &exp->elts[pc + 2].string; type = value_type (lhs); - if (TYPE_CODE (type) == TYPE_CODE_UNION - && !rust_union_is_untagged (type)) + if (TYPE_CODE (type) == TYPE_CODE_STRUCT && rust_enum_p (type)) { - int i, start; - struct disr_info disr; - struct type *variant_type; - - disr = rust_get_disr_info (type, value_contents (lhs), - value_embedded_offset (lhs), - value_address (lhs), lhs); - - if (disr.is_encoded && disr.field_no == RUST_ENCODED_ENUM_HIDDEN) - error(_("Could not find field %s of struct variant %s"), - field_name, disr.name.c_str ()); - - variant_type = TYPE_FIELD_TYPE (type, disr.field_no); - - if (variant_type == NULL - || (disr.is_encoded - ? rust_tuple_struct_type_p (variant_type) - : rust_tuple_variant_type_p (variant_type))) - error(_("Attempting to access named field %s of tuple variant %s, \ -which has only anonymous fields"), - field_name, disr.name.c_str ()); - - start = disr.is_encoded ? 0 : 1; - for (i = start; i < TYPE_NFIELDS (variant_type); i++) + const gdb_byte *valaddr = value_contents (lhs); + struct field *variant_field = rust_enum_variant (type, valaddr); + + struct value *union_value = value_primitive_field (lhs, 0, 0, + type); + + int fieldno = (variant_field + - &TYPE_FIELD (value_type (union_value), 0)); + lhs = value_primitive_field (union_value, 0, fieldno, + value_type (union_value)); + + struct type *outer_type = type; + type = value_type (lhs); + if (rust_tuple_type_p (type) || rust_tuple_struct_type_p (type)) + error (_("Attempting to access named field foo of tuple " + "variant %s::%s, which has only anonymous fields"), + TYPE_TAG_NAME (outer_type), + rust_last_path_segment (TYPE_NAME (type))); + + TRY { - if (strcmp (TYPE_FIELD_NAME (variant_type, i), - field_name) == 0) { - result = value_primitive_field (lhs, 0, i, variant_type); - break; - } + result = value_struct_elt (&lhs, NULL, field_name, + NULL, "structure"); } - - if (i == TYPE_NFIELDS (variant_type)) - /* We didn't find it. */ - error(_("Could not find field %s of struct variant %s"), - field_name, disr.name.c_str ()); + CATCH (except, RETURN_MASK_ERROR) + { + error (_("Could not find field %s of struct variant %s::%s"), + field_name, TYPE_TAG_NAME (outer_type), + rust_last_path_segment (TYPE_NAME (type))); + } + END_CATCH } else - { - result = value_struct_elt (&lhs, NULL, field_name, NULL, - "structure"); - if (noside == EVAL_AVOID_SIDE_EFFECTS) - result = value_zero (value_type (result), VALUE_LVAL (result)); - } + result = value_struct_elt (&lhs, NULL, field_name, NULL, "structure"); + if (noside == EVAL_AVOID_SIDE_EFFECTS) + result = value_zero (value_type (result), VALUE_LVAL (result)); } break; diff --git a/gdb/rust-lang.h b/gdb/rust-lang.h index 554a440d58..ce30e33276 100644 --- a/gdb/rust-lang.h +++ b/gdb/rust-lang.h @@ -39,6 +39,11 @@ extern bool rust_tuple_struct_type_p (struct type *type); stringif no crate name can be found. */ extern std::string rust_crate_for_block (const struct block *block); +/* Returns the last segment of a Rust path like foo::bar::baz. Will + not handle cases where the last segment contains generics. */ + +extern const char *rust_last_path_segment (const char *path); + /* Create a new slice type. NAME is the name of the type. ELT_TYPE is the type of the elements of the slice. USIZE_TYPE is the Rust "usize" type to use. The new type is allocated whereever ELT_TYPE diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 7ee3e0b835..9510dcff39 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,7 @@ +2018-02-22 Tom Tromey <tom@tromey.com> + + * gdb.rust/simple.exp: Accept more possible results in enum test. + 2018-02-21 John Baldwin <jhb@FreeBSD.org> * gdb.arch/amd64-i386-address.exp: Fix a typo. diff --git a/gdb/testsuite/gdb.rust/simple.exp b/gdb/testsuite/gdb.rust/simple.exp index 04750d9eb1..230e6a7a88 100644 --- a/gdb/testsuite/gdb.rust/simple.exp +++ b/gdb/testsuite/gdb.rust/simple.exp @@ -220,8 +220,8 @@ gdb_test "print .." " = .*::ops::RangeFull" gdb_test "print str_some" \ " = core::option::Option<\[a-z\]+::string::String>::Some\\(\[a-z\]+::string::String .*" gdb_test "print str_none" " = core::option::Option<\[a-z\]+::string::String>::None" -gdb_test "print int_some" " = core::option::Option::Some\\(1\\)" -gdb_test "print int_none" " = core::option::Option::None" +gdb_test "print int_some" " = core::option::Option<u8>::Some\\(1\\)" +gdb_test "print int_none" " = core::option::Option<u8>::None" gdb_test "print box_some" " = core::option::Option<\[a-z:\]*Box<u8>>::Some\\(.*\\)" gdb_test "print box_none" " = core::option::Option<\[a-z:\]*Box<u8>>::None" gdb_test "print custom_some" \ -- 2.13.6 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFA v2 3/4] Convert Rust to use discriminated unions 2018-02-22 20:30 ` [RFA v2 3/4] Convert Rust to use discriminated unions Tom Tromey @ 2018-02-26 6:50 ` Joel Brobecker 2018-02-27 23:23 ` Pedro Alves 2018-04-10 20:36 ` -readnow crash Rust regression [Re: [RFA v2 3/4] Convert Rust to use discriminated unions] Jan Kratochvil 2 siblings, 0 replies; 18+ messages in thread From: Joel Brobecker @ 2018-02-26 6:50 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches > 2018-02-22 Tom Tromey <tom@tromey.com> > > * rust-lang.h (rust_last_path_segment): Declare. > * rust-lang.c (rust_last_path_segment): Now public. Change > contract. > (struct disr_info): Remove. > (RUST_ENUM_PREFIX, RUST_ENCODED_ENUM_REAL) > (RUST_ENCODED_ENUM_HIDDEN, rust_union_is_untagged) > (rust_get_disr_info, rust_tuple_variant_type_p): Remove. > (rust_enum_p, rust_enum_variant): New function. > (rust_underscore_fields): Remove "offset" parameter. > (rust_print_enum): New function. > (rust_val_print) <TYPE_CODE_UNION>: Remove enum code. > <TYPE_CODE_STRUCT>: Call rust_print_enum when appropriate. > (rust_print_struct_def): Add "for_rust_enum" parameter. Handle > enums. > (rust_internal_print_type): New function, from rust_print_type. > Remove enum code. > (rust_print_type): Call rust_internal_print_type. > (rust_evaluate_subexp) <STRUCTOP_ANONYMOUS, STRUCTOP_STRUCT>: > Update enum handling. > * dwarf2read.c (struct dwarf2_cu) <rust_unions>: New field. > (rust_fully_qualify, alloc_discriminant_info, quirk_rust_enum) > (rust_union_quirks): New functions. > (process_full_comp_unit, process_full_type_unit): Call > rust_union_quirks. > (process_structure_scope): Update rust_unions if necessary. > > 2018-02-22 Tom Tromey <tom@tromey.com> > > * gdb.rust/simple.exp: Accept more possible results in enum test. Looks good to me. Thanks Tom. -- Joel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFA v2 3/4] Convert Rust to use discriminated unions 2018-02-22 20:30 ` [RFA v2 3/4] Convert Rust to use discriminated unions Tom Tromey 2018-02-26 6:50 ` Joel Brobecker @ 2018-02-27 23:23 ` Pedro Alves 2018-02-28 0:23 ` Tom Tromey 2018-04-10 20:36 ` -readnow crash Rust regression [Re: [RFA v2 3/4] Convert Rust to use discriminated unions] Jan Kratochvil 2 siblings, 1 reply; 18+ messages in thread From: Pedro Alves @ 2018-02-27 23:23 UTC (permalink / raw) To: Tom Tromey, gdb-patches Hi Tom, I was comparing test results from 20180207 and today, I noticed that gdb.rust/simple.exp regressed: ~~~~~~ Running src/gdb/testsuite/gdb.rust/simple.exp ... -PASS: gdb.rust/simple.exp: print a -PASS: gdb.rust/simple.exp: ptype a -PASS: gdb.rust/simple.exp: print sizeof(a) -PASS: gdb.rust/simple.exp: print b (...) +UNTESTED: gdb.rust/simple.exp: could not run to breakpoint ~~~~~~ Looking at gdb.log I see: ~~~~~~~~~~~~~~~~~~~~~~ gdb) break simple.rs:152 src/gdb/dwarf2read.c:10367: internal-error: discriminant_info* alloc_discriminant_info(type*, int, int): Assertion `default_index == -1 || (default_index > 0 && default_index < TYPE_NFIELDS (type))' failed. A problem internal to GDB has been detected, further debugging may prove unreliable. Quit this debugging session? (y or n) Resyncing due to internal error. n This is a bug, please report it. For instructions, see: <http://www.gnu.org/software/gdb/bugs/>. src/gdb/dwarf2read.c:10367: internal-error: discriminant_info* alloc_discriminant_info(type*, int, int): Assertion `default_index == -1 || (default_index > 0 && default_index < TYPE_NFIELDS (type))' failed. A problem internal to GDB has been detected, further debugging may prove unreliable. Create a core file of GDB? (y or n) n Command aborted. (gdb) UNTESTED: gdb.rust/simple.exp: could not run to breakpointt ~~~~~~~~~~~~~~~~~~~~~~ I haven't bisected, but the references to discriminant info make me suspect this patch I'm replying to. This is Fedora 27 with: $ rustc --version rustc 1.13.0 (2c6933acc 2016-11-07) Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFA v2 3/4] Convert Rust to use discriminated unions 2018-02-27 23:23 ` Pedro Alves @ 2018-02-28 0:23 ` Tom Tromey 0 siblings, 0 replies; 18+ messages in thread From: Tom Tromey @ 2018-02-28 0:23 UTC (permalink / raw) To: Pedro Alves; +Cc: Tom Tromey, gdb-patches >>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes: Pedro> I was comparing test results from 20180207 and today, I Pedro> noticed that gdb.rust/simple.exp regressed: Thanks. Pedro> I haven't bisected, but the references to discriminant info make me suspect Pedro> this patch I'm replying to. Yeah, it has to be. Pedro> This is Fedora 27 with: Pedro> $ rustc --version Pedro> rustc 1.13.0 (2c6933acc 2016-11-07) I will look into it tomorrow. Tom ^ permalink raw reply [flat|nested] 18+ messages in thread
* -readnow crash Rust regression [Re: [RFA v2 3/4] Convert Rust to use discriminated unions] 2018-02-22 20:30 ` [RFA v2 3/4] Convert Rust to use discriminated unions Tom Tromey 2018-02-26 6:50 ` Joel Brobecker 2018-02-27 23:23 ` Pedro Alves @ 2018-04-10 20:36 ` Jan Kratochvil 2018-04-11 2:52 ` Tom Tromey 2 siblings, 1 reply; 18+ messages in thread From: Jan Kratochvil @ 2018-04-10 20:36 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches Hi Tom, c9317f214b274b805190b8e878c79f4181d93bb4 is the first bad commit commit c9317f214b274b805190b8e878c79f4181d93bb4 Author: Tom Tromey <tom@tromey.com> Date: Fri Feb 9 13:31:51 2018 -0700 Convert Rust to use discriminated unions https://people.redhat.com/jkratoch/rustgdbbug.tar.xz ./gdb/gdb -batch -readnow /tmp/rustgdbbug/a/b/rustdoc-1.24.0-2.fc27.x86_64.debug Segmentation fault Jan ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: -readnow crash Rust regression [Re: [RFA v2 3/4] Convert Rust to use discriminated unions] 2018-04-10 20:36 ` -readnow crash Rust regression [Re: [RFA v2 3/4] Convert Rust to use discriminated unions] Jan Kratochvil @ 2018-04-11 2:52 ` Tom Tromey 2018-04-11 7:04 ` Jan Kratochvil 0 siblings, 1 reply; 18+ messages in thread From: Tom Tromey @ 2018-04-11 2:52 UTC (permalink / raw) To: Jan Kratochvil; +Cc: Tom Tromey, gdb-patches >>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes: Jan> Hi Tom, Jan> c9317f214b274b805190b8e878c79f4181d93bb4 is the first bad commit Jan> commit c9317f214b274b805190b8e878c79f4181d93bb4 Jan> Author: Tom Tromey <tom@tromey.com> Jan> Date: Fri Feb 9 13:31:51 2018 -0700 Jan> Convert Rust to use discriminated unions Jan> https://people.redhat.com/jkratoch/rustgdbbug.tar.xz Jan> ./gdb/gdb -batch -readnow /tmp/rustgdbbug/a/b/rustdoc-1.24.0-2.fc27.x86_64.debug Jan> Segmentation fault Try this: https://sourceware.org/ml/gdb-patches/2018-03/msg00600.html Tom ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: -readnow crash Rust regression [Re: [RFA v2 3/4] Convert Rust to use discriminated unions] 2018-04-11 2:52 ` Tom Tromey @ 2018-04-11 7:04 ` Jan Kratochvil 2018-04-11 19:49 ` Tom Tromey 0 siblings, 1 reply; 18+ messages in thread From: Jan Kratochvil @ 2018-04-11 7:04 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches On Wed, 11 Apr 2018 04:52:09 +0200, Tom Tromey wrote: > >>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes: > Jan> ./gdb/gdb -batch -readnow /tmp/rustgdbbug/a/b/rustdoc-1.24.0-2.fc27.x86_64.debug > Jan> Segmentation fault > > Try this: > > https://sourceware.org/ml/gdb-patches/2018-03/msg00600.html Still crashing: #0 __strrchr_avx2 () at ../sysdeps/x86_64/multiarch/strrchr-avx2.S:54 #1 0x0000000000aaff44 in rust_last_path_segment (path=0x0) at rust-lang.c:44 #2 0x00000000008ca07d in quirk_rust_enum (type=0x4c53b80, objfile=0x3089a00) at dwarf2read.c:10076 #3 0x00000000008ca3cd in rust_union_quirks (cu=0x30fc820) at dwarf2read.c:10101 #4 0x00000000008cab85 in process_full_comp_unit (per_cu=0x3149f10, pretend_language=language_minimal) at dwarf2read.c:10289 #5 0x00000000008c7cfa in process_queue (dwarf2_per_objfile=0x329f5d0) at dwarf2read.c:9501 #6 0x00000000008b4edb in dw2_do_instantiate_symtab (per_cu=0x3149f10) at dwarf2read.c:2888 #7 0x00000000008b4fd2 in dw2_instantiate_symtab (per_cu=0x3149f10) at dwarf2read.c:2909 #8 0x00000000008b9566 in dw2_expand_all_symtabs (objfile=0x3089a00) at dwarf2read.c:4147 Jan ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: -readnow crash Rust regression [Re: [RFA v2 3/4] Convert Rust to use discriminated unions] 2018-04-11 7:04 ` Jan Kratochvil @ 2018-04-11 19:49 ` Tom Tromey 2018-04-12 18:10 ` Tom Tromey 0 siblings, 1 reply; 18+ messages in thread From: Tom Tromey @ 2018-04-11 19:49 UTC (permalink / raw) To: Jan Kratochvil; +Cc: Tom Tromey, gdb-patches >>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes: Jan> On Wed, 11 Apr 2018 04:52:09 +0200, Tom Tromey wrote: >> >>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes: Jan> ./gdb/gdb -batch -readnow /tmp/rustgdbbug/a/b/rustdoc-1.24.0-2.fc27.x86_64.debug Jan> Segmentation fault >> >> Try this: >> >> https://sourceware.org/ml/gdb-patches/2018-03/msg00600.html Jan> Still crashing: Jan> #0 __strrchr_avx2 () at ../sysdeps/x86_64/multiarch/strrchr-avx2.S:54 Jan> #1 0x0000000000aaff44 in rust_last_path_segment (path=0x0) at rust-lang.c:44 Jan> #2 0x00000000008ca07d in quirk_rust_enum (type=0x4c53b80, objfile=0x3089a00) at dwarf2read.c:10076 Jan> #3 0x00000000008ca3cd in rust_union_quirks (cu=0x30fc820) at dwarf2read.c:10101 Jan> #4 0x00000000008cab85 in process_full_comp_unit (per_cu=0x3149f10, I think this another variant of https://sourceware.org/bugzilla/show_bug.cgi?id=23010 The immediate bug here is that the (rust) enum member is being read from a partial unit, but it is being read as language_minimal, not language_rust. This causes it to have a TYPE_NAME==NULL, whereas language_rust would have set it to follow TYPE_TAG_NAME. I have a hack to fix that but it reveals another problem, which is that some of the type rewriting can be done multiple times, causing other bugs. I hadn't considered this possibility. I'm looking into a good way to fix it. Tom ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: -readnow crash Rust regression [Re: [RFA v2 3/4] Convert Rust to use discriminated unions] 2018-04-11 19:49 ` Tom Tromey @ 2018-04-12 18:10 ` Tom Tromey 2018-04-12 18:45 ` Keith Seitz 0 siblings, 1 reply; 18+ messages in thread From: Tom Tromey @ 2018-04-12 18:10 UTC (permalink / raw) To: Tom Tromey; +Cc: Jan Kratochvil, gdb-patches >>>>> "Tom" == Tom Tromey <tom@tromey.com> writes: Tom> I have a hack to fix that but it reveals another problem, which is that Tom> some of the type rewriting can be done multiple times, causing other bugs. Tom> I hadn't considered this possibility. I'm looking into a good way to Tom> fix it. This analysis was mistaken, and it turned out to be simpler -- good news. I will send out my patches, but writing a test case for 23010 seems quite painful. Based on the stack trace I got, and investigating the DWARF of the reproducer, you need: * A partial CU with no language, which has a function that has some parameters * A full C++ CU that references that function using DW_AT_abstract_origin In the webkit reproducer this DW_AT_abstract_origin occurs in an inlined function IIRC. Tom ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: -readnow crash Rust regression [Re: [RFA v2 3/4] Convert Rust to use discriminated unions] 2018-04-12 18:10 ` Tom Tromey @ 2018-04-12 18:45 ` Keith Seitz 0 siblings, 0 replies; 18+ messages in thread From: Keith Seitz @ 2018-04-12 18:45 UTC (permalink / raw) To: gdb-patches On 04/12/2018 11:09 AM, Tom Tromey wrote: >>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes: > > Tom> I have a hack to fix that but it reveals another problem, which is that > Tom> some of the type rewriting can be done multiple times, causing other bugs. > Tom> I hadn't considered this possibility. I'm looking into a good way to > Tom> fix it. > > This analysis was mistaken, and it turned out to be simpler -- good > news. This is excellent! I am looking forward to reading about this. > I will send out my patches, but writing a test case for 23010 seems > quite painful. Based on the stack trace I got, and investigating the > DWARF of the reproducer, you need: > > * A partial CU with no language, which has a function that has some parameters > * A full C++ CU that references that function using DW_AT_abstract_origin > > In the webkit reproducer this DW_AT_abstract_origin occurs in an inlined > function IIRC. It /is/ painful. Very. I have extracted (most of?) the DIE tree from the webkit reproducer, but it does not reproduce the problem, so I am missing something in my test. Maybe it's the inlined function... [If you'd like it, just lemme know. I may yet play with it some more (re: inlined function).] Keith ^ permalink raw reply [flat|nested] 18+ messages in thread
* [RFA v2 2/4] Initial support for variant parts 2018-02-22 20:30 [RFA v2 0/4] variants and variant parts Tom Tromey ` (2 preceding siblings ...) 2018-02-22 20:30 ` [RFA v2 3/4] Convert Rust to use discriminated unions Tom Tromey @ 2018-02-22 20:30 ` Tom Tromey 2018-02-26 6:49 ` Joel Brobecker 3 siblings, 1 reply; 18+ messages in thread From: Tom Tromey @ 2018-02-22 20:30 UTC (permalink / raw) To: gdb-patches; +Cc: Tom Tromey This adds some initial support for variant parts to gdbtypes.h. A variant part is represented as a union. The union has a flag indicating that it has a discriminant, and information about the discriminant is attached using the dynamic property system. 2018-02-22 Tom Tromey <tom@tromey.com> * value.h (value_union_variant): Declare. * valops.c (value_union_variant): New function. * gdbtypes.h (TYPE_FLAG_DISCRIMINATED_UNION): New macro. (struct discriminant_info): New. (enum dynamic_prop_node_kind) <DYN_PROP_DISCRIMINATED>: New enumerator. (struct main_type) <flag_discriminated_union>: New field. --- gdb/ChangeLog | 10 ++++++++++ gdb/gdbtypes.h | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++ gdb/valops.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ gdb/value.h | 8 ++++++++ 4 files changed, 113 insertions(+) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 734bbdf832..2b9d886b76 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,5 +1,15 @@ 2018-02-22 Tom Tromey <tom@tromey.com> + * value.h (value_union_variant): Declare. + * valops.c (value_union_variant): New function. + * gdbtypes.h (TYPE_FLAG_DISCRIMINATED_UNION): New macro. + (struct discriminant_info): New. + (enum dynamic_prop_node_kind) <DYN_PROP_DISCRIMINATED>: New + enumerator. + (struct main_type) <flag_discriminated_union>: New field. + +2018-02-22 Tom Tromey <tom@tromey.com> + * Makefile.in (SUBDIR_UNITTESTS_SRCS): Add unittests/unpack-selftests.c. * unittests/unpack-selftests.c: New file. diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h index 613257c47d..62cd8b6f82 100644 --- a/gdb/gdbtypes.h +++ b/gdb/gdbtypes.h @@ -304,6 +304,14 @@ DEF_ENUM_FLAGS_TYPE (enum type_instance_flag_value, type_instance_flags); #define TYPE_FLAG_ENUM(t) (TYPE_MAIN_TYPE (t)->flag_flag_enum) +/* * True if this type is a discriminated union type. Only valid for + TYPE_CODE_UNION. A discriminated union stores a reference to the + discriminant field along with the discriminator values in a dynamic + property. */ + +#define TYPE_FLAG_DISCRIMINATED_UNION(t) \ + (TYPE_MAIN_TYPE (t)->flag_discriminated_union) + /* * Constant type. If this is set, the corresponding type has a const modifier. */ @@ -373,6 +381,39 @@ DEF_ENUM_FLAGS_TYPE (enum type_instance_flag_value, type_instance_flags); #define TYPE_ADDRESS_CLASS_ALL(t) (TYPE_INSTANCE_FLAGS(t) \ & TYPE_INSTANCE_FLAG_ADDRESS_CLASS_ALL) +/* * Information needed for a discriminated union. A discriminated + union is handled somewhat differently from an ordinary union. + + One field is designated as the discriminant. Only one other field + is active at a time; which one depends on the value of the + discriminant and the data in this structure. + + Additionally, it is possible to have a univariant discriminated + union. In this case, the union has just a single field, which is + assumed to be the only active variant -- in this case no + discriminant is provided. */ + +struct discriminant_info +{ + /* * The index of the discriminant field. If -1, then this union + must have just a single field. */ + + int discriminant_index; + + /* * The index of the default branch of the union. If -1, then + there is no default branch. */ + + int default_index; + + /* * The discriminant values corresponding to each branch. This has + a number of entries equal to the number of fields in this union. + If discriminant_index is not -1, then that entry in this array is + not used. If default_index is not -1, then that entry in this + array is not used. */ + + ULONGEST discriminants[1]; +}; + enum dynamic_prop_kind { PROP_UNDEFINED, /* Not defined. */ @@ -431,6 +472,9 @@ enum dynamic_prop_node_kind /* A property providing an array's byte stride. */ DYN_PROP_BYTE_STRIDE, + + /* A property holding information about a discriminated union. */ + DYN_PROP_DISCRIMINATED, }; /* * List for dynamic type attributes. */ @@ -650,6 +694,13 @@ struct main_type unsigned int flag_flag_enum : 1; + /* * True if this type is a discriminated union type. Only valid + for TYPE_CODE_UNION. A discriminated union stores a reference to + the discriminant field along with the discriminator values in a + dynamic property. */ + + unsigned int flag_discriminated_union : 1; + /* * A discriminant telling us which field of the type_specific union is being used for this type, if any. */ diff --git a/gdb/valops.c b/gdb/valops.c index e038c04fd1..808a98beb7 100644 --- a/gdb/valops.c +++ b/gdb/valops.c @@ -2257,6 +2257,50 @@ value_struct_elt_bitpos (struct value **argp, int bitpos, struct type *ftype, return NULL; } +/* See value.h. */ + +int +value_union_variant (struct type *union_type, const gdb_byte *contents) +{ + gdb_assert (TYPE_CODE (union_type) == TYPE_CODE_UNION + && TYPE_FLAG_DISCRIMINATED_UNION (union_type)); + + struct dynamic_prop *discriminant_prop + = get_dyn_prop (DYN_PROP_DISCRIMINATED, union_type); + gdb_assert (discriminant_prop != nullptr); + + struct discriminant_info *info + = (struct discriminant_info *) discriminant_prop->data.baton; + gdb_assert (info != nullptr); + + /* If this is a univariant union, just return the sole field. */ + if (TYPE_NFIELDS (union_type) == 1) + return 0; + /* This should only happen for univariants, which we already dealt + with. */ + gdb_assert (info->discriminant_index != -1); + + /* Compute the discriminant. Note that unpack_field_as_long handles + sign extension when necessary, as does the DWARF reader -- so + signed discriminants will be handled correct despite the use of + an unsigned type here. */ + ULONGEST discriminant = unpack_field_as_long (union_type, contents, + info->discriminant_index); + + for (int i = 0; i < TYPE_NFIELDS (union_type); ++i) + { + if (i != info->default_index + && i != info->discriminant_index + && discriminant == info->discriminants[i]) + return i; + } + + if (info->default_index == -1) + error (_("Could not find variant corresponding to discriminant %s"), + pulongest (discriminant)); + return info->default_index; +} + /* Search through the methods of an object (and its bases) to find a specified method. Return the pointer to the fn_field list FN_LIST of overloaded instances defined in the source language. If available diff --git a/gdb/value.h b/gdb/value.h index 5676d245b9..0bc51304ea 100644 --- a/gdb/value.h +++ b/gdb/value.h @@ -1169,4 +1169,12 @@ extern struct type *result_type_of_xmethod (struct value *method, extern struct value *call_xmethod (struct value *method, int argc, struct value **argv); +/* Given a discriminated union type and some corresponding value + contents, this will return the field index of the currently active + variant. This will throw an exception if no active variant can be + found. */ + +extern int value_union_variant (struct type *union_type, + const gdb_byte *contents); + #endif /* !defined (VALUE_H) */ -- 2.13.6 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFA v2 2/4] Initial support for variant parts 2018-02-22 20:30 ` [RFA v2 2/4] Initial support for variant parts Tom Tromey @ 2018-02-26 6:49 ` Joel Brobecker 0 siblings, 0 replies; 18+ messages in thread From: Joel Brobecker @ 2018-02-26 6:49 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches On Thu, Feb 22, 2018 at 01:30:16PM -0700, Tom Tromey wrote: > This adds some initial support for variant parts to gdbtypes.h. A > variant part is represented as a union. The union has a flag > indicating that it has a discriminant, and information about the > discriminant is attached using the dynamic property system. > > 2018-02-22 Tom Tromey <tom@tromey.com> > > * value.h (value_union_variant): Declare. > * valops.c (value_union_variant): New function. > * gdbtypes.h (TYPE_FLAG_DISCRIMINATED_UNION): New macro. > (struct discriminant_info): New. > (enum dynamic_prop_node_kind) <DYN_PROP_DISCRIMINATED>: New > enumerator. > (struct main_type) <flag_discriminated_union>: New field. OK, with just one minor typo below... > + /* Compute the discriminant. Note that unpack_field_as_long handles > + sign extension when necessary, as does the DWARF reader -- so > + signed discriminants will be handled correct despite the use of correct -> correctly Thanks! -- Joel ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2018-04-12 18:45 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-02-22 20:30 [RFA v2 0/4] variants and variant parts Tom Tromey 2018-02-22 20:30 ` [RFA v2 4/4] Handle DW_TAG_variant_part and DW_TAG_variant Tom Tromey 2018-02-26 6:56 ` Joel Brobecker 2018-02-26 16:16 ` Tom Tromey 2018-02-22 20:30 ` [RFA v2 1/4] Sign-extend non-bit-fields in unpack_bits_as_long Tom Tromey 2018-02-26 6:45 ` Joel Brobecker 2018-02-22 20:30 ` [RFA v2 3/4] Convert Rust to use discriminated unions Tom Tromey 2018-02-26 6:50 ` Joel Brobecker 2018-02-27 23:23 ` Pedro Alves 2018-02-28 0:23 ` Tom Tromey 2018-04-10 20:36 ` -readnow crash Rust regression [Re: [RFA v2 3/4] Convert Rust to use discriminated unions] Jan Kratochvil 2018-04-11 2:52 ` Tom Tromey 2018-04-11 7:04 ` Jan Kratochvil 2018-04-11 19:49 ` Tom Tromey 2018-04-12 18:10 ` Tom Tromey 2018-04-12 18:45 ` Keith Seitz 2018-02-22 20:30 ` [RFA v2 2/4] Initial support for variant parts Tom Tromey 2018-02-26 6:49 ` Joel Brobecker
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).