[RFA,v2,4/4] Handle DW_TAG_variant_part and DW_TAG_variant

Message ID 20180222203018.23551-5-tom@tromey.com
State New
Headers show
Series
  • variants and variant parts
Related show

Commit Message

Tom Tromey Feb. 22, 2018, 8:30 p.m.
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

-- 
2.13.6

Comments

Joel Brobecker Feb. 26, 2018, 6:56 a.m. | #1
> 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
Tom Tromey Feb. 26, 2018, 4:16 p.m. | #2
>>>>> "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

Patch

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"