[FYI v2 1/2] Fix crash with empty Rust enum

[FYI v2 1/2] Fix crash with empty Rust enum

[Tom Tromey]

While testing my Rust compiler patch to fix the DWARF representation
of Rust enums (https://github.com/rust-lang/rust/pull/54004), I found
a gdb crash coming from one of the Rust test cases.

The bug here is that the new variant support in gdb does not handle
the case where there are no variants in the enum.

This patch fixes the problem in a straightforward way.  Note that the
new tests are somewhat lax because I did not want to try to fully fix
this corner case for older compilers.  If you think that's
unacceptable, let meknow.

Tested on x86-64 Fedora 28 using several versions of the Rust
compiler.  I intend to push this to the 8.2 branch as well.

gdb/ChangeLog
2018-09-13  Tom Tromey  <tom@tromey.com>

	PR rust/23626:
	* rust-lang.c (rust_enum_variant): Now static.
	(rust_empty_enum_p): New function.
	(rust_print_enum, rust_evaluate_subexp, rust_print_struct_def):
	Handle empty enum.

gdb/testsuite/ChangeLog
2018-09-13  Tom Tromey  <tom@tromey.com>

	PR rust/23626:
	* gdb.rust/simple.rs (EmptyEnum): New type.
	(main): Use it.
	* gdb.rust/simple.exp (test_one_slice): Add empty enum test.
---
 gdb/ChangeLog                     |  8 ++++++
 gdb/rust-lang.c                   | 42 ++++++++++++++++++++++++++++++-
 gdb/testsuite/ChangeLog           |  7 ++++++
 gdb/testsuite/gdb.rust/simple.exp | 12 +++++++++
 gdb/testsuite/gdb.rust/simple.rs  |  4 +++
 5 files changed, 72 insertions(+), 1 deletion(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 50a6519105..1613808a6c 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,11 @@
+2018-09-13  Tom Tromey  <tom@tromey.com>
+
+	PR rust/23626:
+	* rust-lang.c (rust_enum_variant): Now static.
+	(rust_empty_enum_p): New function.
+	(rust_print_enum, rust_evaluate_subexp, rust_print_struct_def):
+	Handle empty enum.
+
 2018-09-13  Simon Marchi  <simon.marchi@ericsson.com>
 
 	* python/py-inferior.c (infpy_repr): New.
diff --git a/gdb/rust-lang.c b/gdb/rust-lang.c
index 1871ec7df9..b77738979f 100644
--- a/gdb/rust-lang.c
+++ b/gdb/rust-lang.c
@@ -74,9 +74,22 @@ rust_enum_p (const struct type *type)
 	  && TYPE_FLAG_DISCRIMINATED_UNION (TYPE_FIELD_TYPE (type, 0)));
 }
 
+/* Return true if TYPE, which must be an enum type, has no
+   variants.  */
+
+static bool
+rust_empty_enum_p (const struct type *type)
+{
+  gdb_assert (rust_enum_p (type));
+  /* In Rust the enum always fills the containing structure.  */
+  gdb_assert (TYPE_FIELD_BITPOS (type, 0) == 0);
+
+  return TYPE_NFIELDS (TYPE_FIELD_TYPE (type, 0)) == 0;
+}
+
 /* Given an enum type and contents, find which variant is active.  */
 
