From 57d02173a25465a5400fad3764568cefb8eae9ab Mon Sep 17 00:00:00 2001 From: Tom Tromey Date: Wed, 5 Aug 2020 09:52:55 -0600 Subject: [PATCH] Fix variant part regressions with older Rust compiler Older Rust compilers used special field names, rather than DWARF features, to express the variant parts of Rust enums. This is handled in gdb through a quirk recognizer that rewrites the types. Tom de Vries pointed out in PR rust/26197 that the variant part rewrite regressed this code. This patch fixes the problems: * Univariant enums were not handled properly. Now we simply call alloc_rust_variant for these as well. * There was an off-by-one error in the handling of ordinary enums. * Ordinary enums should have the size of their member types reset to match the size of the enclosing enum. (It's not clear to me if this is truly necessary, but it placates a test, and this is just legacy handling in any case.) Tested with Rust 1.12.0, 1.14.0, 1.19.0, 1.36.0, and 1.45.0 on x86-64 Fedora 32. There were some unrelated failures with 1.14.0 and 1.19,0; but considering that these are fairly old releases, I don't plan to look into them unless someone complains. Note that this patch will not fix all the issues in the PR. In that PR, Tom is using a somewhat unusual build of Rust -- in particular it uses an older (pre-DWARF variant part) LLVM with a newer Rust. I believe this compiler doesn't correctly implement the old-style name fallback; the details are in the bug. gdb/ChangeLog 2020-08-05 Tom Tromey PR rust/26197: * dwarf2/read.c (alloc_rust_variant): Handle univariant case. (quirk_rust_enum): Call alloc_rust_variant for univariant case. Fix off-by-one and type size errors in ordinary case. --- gdb/ChangeLog | 7 +++++++ gdb/dwarf2/read.c | 32 +++++++++++++++++++++++--------- 2 files changed, 30 insertions(+), 9 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 066512703ac..fc4fe7cdaac 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,10 @@ +2020-08-05 Tom Tromey + + PR rust/26197: + * dwarf2/read.c (alloc_rust_variant): Handle univariant case. + (quirk_rust_enum): Call alloc_rust_variant for univariant case. + Fix off-by-one and type size errors in ordinary case. + 2020-08-05 Tom de Vries * gdbtypes.c (type_not_allocated, type_not_associated): Use diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c index f591ef62489..cf3a42ac3ea 100644 --- a/gdb/dwarf2/read.c +++ b/gdb/dwarf2/read.c @@ -9416,7 +9416,8 @@ rust_fully_qualify (struct obstack *obstack, const char *p1, const char *p2) /* A helper that allocates a variant part to attach to a Rust enum type. OBSTACK is where the results should be allocated. TYPE is the type we're processing. DISCRIMINANT_INDEX is the index of the - discriminant. It must be the index of one of the fields of TYPE. + discriminant. It must be the index of one of the fields of TYPE, + or -1 to mean there is no discriminant (univariant enum). DEFAULT_INDEX is the index of the default field; or -1 if there is no default. RANGES is indexed by "effective" field number (the field index, but omitting the discriminant and default fields) and @@ -9429,15 +9430,17 @@ alloc_rust_variant (struct obstack *obstack, struct type *type, int discriminant_index, int default_index, gdb::array_view ranges) { - /* When DISCRIMINANT_INDEX == -1, we have a univariant enum. Those - must be handled by the caller. */ - gdb_assert (discriminant_index >= 0 - && discriminant_index < type->num_fields ()); + /* When DISCRIMINANT_INDEX == -1, we have a univariant enum. */ + gdb_assert (discriminant_index == -1 + || (discriminant_index >= 0 + && discriminant_index < type->num_fields ())); gdb_assert (default_index == -1 || (default_index >= 0 && default_index < type->num_fields ())); /* We have one variant for each non-discriminant field. */ - int n_variants = type->num_fields () - 1; + int n_variants = type->num_fields (); + if (discriminant_index != -1) + --n_variants; variant *variants = new (obstack) variant[n_variants]; int var_idx = 0; @@ -9466,7 +9469,12 @@ alloc_rust_variant (struct obstack *obstack, struct type *type, variant_part *part = new (obstack) variant_part; part->discriminant_index = discriminant_index; - part->is_unsigned = TYPE_UNSIGNED (type->field (discriminant_index).type ()); + /* If there is no discriminant, then whether it is signed is of no + consequence. */ + part->is_unsigned + = (discriminant_index == -1 + ? false + : TYPE_UNSIGNED (type->field (discriminant_index).type ())); part->variants = gdb::array_view (variants, n_variants); void *storage = obstack_alloc (obstack, sizeof (gdb::array_view)); @@ -9594,6 +9602,8 @@ quirk_rust_enum (struct type *type, struct objfile *objfile) field_type->set_name (rust_fully_qualify (&objfile->objfile_obstack, type->name (), variant_name)); + + alloc_rust_variant (&objfile->objfile_obstack, type, -1, 0, {}); } else { @@ -9682,10 +9692,14 @@ quirk_rust_enum (struct type *type, struct objfile *objfile) auto iter = discriminant_map.find (variant_name); if (iter != discriminant_map.end ()) { - ranges[i].low = iter->second; - ranges[i].high = iter->second; + ranges[i - 1].low = iter->second; + ranges[i - 1].high = iter->second; } + /* In Rust, each element should have the size of the + enclosing enum. */ + TYPE_LENGTH (type->field (i).type ()) = TYPE_LENGTH (type); + /* Remove the discriminant field, if it exists. */ struct type *sub_type = type->field (i).type (); if (sub_type->num_fields () > 0) -- 2.30.2