[PATCH] Fix variant part regressions with older Rust compiler

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] Fix variant part regressions with older Rust compiler

Tom Tromey-4
From: Tom Tromey <[hidden email]>

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-07-22  Tom Tromey  <[hidden email]>

        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/dwarf2/read.c b/gdb/dwarf2/read.c
index 39ed455def5..278290a67d4 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<discriminant_range> 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<variant> (variants, n_variants);
 
   void *storage = obstack_alloc (obstack, sizeof (gdb::array_view<variant_part>));
@@ -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.26.2

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix variant part regressions with older Rust compiler

Tom Tromey-4
>>>>> "Tom" == Tom Tromey <[hidden email]> writes:

Tom> gdb/ChangeLog
Tom> 2020-07-22  Tom Tromey  <[hidden email]>

Tom> PR rust/26197:
Tom> * dwarf2/read.c (alloc_rust_variant): Handle univariant case.
Tom> (quirk_rust_enum): Call alloc_rust_variant for univariant case.
Tom> Fix off-by-one and type size errors in ordinary case.

I'm checking this in now.

Tom