-struct field *
+static struct field *
 rust_enum_variant (struct type *type, const gdb_byte *contents)
 {
   /* In Rust the enum always fills the containing structure.  */
@@ -429,6 +442,13 @@ rust_print_enum (struct type *type, int embedded_offset,
 
   opts.deref_ref = 0;
 
+  if (rust_empty_enum_p (type))
+    {
+      /* Print the enum type name here to be more clear.  */
+      fprintf_filtered (stream, _("%s {<No data fields>}"), TYPE_NAME (type));
+      return;
+    }
+
   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;
@@ -664,6 +684,18 @@ rust_print_struct_def (struct type *type, const char *varstring,
       if (is_enum)
 	{
 	  fputs_filtered ("enum ", stream);
+
+	  if (rust_empty_enum_p (type))
+	    {
+	      if (tagname != NULL)
+		{
+		  fputs_filtered (tagname, stream);
+		  fputs_filtered (" ", stream);
+		}
+	      fputs_filtered ("{}", stream);
+	      return;
+	    }
+
 	  type = TYPE_FIELD_TYPE (type, 0);
 
 	  struct dynamic_prop *discriminant_prop
@@ -1604,6 +1636,10 @@ rust_evaluate_subexp (struct type *expect_type, struct expression *exp,
 
 	    if (rust_enum_p (type))
 	      {
+		if (rust_empty_enum_p (type))
+		  error (_("Cannot access field %d of empty enum %s"),
+			 field_number, TYPE_NAME (type));
+
 		const gdb_byte *valaddr = value_contents (lhs);
 		struct field *variant_field = rust_enum_variant (type, valaddr);
 
@@ -1672,6 +1708,10 @@ tuple structs, and tuple-like enum variants"));
         type = value_type (lhs);
         if (TYPE_CODE (type) == TYPE_CODE_STRUCT && rust_enum_p (type))
 	  {
+	    if (rust_empty_enum_p (type))
+	      error (_("Cannot access field %s of empty enum %s"),
+		     field_name, TYPE_NAME (type));
+
 	    const gdb_byte *valaddr = value_contents (lhs);
 	    struct field *variant_field = rust_enum_variant (type, valaddr);
 
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index db9c214199..c356d0287a 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,10 @@
+2018-09-13  Tom Tromey  <tom@tromey.com>
+
+	PR rust/23626:
+	* gdb.rust/simple.rs (EmptyEnum): New type.
+	(main): Use it.
+	* gdb.rust/simple.exp (test_one_slice): Add empty enum test.
+
 2018-09-13  Simon Marchi  <simon.marchi@ericsson.com>
 
 	* gdb.python/py-inferior.exp: Test repr() of gdb.Inferior.
diff --git a/gdb/testsuite/gdb.rust/simple.exp b/gdb/testsuite/gdb.rust/simple.exp
index 20fe8dd0a8..07b2512220 100644
--- a/gdb/testsuite/gdb.rust/simple.exp
+++ b/gdb/testsuite/gdb.rust/simple.exp
@@ -303,6 +303,18 @@ gdb_test_sequence "ptype/o SimpleLayout" "" {
     "                         }"
 }
 
+# PR rust/23626 - this used to crash.  Note that the results are
+# fairly lax because most existing versions of Rust (those before the
+# DW_TAG_variant patches) do not emit what gdb wants here; and there
+# was little point fixing gdb to cope with these cases as the fixed
+# compilers will be available soon
+gdb_test "print empty_enum_value" \
+    " = simple::EmptyEnum.*"
+gdb_test "ptype empty_enum_value" "simple::EmptyEnum.*"
+# Just make sure these don't crash, for the same reason.
+gdb_test "print empty_enum_value.0" ""
+gdb_test "print empty_enum_value.something" ""
+
 load_lib gdb-python.exp
 if {[skip_python_tests]} {
     continue
diff --git a/gdb/testsuite/gdb.rust/simple.rs b/gdb/testsuite/gdb.rust/simple.rs
index 1bcc030d60..00a25e0828 100644
--- a/gdb/testsuite/gdb.rust/simple.rs
+++ b/gdb/testsuite/gdb.rust/simple.rs
@@ -92,6 +92,8 @@ struct SimpleLayout {
     f2: u16
 }
 
+enum EmptyEnum {}
+
 fn main () {
     let a = ();
     let b : [i32; 0] = [];
@@ -168,6 +170,8 @@ fn main () {
     let u = Union { f2: 255 };
     let simplelayout = SimpleLayout { f1: 8, f2: 9 };
 
+    let empty_enum_value: EmptyEnum = unsafe { ::std::mem::zeroed() };
+
     println!("{}, {}", x.0, x.1);        // set breakpoint here
     println!("{}", diff2(92, 45));
     empty();
-- 
2.17.1

[FYI v2 0/2] Fix two Rust bugs [also for 8.2.1]

[Tom Tromey]

This is v2 of a patch I sent yesterday to fix a gdb crash with a
certain Rust program (and compiler).

This version addresses some of the comments Simon made.  I've filed a
bug for further fixes, after the necessary version of rustc is
released.

I noticed one further buglet in this area, which is fixed by the
second patch.

Tested on x86-64 Fedora 28.  I'm checking this in to the 8.2 branch as
well.

Tom

[FYI v2 2/2] Make Rust error message mention the field name

[Tom Tromey]

I noticed a spot in rust-lang.c where the placeholder "foo" was used
instead of the actual field name.  This patch fixes the bug.

gdb/ChangeLog
2018-09-13  Tom Tromey  <tom@tromey.com>

	PR rust/23650:
	* rust-lang.c (rust_evaluate_subexp): Use field name, not "foo".

gdb/testsuite/ChangeLog
2018-09-13  Tom Tromey  <tom@tromey.com>

	PR rust/23650:
	* gdb.rust/simple.exp: Add test for enum field access error.
---
 gdb/ChangeLog                     | 5 +++++
 gdb/rust-lang.c                   | 4 ++--
 gdb/testsuite/ChangeLog           | 5 +++++
 gdb/testsuite/gdb.rust/simple.exp | 2 ++
 4 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 1613808a6c..5783e7cb0a 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2018-09-13  Tom Tromey  <tom@tromey.com>
+
+	PR rust/23650:
+	* rust-lang.c (rust_evaluate_subexp): Use field name, not "foo".
+
 2018-09-13  Tom Tromey  <tom@tromey.com>
 
 	PR rust/23626:
diff --git a/gdb/rust-lang.c b/gdb/rust-lang.c
index b77738979f..43db722142 100644
--- a/gdb/rust-lang.c
+++ b/gdb/rust-lang.c
@@ -1726,9 +1726,9 @@ tuple structs, and tuple-like enum variants"));
 	    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 "
+		error (_("Attempting to access named field %s of tuple "
 			 "variant %s::%s, which has only anonymous fields"),
-		       TYPE_NAME (outer_type),
+		       field_name, TYPE_NAME (outer_type),
 		       rust_last_path_segment (TYPE_NAME (type)));
 
 	    TRY
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index c356d0287a..7d6a2ecbf0 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2018-09-13  Tom Tromey  <tom@tromey.com>
+
+	PR rust/23650:
+	* gdb.rust/simple.exp: Add test for enum field access error.
+
 2018-09-13  Tom Tromey  <tom@tromey.com>
 
 	PR rust/23626:
diff --git a/gdb/testsuite/gdb.rust/simple.exp b/gdb/testsuite/gdb.rust/simple.exp
index 07b2512220..956a6ca6fe 100644
--- a/gdb/testsuite/gdb.rust/simple.exp
+++ b/gdb/testsuite/gdb.rust/simple.exp
@@ -134,6 +134,8 @@ gdb_test "print univariant" " = simple::Univariant::Foo{a: 1}"
 gdb_test "print univariant.a" " = 1"
 gdb_test "print univariant_anon" " = simple::UnivariantAnon::Foo\\(1\\)"
 gdb_test "print univariant_anon.0" " = 1"
+gdb_test "print univariant_anon.sss" \
+    "Attempting to access named field sss of tuple variant simple::UnivariantAnon::Foo, which has only anonymous fields"
 
 gdb_test_sequence "ptype simple::Univariant" "" {
     "type = enum simple::Univariant \\{"
-- 
2.17.1