[PATCH 0/11] Variant part support, plus more

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

[PATCH 0/11] Variant part support, plus more

Tom Tromey-4
Currently, the Ada support in gdb relies on what are called "gnat
encodings" -- which really just means that some debug info is
represented using magic symbol names, attached to other debug objects.
A long term goal here is to slowly remove gnat encodings, in favor of
using standard DWARF.

This series is one step toward that goal.  It adds support for DWARF
variant parts to gdb.  It also brings in support for fields that have
dynamic offsets, and types that have dynamic lengths.  It also adds a
Python API to let pretty-printers know a bit more about dynamic types.

Along the way, the Rust enum code is rewritten to use the new
machinery; this provides benefits visible from Python and MI.  Also,
at least one bug is uncovered for future fixing (see patch #7).

One future direction might be to arrange for resolve_dynamic_type to
record concrete instances of dynamic types, and reuse them when
appropriate.  I haven't attempted this.

Let me know what you think.

Tom


Reply | Threaded
Open this post in threaded view
|

[PATCH 01/11] Rename "variant" to "ppc_variant"

Tom Tromey-4
I wanted to use the name "variant" to represent a DWARF variant, but
it turned out there was an existing structure of that name.  This
renames the existing variant to "ppc_variant".

gdb/ChangeLog
2020-04-08  Tom Tromey  <[hidden email]>

        * rs6000-tdep.c (struct ppc_variant): Rename from "variant".
        (variants, find_variant_by_arch, rs6000_gdbarch_init): Update.
---
 gdb/ChangeLog     |  5 +++++
 gdb/rs6000-tdep.c | 10 +++++-----
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c
index 2c41e1c858b..6a3ae7b0ee8 100644
--- a/gdb/rs6000-tdep.c
+++ b/gdb/rs6000-tdep.c
@@ -3315,7 +3315,7 @@ rs6000_adjust_frame_regnum (struct gdbarch *gdbarch, int num, int eh_frame_p)
 
 /* Information about a particular processor variant.  */
 
-struct variant
+struct ppc_variant
   {
     /* Name of this variant.  */
     const char *name;
@@ -3333,7 +3333,7 @@ struct variant
     struct target_desc **tdesc;
   };
 
-static struct variant variants[] =
+static struct ppc_variant variants[] =
 {
   {"powerpc", "PowerPC user-level", bfd_arch_powerpc,
    bfd_mach_ppc, &tdesc_powerpc_altivec32},
@@ -3392,10 +3392,10 @@ static struct variant variants[] =
 /* Return the variant corresponding to architecture ARCH and machine number
    MACH.  If no such variant exists, return null.  */
 
-static const struct variant *
+static const struct ppc_variant *
 find_variant_by_arch (enum bfd_architecture arch, unsigned long mach)
 {
-  const struct variant *v;
+  const struct ppc_variant *v;
 
   for (v = variants; v->name; v++)
     if (arch == v->arch && mach == v->mach)
@@ -6199,7 +6199,7 @@ rs6000_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
      layout, if we do not already have one.  */
   if (! tdesc_has_registers (tdesc))
     {
-      const struct variant *v;
+      const struct ppc_variant *v;
 
       /* Choose variant.  */
       v = find_variant_by_arch (arch, mach);
--
2.21.1

Reply | Threaded
Open this post in threaded view
|

[PATCH 02/11] Add new variant part code

Tom Tromey-4
In reply to this post by Tom Tromey-4
This patch adds the infrastructure for the new variant part code.  At
this point, nothing uses this code.  This is done in a separate patch
to make it simpler to review.

I examined a few possible approaches to handling variant parts.  In
particular, I considered having a DWARF variant part be a union
(similar to how the Rust code works now); and I considered having type
fields have a flag indicating that they are variants.

Having separate types seemed bad conceptually, because these variants
aren't truly separate -- they rely on the "parent" type.  And,
changing how fields worked seemed excessively invasive.

So, in the end I thought the approach taken in this patch was both
simple to implement and understand, without losing generality.  The
idea in this patch is that all the fields of a type with variant parts
will be stored in a single field array, just as if they'd all been
listed directly.  Then, the variants are attached as a dynamic
property.  These control which fields end up in the type that's
constructed during dynamic type resolution.

gdb/ChangeLog
2020-04-08  Tom Tromey  <[hidden email]>

        * gdbtypes.c (is_dynamic_type_internal): Check for variant parts.
        (variant::matches, compute_variant_fields_recurse)
        (compute_variant_fields_inner, compute_variant_fields): New
        functions.
        (resolve_dynamic_struct): Check for DYN_PROP_VARIANT_PARTS.
        Use resolved_type after type is made.
        (operator==): Add new cases.
        * gdbtypes.h (TYPE_HAS_VARIANT_PARTS): New macro.
        (struct discriminant_range, struct variant, struct variant_part):
        New.
        (union dynamic_prop_data) <variant_parts, original_type>: New
        members.
        (enum dynamic_prop_node_kind) <DYN_PROP_VARIANT_PARTS>: New constant.
        (enum dynamic_prop_kind) <PROP_TYPE, PROP_VARIANT_PARTS>: New
        constants.
        * value.c (unpack_bits_as_long): Now public.
        * value.h (unpack_bits_as_long): Declare.
---
 gdb/ChangeLog  |  20 +++++
 gdb/gdbtypes.c | 200 ++++++++++++++++++++++++++++++++++++++++++++++---
 gdb/gdbtypes.h | 105 +++++++++++++++++++++++++-
 gdb/value.c    |  20 +----
 gdb/value.h    |  21 ++++++
 5 files changed, 338 insertions(+), 28 deletions(-)

diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index f23def1ff71..6b32fc9a8ff 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -39,6 +39,7 @@
 #include "dwarf2/loc.h"
 #include "gdbcore.h"
 #include "floatformat.h"
+#include <algorithm>
 
 /* Initialize BADNESS constants.  */
 
@@ -886,6 +887,10 @@ operator== (const dynamic_prop &l, const dynamic_prop &r)
     case PROP_LOCEXPR:
     case PROP_LOCLIST:
       return l.data.baton == r.data.baton;
+    case PROP_VARIANT_PARTS:
+      return l.data.variant_parts == r.data.variant_parts;
+    case PROP_TYPE:
+      return l.data.original_type == r.data.original_type;
     }
 
   gdb_assert_not_reached ("unhandled dynamic_prop kind");
@@ -1966,6 +1971,10 @@ is_dynamic_type_internal (struct type *type, int top_level)
   if (TYPE_ALLOCATED_PROP (type))
     return 1;
 
+  struct dynamic_prop *prop = get_dyn_prop (DYN_PROP_VARIANT_PARTS, type);
+  if (prop != nullptr && prop->kind != PROP_TYPE)
+    return 1;
+
   switch (TYPE_CODE (type))
     {
     case TYPE_CODE_RANGE:
@@ -2215,6 +2224,161 @@ resolve_dynamic_union (struct type *type,
   return resolved_type;
 }
 
+/* See gdbtypes.h.  */
+
+bool
+variant::matches (ULONGEST value, bool is_unsigned) const
+{
+  for (const discriminant_range &range : discriminants)
+    if (range.contains (value, is_unsigned))
+      return true;
+  return false;
+}
+
+static void
+compute_variant_fields_inner (struct type *type,
+      struct property_addr_info *addr_stack,
+      const variant_part &part,
+      std::vector<bool> &flags);
+
+/* A helper function to determine which variant fields will be active.
+   This handles both the variant's direct fields, and any variant
+   parts embedded in this variant.  TYPE is the type we're examining.
+   ADDR_STACK holds information about the concrete object.  VARIANT is
+   the current variant to be handled.  FLAGS is where the results are
+   stored -- this function sets the Nth element in FLAGS if the
+   corresponding field is enabled.  ENABLED is whether this variant is
+   enabled or not.  */
+
+static void
+compute_variant_fields_recurse (struct type *type,
+ struct property_addr_info *addr_stack,
+ const variant &variant,
+ std::vector<bool> &flags,
+ bool enabled)
+{
+  for (int field = variant.first_field; field < variant.last_field; ++field)
+    flags[field] = enabled;
+
+  for (const variant_part &new_part : variant.parts)
+    {
+      if (enabled)
+ compute_variant_fields_inner (type, addr_stack, new_part, flags);
+      else
+ {
+  for (const auto &sub_variant : new_part.variants)
+    compute_variant_fields_recurse (type, addr_stack, sub_variant,
+    flags, enabled);
+ }
+    }
+}
+
+/* A helper function to determine which variant fields will be active.
+   This evaluates the discriminant, decides which variant (if any) is
+   active, and then updates FLAGS to reflect which fields should be
+   available.  TYPE is the type we're examining.  ADDR_STACK holds
+   information about the concrete object.  VARIANT is the current
+   variant to be handled.  FLAGS is where the results are stored --
+   this function sets the Nth element in FLAGS if the corresponding
+   field is enabled.  */
+
+static void
+compute_variant_fields_inner (struct type *type,
+      struct property_addr_info *addr_stack,
+      const variant_part &part,
+      std::vector<bool> &flags)
+{
+  /* Evaluate the discriminant.  */
+  gdb::optional<ULONGEST> discr_value;
+  if (part.discriminant_index != -1)
+    {
+      int idx = part.discriminant_index;
+
+      if (TYPE_FIELD_LOC_KIND (type, idx) != FIELD_LOC_KIND_BITPOS)
+ error (_("Cannot determine struct field location"
+ " (invalid location kind)"));
+
+      if (addr_stack->valaddr != NULL)
+ discr_value = unpack_field_as_long (type, addr_stack->valaddr, idx);
+      else
+ {
+  CORE_ADDR addr = (addr_stack->addr
+    + (TYPE_FIELD_BITPOS (type, idx)
+       / TARGET_CHAR_BIT));
+
+  LONGEST bitsize = TYPE_FIELD_BITSIZE (type, idx);
+  LONGEST size = bitsize / 8;
+  if (size == 0)
+    size = TYPE_LENGTH (TYPE_FIELD_TYPE (type, idx));
+
+  gdb_byte bits[sizeof (ULONGEST)];
+  read_memory (addr, bits, size);
+
+  LONGEST bitpos = (TYPE_FIELD_BITPOS (type, idx)
+    % TARGET_CHAR_BIT);
+
+  discr_value = unpack_bits_as_long (TYPE_FIELD_TYPE (type, idx),
+     bits, bitpos, bitsize);
+ }
+    }
+
+  /* Go through each variant and see which applies.  */
+  const variant *default_variant = nullptr;
+  const variant *applied_variant = nullptr;
+  for (const auto &variant : part.variants)
+    {
+      if (variant.is_default ())
+ default_variant = &variant;
+      else if (discr_value.has_value ()
+       && variant.matches (*discr_value, part.is_unsigned))
+ {
+  applied_variant = &variant;
+  break;
+ }
+    }
+  if (applied_variant == nullptr)
+    applied_variant = default_variant;
+
+  for (const auto &variant : part.variants)
+    compute_variant_fields_recurse (type, addr_stack, variant,
+    flags, applied_variant == &variant);
+}  
+
+/* Determine which variant fields are available in TYPE.  The enabled
+   fields are stored in RESOLVED_TYPE.  ADDR_STACK holds information
+   about the concrete object.  PARTS describes the top-level variant
+   parts for this type.  */
+
+static void
+compute_variant_fields (struct type *type,
+ struct type *resolved_type,
+ struct property_addr_info *addr_stack,
+ const gdb::array_view<variant_part> &parts)
+{
+  /* Assume all fields are included by default.  */
+  std::vector<bool> flags (TYPE_NFIELDS (resolved_type), true);
+
+  /* Now disable fields based on the variants that control them.  */
+  for (const auto &part : parts)
+    compute_variant_fields_inner (type, addr_stack, part, flags);
+
+  TYPE_NFIELDS (resolved_type) = std::count (flags.begin (), flags.end (),
+     true);
+  TYPE_FIELDS (resolved_type)
+    = (struct field *) TYPE_ALLOC (resolved_type,
+   TYPE_NFIELDS (resolved_type)
+   * sizeof (struct field));
+  int out = 0;
+  for (int i = 0; i < TYPE_NFIELDS (type); ++i)
+    {
+      if (!flags[i])
+ continue;
+
+      TYPE_FIELD (resolved_type, out) = TYPE_FIELD (type, i);
+      ++out;
+    }
+}
+
 /* Resolve dynamic bounds of members of the struct TYPE to static
    bounds.  ADDR_STACK is a stack of struct property_addr_info to
    be used if needed during the dynamic resolution.  */
@@ -2231,19 +2395,35 @@ resolve_dynamic_struct (struct type *type,
   gdb_assert (TYPE_NFIELDS (type) > 0);
 
   resolved_type = copy_type (type);
-  TYPE_FIELDS (resolved_type)
-    = (struct field *) TYPE_ALLOC (resolved_type,
-   TYPE_NFIELDS (resolved_type)
-   * sizeof (struct field));
-  memcpy (TYPE_FIELDS (resolved_type),
-  TYPE_FIELDS (type),
-  TYPE_NFIELDS (resolved_type) * sizeof (struct field));
+
+  struct dynamic_prop *variant_prop = get_dyn_prop (DYN_PROP_VARIANT_PARTS,
+    resolved_type);
+  if (variant_prop != nullptr && variant_prop->kind == PROP_VARIANT_PARTS)
+    {
+      compute_variant_fields (type, resolved_type, addr_stack,
+      *variant_prop->data.variant_parts);
+      /* We want to leave the property attached, so that the Rust code
+ can tell whether the type was originally an enum.  */
+      variant_prop->kind = PROP_TYPE;
+      variant_prop->data.original_type = type;
+    }
+  else
+    {
+      TYPE_FIELDS (resolved_type)
+ = (struct field *) TYPE_ALLOC (resolved_type,
+       TYPE_NFIELDS (resolved_type)
+       * sizeof (struct field));
+      memcpy (TYPE_FIELDS (resolved_type),
+      TYPE_FIELDS (type),
+      TYPE_NFIELDS (resolved_type) * sizeof (struct field));
+    }
+
   for (i = 0; i < TYPE_NFIELDS (resolved_type); ++i)
     {
       unsigned new_bit_length;
       struct property_addr_info pinfo;
 
-      if (field_is_static (&TYPE_FIELD (type, i)))
+      if (field_is_static (&TYPE_FIELD (resolved_type, i)))
  continue;
 
       /* As we know this field is not a static field, the field's
@@ -2253,11 +2433,11 @@ resolve_dynamic_struct (struct type *type,
  that verification indicates a bug in our code, the error
  is not severe enough to suggest to the user he stops
  his debugging session because of it.  */
-      if (TYPE_FIELD_LOC_KIND (type, i) != FIELD_LOC_KIND_BITPOS)
+      if (TYPE_FIELD_LOC_KIND (resolved_type, i) != FIELD_LOC_KIND_BITPOS)
  error (_("Cannot determine struct field location"
  " (invalid location kind)"));
 
-      pinfo.type = check_typedef (TYPE_FIELD_TYPE (type, i));
+      pinfo.type = check_typedef (TYPE_FIELD_TYPE (resolved_type, i));
       pinfo.valaddr = addr_stack->valaddr;
       pinfo.addr
  = (addr_stack->addr
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index 77cc92e419d..8b4da6e3f9f 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -51,6 +51,7 @@
 #include "gdbsupport/underlying.h"
 #include "gdbsupport/print-utils.h"
 #include "dwarf2.h"
+#include "gdb_obstack.h"
 
 /* Forward declarations for prototypes.  */
 struct field;
@@ -358,6 +359,10 @@ DEF_ENUM_FLAGS_TYPE (enum type_instance_flag_value, type_instance_flags);
 #define TYPE_IS_ALLOCATABLE(t) \
   (get_dyn_prop (DYN_PROP_ALLOCATED, t) != NULL)
 
+/* * True if this type has variant parts.  */
+#define TYPE_HAS_VARIANT_PARTS(t) \
+  (get_dyn_prop (DYN_PROP_VARIANT_PARTS, t) != nullptr)
+
 /* * Instruction-space delimited type.  This is for Harvard architectures
    which have separate instruction and data address spaces (and perhaps
    others).
@@ -399,6 +404,84 @@ 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 about a single discriminant.  */
+
+struct discriminant_range
+{
+  /* * The range of values for the variant.  This is an inclusive
+     range.  */
+  ULONGEST low, high;
+
+  /* * Return true if VALUE is contained in this range.  IS_UNSIGNED
+     is true if this should be an unsigned comparison; false for
+     signed.  */
+  bool contains (ULONGEST value, bool is_unsigned) const
+  {
+    if (is_unsigned)
+      return value >= low && value <= high;
+    LONGEST valuel = (LONGEST) value;
+    return valuel >= (LONGEST) low && valuel <= (LONGEST) high;
+  }
+};
+
+struct variant_part;
+
+/* * A single variant.  A variant has a list of discriminant values.
+   When the discriminator matches one of these, the variant is
+   enabled.  Each variant controls zero or more fields; and may also
+   control other variant parts as well.  This struct corresponds to
+   DW_TAG_variant in DWARF.  */
+
+struct variant : allocate_on_obstack
+{
+  /* * The discriminant ranges for this variant.  */
+  gdb::array_view<discriminant_range> discriminants;
+
+  /* * The fields controlled by this variant.  This is inclusive on
+     the low end and exclusive on the high end.  A variant may not
+     control any fields, in which case the two values will be equal.
+     These are indexes into the type's array of fields.  */
+  int first_field;
+  int last_field;
+
+  /* * Variant parts controlled by this variant.  */
+  gdb::array_view<variant_part> parts;
+
+  /* * Return true if this is the default variant.  The default
+     variant can be recognized because it has no associated
+     discriminants.  */
+  bool is_default () const
+  {
+    return discriminants.empty ();
+  }
+
+  /* * Return true if this variant matches VALUE.  IS_UNSIGNED is true
+     if this should be an unsigned comparison; false for signed.  */
+  bool matches (ULONGEST value, bool is_unsigned) const;
+};
+
+/* * A variant part.  Each variant part has an optional discriminant
+   and holds an array of variants.  This struct corresponds to
+   DW_TAG_variant_part in DWARF.  */
+
+struct variant_part : allocate_on_obstack
+{
+  /* * The index of the discriminant field in the outer type.  This is
+     an index into the type's array of fields.  If this is -1, there
+     is no discriminant, and only the default variant can be
+     considered to be selected.  */
+  int discriminant_index;
+
+  /* * True if this discriminant is unsigned; false if signed.  This
+     comes from the type of the discriminant.  */
+  bool is_unsigned;
+
+  /* * The variants that are controlled by this variant part.  Note
+     that these will always be sorted by field number.  */
+  gdb::array_view<variant> variants;
+};
+
+
 /* * Information needed for a discriminated union.  A discriminated
    union is handled somewhat differently from an ordinary union.
 
@@ -438,7 +521,9 @@ enum dynamic_prop_kind
   PROP_CONST,     /* Constant.  */
   PROP_ADDR_OFFSET, /* Address offset.  */
   PROP_LOCEXPR,   /* Location expression.  */
-  PROP_LOCLIST    /* Location list.  */
+  PROP_LOCLIST,    /* Location list.  */
+  PROP_VARIANT_PARTS, /* Variant parts.  */
+  PROP_TYPE,   /* Type.  */
 };
 
 union dynamic_prop_data
@@ -450,6 +535,21 @@ union dynamic_prop_data
   /* Storage for dynamic property.  */
 
   void *baton;
+
+  /* Storage of variant parts for a type.  A type with variant parts
+     has all its fields "linearized" -- stored in a single field
+     array, just as if they had all been declared that way.  The
+     variant parts are attached via a dynamic property, and then are
+     used to control which fields end up in the final type during
+     dynamic type resolution.  */
+
+  const gdb::array_view<variant_part> *variant_parts;
+
+  /* Once a variant type is resolved, we may want to be able to go
+     from the resolved type to the original type.  In this case we
+     rewrite the property's kind and set this field.  */
+
+  struct type *original_type;
 };
 
 /* * Used to store a dynamic property.  */
@@ -493,6 +593,9 @@ enum dynamic_prop_node_kind
 
   /* A property holding information about a discriminated union.  */
   DYN_PROP_DISCRIMINATED,
+
+  /* A property holding variant parts.  */
+  DYN_PROP_VARIANT_PARTS,
 };
 
 /* * List for dynamic type attributes.  */
diff --git a/gdb/value.c b/gdb/value.c
index f722c272d8b..641c09a0624 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -3090,23 +3090,9 @@ value_fn_field (struct value **arg1p, struct fn_field *f,
 
 
 
-/* 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; if
-   BITSIZE is zero, then the length is taken from FIELD_TYPE.
-
-   Extracting bits depends on endianness of the machine.  Compute the
-   number of least significant bits to discard.  For big endian machines,
-   we compute the total number of bits in the anonymous object, subtract
-   off the bit count from the MSB of the object to the MSB of the
-   bitfield, then the size of the bitfield, which leaves the LSB discard
-   count.  For little endian machines, the discard count is simply the
-   number of bits from the LSB of the anonymous object to the LSB of the
-   bitfield.
-
-   If the field is signed, we also do sign extension.  */
-
-static LONGEST
+/* See value.h.  */
+
+LONGEST
 unpack_bits_as_long (struct type *field_type, const gdb_byte *valaddr,
      LONGEST bitpos, LONGEST bitsize)
 {
diff --git a/gdb/value.h b/gdb/value.h
index 247be13a0d9..dfe3e8e3ed3 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -651,6 +651,27 @@ extern CORE_ADDR unpack_pointer (struct type *type, const gdb_byte *valaddr);
 extern LONGEST unpack_field_as_long (struct type *type,
      const gdb_byte *valaddr,
      int fieldno);
+
+/* 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; if
+   BITSIZE is zero, then the length is taken from FIELD_TYPE.
+
+   Extracting bits depends on endianness of the machine.  Compute the
+   number of least significant bits to discard.  For big endian machines,
+   we compute the total number of bits in the anonymous object, subtract
+   off the bit count from the MSB of the object to the MSB of the
+   bitfield, then the size of the bitfield, which leaves the LSB discard
+   count.  For little endian machines, the discard count is simply the
+   number of bits from the LSB of the anonymous object to the LSB of the
+   bitfield.
+
+   If the field is signed, we also do sign extension.  */
+
+extern LONGEST unpack_bits_as_long (struct type *field_type,
+    const gdb_byte *valaddr,
+    LONGEST bitpos, LONGEST bitsize);
+
 extern int unpack_value_field_as_long (struct type *type, const gdb_byte *valaddr,
  LONGEST embedded_offset, int fieldno,
  const struct value *val, LONGEST *result);
--
2.21.1

Reply | Threaded
Open this post in threaded view
|

[PATCH 03/11] Allow DWARF expression to push the initial address

Tom Tromey-4
In reply to this post by Tom Tromey-4
Some DWARF expressions must be evaluated by first pushing the object
address onto the evaluation stack.  This patch adds this ability.
This functionality is not used yet, but it will be used in a later
patch.  This is split out for easier review and also because it
improved the patch series ordering.

gdb/ChangeLog
2020-04-08  Tom Tromey  <[hidden email]>

        * dwarf2/loc.c (dwarf2_locexpr_baton_eval): Add
        "push_initial_value" parameter.
        (dwarf2_evaluate_property): Likewise.
        * dwarf2/loc.h (dwarf2_evaluate_property): Update.
---
 gdb/ChangeLog    |  7 +++++++
 gdb/dwarf2/loc.c | 15 +++++++++++----
 gdb/dwarf2/loc.h |  8 ++++++--
 3 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/gdb/dwarf2/loc.c b/gdb/dwarf2/loc.c
index 2ec4626b17c..957a2029798 100644
--- a/gdb/dwarf2/loc.c
+++ b/gdb/dwarf2/loc.c
@@ -2388,13 +2388,16 @@ dwarf2_evaluate_loc_desc (struct type *type, struct frame_info *frame,
    that the dwarf expression only produces a single CORE_ADDR.  FRAME is the
    frame in which the expression is evaluated.  ADDR is a context (location of
    a variable) and might be needed to evaluate the location expression.
-   Returns 1 on success, 0 otherwise.   */
+   PUSH_INITIAL_VALUE is true if ADDR should be pushed on the stack
+   before evaluating the expression;  this is required by certain
+   forms of DWARF expression.  Returns 1 on success, 0 otherwise.  */
 
 static int
 dwarf2_locexpr_baton_eval (const struct dwarf2_locexpr_baton *dlbaton,
    struct frame_info *frame,
    CORE_ADDR addr,
-   CORE_ADDR *valp)
+   CORE_ADDR *valp,
+   bool push_initial_value)
 {
   struct objfile *objfile;
 
@@ -2414,6 +2417,9 @@ dwarf2_locexpr_baton_eval (const struct dwarf2_locexpr_baton *dlbaton,
   ctx.ref_addr_size = dlbaton->per_cu->ref_addr_size ();
   ctx.offset = dlbaton->per_cu->text_offset ();
 
+  if (push_initial_value)
+    ctx.push_address (addr, false);
+
   try
     {
       ctx.eval (dlbaton->data, dlbaton->size);
@@ -2462,7 +2468,8 @@ bool
 dwarf2_evaluate_property (const struct dynamic_prop *prop,
   struct frame_info *frame,
   const struct property_addr_info *addr_stack,
-  CORE_ADDR *value)
+  CORE_ADDR *value,
+  bool push_initial_value)
 {
   if (prop == NULL)
     return false;
@@ -2480,7 +2487,7 @@ dwarf2_evaluate_property (const struct dynamic_prop *prop,
 
  if (dwarf2_locexpr_baton_eval (&baton->locexpr, frame,
        addr_stack ? addr_stack->addr : 0,
-       value))
+       value, push_initial_value))
   {
     if (baton->locexpr.is_reference)
       {
diff --git a/gdb/dwarf2/loc.h b/gdb/dwarf2/loc.h
index a59d3f998fd..6ff9b79dc03 100644
--- a/gdb/dwarf2/loc.h
+++ b/gdb/dwarf2/loc.h
@@ -92,12 +92,16 @@ struct property_addr_info
    be NULL.
 
    Returns true if PROP could be converted and the static value is passed
-   back into VALUE, otherwise returns false.  */
+   back into VALUE, otherwise returns false.
+
+   If PUSH_INITIAL_VALUE is true, then the top value of ADDR_STACK
+   will be pushed before evaluating a location expression.  */
 
 bool dwarf2_evaluate_property (const struct dynamic_prop *prop,
        struct frame_info *frame,
        const struct property_addr_info *addr_stack,
-       CORE_ADDR *value);
+       CORE_ADDR *value,
+       bool push_initial_value = false);
 
 /* A helper for the compiler interface that compiles a single dynamic
    property to C code.
--
2.21.1

Reply | Threaded
Open this post in threaded view
|

[PATCH 04/11] Prefer existing data when evaluating DWARF expression

Tom Tromey-4
In reply to this post by Tom Tromey-4
When evaluating a DWARF expression, the dynamic type resolution code
will pass in a buffer of bytes via the property_addr_info.  However,
the DWARF expression evaluator will then proceed to read memory from
the inferior, even when the request could be filled from this buffer.

This, in turn, is a problem in some cases; and specifically when
trying to handle the Ada scenario of extracting a variable-length
value from a packed array.  Here, the ordinary DWARF expression cannot
be directly evaluated, because the data may appear at some arbitrary
bit offset.  So, it is unpacked into a staging area and then the
expression is evaluated -- using an address of 0.

This patch fixes the problem by arranging for the DWARF evaluator, in
this case, to prefer passed-in memory when possible.  The type of the
buffer in the property_addr_info is changed to an array_view so that
bounds checking can be done.

gdb/ChangeLog
2020-04-08  Tom Tromey  <[hidden email]>

        * ada-lang.c (ada_discrete_type_high_bound, ada_discrete_type_low)
        (ada_value_primitive_packed_val): Update.
        * ada-valprint.c (ada_value_print_1): Update.
        * dwarf2/loc.c (evaluate_for_locexpr_baton): New struct.
        (dwarf2_locexpr_baton_eval): Take a property_addr_info rather than
        just an address.  Use evaluate_for_locexpr_baton.
        (dwarf2_evaluate_property): Update.
        * dwarf2/loc.h (struct property_addr_info) <valaddr>: Now an
        array_view.
        * findvar.c (default_read_var_value): Update.
        * gdbtypes.c (compute_variant_fields_inner)
        (resolve_dynamic_type_internal): Update.
        (resolve_dynamic_type): Change type of valaddr parameter.
        * gdbtypes.h (resolve_dynamic_type): Update.
        * valarith.c (value_subscripted_rvalue): Update.
        * value.c (value_from_contents_and_address): Update.
---
 gdb/ChangeLog      | 19 ++++++++++++
 gdb/ada-lang.c     |  6 ++--
 gdb/ada-valprint.c |  4 ++-
 gdb/dwarf2/loc.c   | 73 ++++++++++++++++++++++++++++++++++++----------
 gdb/dwarf2/loc.h   |  2 +-
 gdb/findvar.c      |  6 ++--
 gdb/gdbtypes.c     | 15 ++++++----
 gdb/gdbtypes.h     |  6 ++--
 gdb/valarith.c     |  2 +-
 gdb/value.c        |  5 +++-
 10 files changed, 104 insertions(+), 34 deletions(-)

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 029a7912a03..2a61e3d3074 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -769,7 +769,7 @@ min_of_type (struct type *t)
 LONGEST
 ada_discrete_type_high_bound (struct type *type)
 {
-  type = resolve_dynamic_type (type, NULL, 0);
+  type = resolve_dynamic_type (type, {}, 0);
   switch (TYPE_CODE (type))
     {
     case TYPE_CODE_RANGE:
@@ -790,7 +790,7 @@ ada_discrete_type_high_bound (struct type *type)
 LONGEST
 ada_discrete_type_low_bound (struct type *type)
 {
-  type = resolve_dynamic_type (type, NULL, 0);
+  type = resolve_dynamic_type (type, {}, 0);
   switch (TYPE_CODE (type))
     {
     case TYPE_CODE_RANGE:
@@ -2525,7 +2525,7 @@ ada_value_primitive_packed_val (struct value *obj, const gdb_byte *valaddr,
         staging.data (), staging.size (),
  is_big_endian, has_negatives (type),
  is_scalar);
-      type = resolve_dynamic_type (type, staging.data (), 0);
+      type = resolve_dynamic_type (type, staging, 0);
       if (TYPE_LENGTH (type) < (bit_size + HOST_CHAR_BIT - 1) / HOST_CHAR_BIT)
  {
   /* This happens when the length of the object is dynamic,
diff --git a/gdb/ada-valprint.c b/gdb/ada-valprint.c
index 2768829cdb3..474b0799910 100644
--- a/gdb/ada-valprint.c
+++ b/gdb/ada-valprint.c
@@ -1057,7 +1057,9 @@ ada_value_print_1 (struct value *val, struct ui_file *stream, int recurse,
 
   const gdb_byte *valaddr = value_contents_for_printing (val);
   CORE_ADDR address = value_address (val);
-  type = ada_check_typedef (resolve_dynamic_type (type, valaddr, address));
+  gdb::array_view<const gdb_byte> view
+    = gdb::make_array_view (valaddr, TYPE_LENGTH (type));
+  type = ada_check_typedef (resolve_dynamic_type (type, view, address));
   if (type != saved_type)
     {
       val = value_copy (val);
diff --git a/gdb/dwarf2/loc.c b/gdb/dwarf2/loc.c
index 957a2029798..2eced0e3381 100644
--- a/gdb/dwarf2/loc.c
+++ b/gdb/dwarf2/loc.c
@@ -2384,18 +2384,56 @@ dwarf2_evaluate_loc_desc (struct type *type, struct frame_info *frame,
  NULL, 0);
 }
 
-/* Evaluates a dwarf expression and stores the result in VAL, expecting
-   that the dwarf expression only produces a single CORE_ADDR.  FRAME is the
-   frame in which the expression is evaluated.  ADDR is a context (location of
-   a variable) and might be needed to evaluate the location expression.
-   PUSH_INITIAL_VALUE is true if ADDR should be pushed on the stack
-   before evaluating the expression;  this is required by certain
-   forms of DWARF expression.  Returns 1 on success, 0 otherwise.  */
+/* A specialization of dwarf_evaluate_loc_desc that is used by
+   dwarf2_locexpr_baton_eval.  This subclass exists to handle the case
+   where a caller of dwarf2_locexpr_baton_eval passes in some data,
+   but with the address being 0.  In this situation, we arrange for
+   memory reads to come from the passed-in buffer.  */
+
+struct evaluate_for_locexpr_baton : public dwarf_evaluate_loc_desc
+{
+  /* The data that was passed in.  */
+  gdb::array_view<const gdb_byte> data_view;
+
+  CORE_ADDR get_object_address () override
+  {
+    if (data_view.data () == nullptr && obj_address == 0)
+      error (_("Location address is not set."));
+    return obj_address;
+  }
+
+  void read_mem (gdb_byte *buf, CORE_ADDR addr, size_t len) override
+  {
+    if (len == 0)
+      return;
+
+    /* Prefer the passed-in memory, if it exists.  */
+    CORE_ADDR offset = addr - obj_address;
+    if (offset < data_view.size () && offset + len <= data_view.size ())
+      {
+ memcpy (buf, data_view.data (), len);
+ return;
+      }
+
+    read_memory (addr, buf, len);
+  }
+};
+
+/* Evaluates a dwarf expression and stores the result in VAL,
+   expecting that the dwarf expression only produces a single
+   CORE_ADDR.  FRAME is the frame in which the expression is
+   evaluated.  ADDR_STACK is a context (location of a variable) and
+   might be needed to evaluate the location expression.
+   PUSH_INITIAL_VALUE is true if the address (either from ADDR_STACK,
+   or the default of 0) should be pushed on the DWARF expression
+   evaluation stack before evaluating the expression; this is required
+   by certain forms of DWARF expression.  Returns 1 on success, 0
+   otherwise.  */
 
 static int
 dwarf2_locexpr_baton_eval (const struct dwarf2_locexpr_baton *dlbaton,
    struct frame_info *frame,
-   CORE_ADDR addr,
+   const struct property_addr_info *addr_stack,
    CORE_ADDR *valp,
    bool push_initial_value)
 {
@@ -2404,11 +2442,17 @@ dwarf2_locexpr_baton_eval (const struct dwarf2_locexpr_baton *dlbaton,
   if (dlbaton == NULL || dlbaton->size == 0)
     return 0;
 
-  dwarf_evaluate_loc_desc ctx;
+  evaluate_for_locexpr_baton ctx;
 
   ctx.frame = frame;
   ctx.per_cu = dlbaton->per_cu;
-  ctx.obj_address = addr;
+  if (addr_stack == nullptr)
+    ctx.obj_address = 0;
+  else
+    {
+      ctx.obj_address = addr_stack->addr;
+      ctx.data_view = addr_stack->valaddr;
+    }
 
   objfile = dlbaton->per_cu->objfile ();
 
@@ -2418,7 +2462,7 @@ dwarf2_locexpr_baton_eval (const struct dwarf2_locexpr_baton *dlbaton,
   ctx.offset = dlbaton->per_cu->text_offset ();
 
   if (push_initial_value)
-    ctx.push_address (addr, false);
+    ctx.push_address (ctx.obj_address, false);
 
   try
     {
@@ -2485,8 +2529,7 @@ dwarf2_evaluate_property (const struct dynamic_prop *prop,
   = (const struct dwarf2_property_baton *) prop->data.baton;
  gdb_assert (baton->property_type != NULL);
 
- if (dwarf2_locexpr_baton_eval (&baton->locexpr, frame,
-       addr_stack ? addr_stack->addr : 0,
+ if (dwarf2_locexpr_baton_eval (&baton->locexpr, frame, addr_stack,
        value, push_initial_value))
   {
     if (baton->locexpr.is_reference)
@@ -2569,10 +2612,10 @@ dwarf2_evaluate_property (const struct dynamic_prop *prop,
   }
  if (pinfo == NULL)
   error (_("cannot find reference address for offset property"));
- if (pinfo->valaddr != NULL)
+ if (pinfo->valaddr.data () != NULL)
   val = value_from_contents
   (baton->offset_info.type,
-   pinfo->valaddr + baton->offset_info.offset);
+   pinfo->valaddr.data () + baton->offset_info.offset);
  else
   val = value_at (baton->offset_info.type,
   pinfo->addr + baton->offset_info.offset);
diff --git a/gdb/dwarf2/loc.h b/gdb/dwarf2/loc.h
index 6ff9b79dc03..9815368d625 100644
--- a/gdb/dwarf2/loc.h
+++ b/gdb/dwarf2/loc.h
@@ -73,7 +73,7 @@ struct property_addr_info
   struct type *type;
 
   /* If not NULL, a buffer containing the object's value.  */
-  const gdb_byte *valaddr;
+  gdb::array_view<const gdb_byte> valaddr;
 
   /* The address of that object.  */
   CORE_ADDR addr;
diff --git a/gdb/findvar.c b/gdb/findvar.c
index a836c63dc5d..ac4f5c3997d 100644
--- a/gdb/findvar.c
+++ b/gdb/findvar.c
@@ -615,7 +615,7 @@ default_read_var_value (struct symbol *var, const struct block *var_block,
       if (is_dynamic_type (type))
  {
   /* Value is a constant byte-sequence and needs no memory access.  */
-  type = resolve_dynamic_type (type, NULL, /* Unused address.  */ 0);
+  type = resolve_dynamic_type (type, {}, /* Unused address.  */ 0);
  }
       /* Put the constant back in target format. */
       v = allocate_value (type);
@@ -647,7 +647,7 @@ default_read_var_value (struct symbol *var, const struct block *var_block,
       if (is_dynamic_type (type))
  {
   /* Value is a constant byte-sequence and needs no memory access.  */
-  type = resolve_dynamic_type (type, NULL, /* Unused address.  */ 0);
+  type = resolve_dynamic_type (type, {}, /* Unused address.  */ 0);
  }
       v = allocate_value (type);
       memcpy (value_contents_raw (v), SYMBOL_VALUE_BYTES (var),
@@ -788,7 +788,7 @@ default_read_var_value (struct symbol *var, const struct block *var_block,
 
     case LOC_OPTIMIZED_OUT:
       if (is_dynamic_type (type))
- type = resolve_dynamic_type (type, NULL, /* Unused address.  */ 0);
+ type = resolve_dynamic_type (type, {}, /* Unused address.  */ 0);
       return allocate_optimized_out_value (type);
 
     default:
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 6b32fc9a8ff..9d0ebaa159e 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -2298,8 +2298,9 @@ compute_variant_fields_inner (struct type *type,
  error (_("Cannot determine struct field location"
  " (invalid location kind)"));
 
-      if (addr_stack->valaddr != NULL)
- discr_value = unpack_field_as_long (type, addr_stack->valaddr, idx);
+      if (addr_stack->valaddr.data () != NULL)
+ discr_value = unpack_field_as_long (type, addr_stack->valaddr.data (),
+    idx);
       else
  {
   CORE_ADDR addr = (addr_stack->addr
@@ -2516,9 +2517,10 @@ resolve_dynamic_type_internal (struct type *type,
     struct property_addr_info pinfo;
 
     pinfo.type = check_typedef (TYPE_TARGET_TYPE (type));
-    pinfo.valaddr = NULL;
-    if (addr_stack->valaddr != NULL)
-      pinfo.addr = extract_typed_address (addr_stack->valaddr, type);
+    pinfo.valaddr = {};
+    if (addr_stack->valaddr.data () != NULL)
+      pinfo.addr = extract_typed_address (addr_stack->valaddr.data (),
+  type);
     else
       pinfo.addr = read_memory_typed_address (addr_stack->addr, type);
     pinfo.next = addr_stack;
@@ -2566,7 +2568,8 @@ resolve_dynamic_type_internal (struct type *type,
 /* See gdbtypes.h  */
 
 struct type *
-resolve_dynamic_type (struct type *type, const gdb_byte *valaddr,
+resolve_dynamic_type (struct type *type,
+      gdb::array_view<const gdb_byte> valaddr,
       CORE_ADDR addr)
 {
   struct property_addr_info pinfo
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index 8b4da6e3f9f..f686e5407ba 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -2140,9 +2140,9 @@ extern void get_signed_type_minmax (struct type *, LONGEST *, LONGEST *);
    ADDR specifies the location of the variable the type is bound to.
    If TYPE has no dynamic properties return TYPE; otherwise a new type with
    static properties is returned.  */
-extern struct type *resolve_dynamic_type (struct type *type,
-  const gdb_byte *valaddr,
-  CORE_ADDR addr);
+extern struct type *resolve_dynamic_type
+  (struct type *type, gdb::array_view<const gdb_byte> valaddr,
+   CORE_ADDR addr);
 
 /* * Predicate if the type has dynamic values, which are not resolved yet.  */
 extern int is_dynamic_type (struct type *type);
diff --git a/gdb/valarith.c b/gdb/valarith.c
index 07cb5014bb2..504264b1d82 100644
--- a/gdb/valarith.c
+++ b/gdb/valarith.c
@@ -220,7 +220,7 @@ value_subscripted_rvalue (struct value *array, LONGEST index, LONGEST lowerbound
       CORE_ADDR address;
 
       address = value_address (array) + elt_offs;
-      elt_type = resolve_dynamic_type (elt_type, NULL, address);
+      elt_type = resolve_dynamic_type (elt_type, {}, address);
     }
 
   return value_from_component (array, elt_type, elt_offs);
diff --git a/gdb/value.c b/gdb/value.c
index 641c09a0624..855fc80b6a6 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -3477,7 +3477,10 @@ value_from_contents_and_address (struct type *type,
  const gdb_byte *valaddr,
  CORE_ADDR address)
 {
-  struct type *resolved_type = resolve_dynamic_type (type, valaddr, address);
+  gdb::array_view<const gdb_byte> view;
+  if (valaddr != nullptr)
+    view = gdb::make_array_view (valaddr, TYPE_LENGTH (type));
+  struct type *resolved_type = resolve_dynamic_type (type, view, address);
   struct type *resolved_type_no_typedef = check_typedef (resolved_type);
   struct value *v;
 
--
2.21.1

Reply | Threaded
Open this post in threaded view
|

[PATCH 05/11] Rewrite the existing variant part code

Tom Tromey-4
In reply to this post by Tom Tromey-4
This rewrites the existing variant part code to follow the new model
implemented in the previous patch.  The old variant part code is
removed.

This only affects Rust for the moment.  I tested this using various
version of the Rust compiler, including one that emits old-style enum
debuginfo, exercising the quirks code.

gdb/ChangeLog
2020-04-08  Tom Tromey  <[hidden email]>

        * dwarf2/read.c (struct variant_field): Rewrite.
        (struct variant_part_builder): New.
        (struct nextfield): Remove "variant" field.  Add "offset".
        (struct field_info): Add "current_variant_part" and
        "variant_parts".
        (alloc_discriminant_info): Remove.
        (alloc_rust_variant): New function.
        (quirk_rust_enum): Update.
        (dwarf2_add_field): Set "offset" member.  Don't handle
        DW_TAG_variant_part.
        (offset_map_type): New typedef.
        (convert_variant_range, create_one_variant)
        (create_one_variant_part, create_variant_parts)
        (add_variant_property): New functions.
        (dwarf2_attach_fields_to_type): Call add_variant_property.
        (read_structure_type): Don't handle DW_TAG_variant_part.
        (handle_variant_part, handle_variant): New functions.
        (handle_struct_member_die): Use them.
        (process_structure_scope): Don't handle variant parts.
        * gdbtypes.h (TYPE_FLAG_DISCRIMINATED_UNION): Remove.
        (struct discriminant_info): Remove.
        (enum dynamic_prop_node_kind) <DYN_PROP_DISCRIMINATED>: Remove.
        (struct main_type) <flag_discriminated_union>: Remove.
        * rust-lang.c (rust_enum_p, rust_empty_enum_p): Rewrite.
        (rust_enum_variant): Return int.  Remove "contents".  Rewrite.
        (rust_print_enum, rust_print_struct_def, rust_evaluate_subexp):
        Update.
        * valops.c (value_union_variant): Remove.
        * value.h (value_union_variant): Don't declare.
---
 gdb/ChangeLog     |  32 ++
 gdb/dwarf2/read.c | 723 ++++++++++++++++++++++++++++++----------------
 gdb/gdbtypes.h    |  51 ----
 gdb/rust-lang.c   | 115 +++-----
 gdb/valops.c      |  44 ---
 gdb/value.h       |   8 -
 6 files changed, 558 insertions(+), 415 deletions(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index da702205c60..c0bced60384 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -1082,29 +1082,52 @@ struct partial_die_info : public allocate_on_obstack
    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_part_builder;
+
+/* When reading a variant, 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;
+  int first_field = -1;
+  int last_field = -1;
+
+  /* A variant can contain other variant parts.  */
+  std::vector<variant_part_builder> variant_parts;
+
   /* 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;
+  bool default_branch = false;
+  /* If we see a DW_AT_discr_value, then this will be the discriminant
+     value.  */
+  ULONGEST discriminant_value = 0;
+  /* If we see a DW_AT_discr_list, then this is a pointer to the list
+     data.  */
+  struct dwarf_block *discr_list_data = nullptr;
+};
+
+/* This represents a DW_TAG_variant_part.  */
+
+struct variant_part_builder
+{
+  /* The offset of the discriminant field.  */
+  sect_offset discriminant_offset {};
+
+  /* Variants that are direct children of this variant part.  */
+  std::vector<variant_field> variants;
+
+  /* True if we're currently reading a variant.  */
+  bool processing_variant = false;
 };
 
 struct nextfield
 {
   int accessibility = 0;
   int virtuality = 0;
-  /* Extra information to describe a variant or variant part.  */
-  struct variant_field variant {};
+  /* Variant parts need to find the discriminant, which is a DIE
+     reference.  We track the section offset of each field to make
+     this link.  */
+  sect_offset offset;
   struct field field {};
 };
 
@@ -1139,6 +1162,13 @@ struct field_info
        list.  */
     std::vector<struct decl_field> nested_types_list;
 
+    /* If non-null, this is the variant part we are currently
+       reading.  */
+    variant_part_builder *current_variant_part = nullptr;
+    /* This holds all the top-level variant parts attached to the type
+       we're reading.  */
+    std::vector<variant_part_builder> variant_parts;
+
     /* Return the total number of fields (including baseclasses).  */
     int nfields () const
     {
@@ -9073,37 +9103,72 @@ 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.  */
+/* 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.
+   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
+   must hold the discriminant values used by the variants.  Note that
+   RANGES must have a lifetime at least as long as OBSTACK -- either
+   already allocated on it, or static.  */
 
-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 (discriminant_index == -1
-      || (discriminant_index >= 0
-  && discriminant_index < TYPE_NFIELDS (type)));
+static void
+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_NFIELDS (type));
   gdb_assert (default_index == -1
       || (default_index >= 0 && default_index < TYPE_NFIELDS (type)));
 
-  TYPE_FLAG_DISCRIMINATED_UNION (type) = 1;
+  /* We have one variant for each non-discriminant field.  */
+  int n_variants = TYPE_NFIELDS (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;
+  variant *variants = new (obstack) variant[n_variants];
+  int var_idx = 0;
+  int range_idx = 0;
+  for (int i = 0; i < TYPE_NFIELDS (type); ++i)
+    {
+      if (i == discriminant_index)
+ continue;
 
-  struct dynamic_prop prop;
-  prop.kind = PROP_UNDEFINED;
-  prop.data.baton = disc;
+      variants[var_idx].first_field = i;
+      variants[var_idx].last_field = i + 1;
+
+      /* The default field does not need a range, but other fields do.
+ We skipped the discriminant above.  */
+      if (i != default_index)
+ {
+  variants[var_idx].discriminants = ranges.slice (range_idx, 1);
+  ++range_idx;
+ }
+
+      ++var_idx;
+    }
+
+  gdb_assert (range_idx == ranges.size ());
+  gdb_assert (var_idx == n_variants);
 
-  add_dyn_prop (DYN_PROP_DISCRIMINATED, prop, type);
+  variant_part *part = new (obstack) variant_part;
+  part->discriminant_index = discriminant_index;
+  part->is_unsigned = TYPE_UNSIGNED (TYPE_FIELD_TYPE (type,
+      discriminant_index));
+  part->variants = gdb::array_view<variant> (variants, n_variants);
 
-  return disc;
+  void *storage = obstack_alloc (obstack, sizeof (gdb::array_view<variant_part>));
+  gdb::array_view<variant_part> *prop_value
+    = new (storage) gdb::array_view<variant_part> (part, 1);
+
+  struct dynamic_prop prop;
+  prop.kind = PROP_VARIANT_PARTS;
+  prop.data.variant_parts = prop_value;
+
+  add_dyn_prop (DYN_PROP_VARIANT_PARTS, prop, type);
 }
 
 /* Some versions of rustc emitted enums in an unusual way.
@@ -9167,55 +9232,44 @@ quirk_rust_enum (struct type *type, struct objfile *objfile)
   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)
+      /* 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) = 3;
+      /* Save the field we care about.  */
+      struct field saved_field = TYPE_FIELD (type, 0);
+      TYPE_FIELDS (type)
  = (struct field *) TYPE_ZALLOC (type, 3 * sizeof (struct field));
-      TYPE_LENGTH (union_type) = TYPE_LENGTH (type);
-      set_type_align (union_type, TYPE_RAW_ALIGN (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);
+      /* Put the discriminant at index 0.  */
+      TYPE_FIELD_TYPE (type, 0) = field_type;
+      TYPE_FIELD_ARTIFICIAL (type, 0) = 1;
+      TYPE_FIELD_NAME (type, 0) = "<<discriminant>>";
+      SET_FIELD_BITPOS (TYPE_FIELD (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))
+      TYPE_FIELD (type, 1) = saved_field;
+      TYPE_FIELD_NAME (type, 1)
+ = rust_last_path_segment (TYPE_NAME (TYPE_FIELD_TYPE (type, 1)));
+      TYPE_NAME (TYPE_FIELD_TYPE (type, 1))
  = rust_fully_qualify (&objfile->objfile_obstack, TYPE_NAME (type),
-      TYPE_FIELD_NAME (union_type, 1));
+      TYPE_FIELD_NAME (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;
+      TYPE_FIELD_TYPE (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));
+      TYPE_FIELD_NAME (type, 2) = name;
+      SET_FIELD_BITPOS (TYPE_FIELD (type, 2), 0);
 
-      /* 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>>";
+      /* Indicate that this is a variant type.  */
+      static discriminant_range ranges[1] = { { 0, 0 } };
+      alloc_rust_variant (&objfile->objfile_obstack, type, 0, 1, ranges);
     }
   /* A union with a single anonymous field is probably an old-style
      univariant enum.  */
@@ -9225,31 +9279,13 @@ quirk_rust_enum (struct type *type, struct objfile *objfile)
  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);
-      set_type_align (union_type, TYPE_RAW_ALIGN (type));
-      TYPE_FIELDS (union_type) = TYPE_FIELDS (type);
-
-      struct type *field_type = TYPE_FIELD_TYPE (union_type, 0);
+      struct type *field_type = TYPE_FIELD_TYPE (type, 0);
       const char *variant_name
  = rust_last_path_segment (TYPE_NAME (field_type));
-      TYPE_FIELD_NAME (union_type, 0) = variant_name;
+      TYPE_FIELD_NAME (type, 0) = variant_name;
       TYPE_NAME (field_type)
  = rust_fully_qualify (&objfile->objfile_obstack,
       TYPE_NAME (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
     {
@@ -9290,33 +9326,20 @@ quirk_rust_enum (struct type *type, struct objfile *objfile)
  because the type has already been recorded.  */
       TYPE_CODE (type) = TYPE_CODE_STRUCT;
 
-      /* Make a union to hold the variants.  */
+      /* Make space for the discriminant field.  */
       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);
-      set_type_align (union_type, TYPE_RAW_ALIGN (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),
+      field *new_fields
+ = (struct field *) TYPE_ZALLOC (type, (TYPE_NFIELDS (type)
+       * sizeof (struct field)));
+      memcpy (new_fields + 1, TYPE_FIELDS (type),
       TYPE_NFIELDS (type) * sizeof (struct field));
+      TYPE_FIELDS (type) = new_fields;
+      TYPE_NFIELDS (type) = TYPE_NFIELDS (type) + 1;
 
       /* 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);
+      TYPE_FIELD (type, 0) = *disr_field;
+      TYPE_FIELD_ARTIFICIAL (type, 0) = 1;
+      TYPE_FIELD_NAME (type, 0) = "<<discriminant>>";
 
       /* We need a way to find the correct discriminant given a
  variant name.  For convenience we build a map here.  */
@@ -9332,9 +9355,13 @@ quirk_rust_enum (struct type *type, struct objfile *objfile)
     }
  }
 
-      int n_fields = TYPE_NFIELDS (union_type);
-      struct discriminant_info *disc
- = alloc_discriminant_info (union_type, 0, -1);
+      int n_fields = TYPE_NFIELDS (type);
+      /* We don't need a range entry for the discriminant, but we do
+ need one for every other field, as there is no default
+ variant.  */
+      discriminant_range *ranges = XOBNEWVEC (&objfile->objfile_obstack,
+      discriminant_range,
+      n_fields - 1);
       /* Skip the discriminant here.  */
       for (int i = 1; i < n_fields; ++i)
  {
@@ -9342,25 +9369,32 @@ quirk_rust_enum (struct type *type, struct objfile *objfile)
      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)));
+    = rust_last_path_segment (TYPE_NAME (TYPE_FIELD_TYPE (type, i)));
 
   auto iter = discriminant_map.find (variant_name);
   if (iter != discriminant_map.end ())
-    disc->discriminants[i] = iter->second;
+    {
+      ranges[i].low = iter->second;
+      ranges[i].high = iter->second;
+    }
 
   /* Remove the discriminant field, if it exists.  */
-  struct type *sub_type = TYPE_FIELD_TYPE (union_type, i);
+  struct type *sub_type = TYPE_FIELD_TYPE (type, i);
   if (TYPE_NFIELDS (sub_type) > 0)
     {
       --TYPE_NFIELDS (sub_type);
       ++TYPE_FIELDS (sub_type);
     }
-  TYPE_FIELD_NAME (union_type, i) = variant_name;
+  TYPE_FIELD_NAME (type, i) = variant_name;
   TYPE_NAME (sub_type)
     = rust_fully_qualify (&objfile->objfile_obstack,
   TYPE_NAME (type), variant_name);
  }
+
+      /* Indicate that this is a variant type.  */
+      alloc_rust_variant (&objfile->objfile_obstack, type, 0, 1,
+  gdb::array_view<discriminant_range> (ranges,
+       n_fields - 1));
     }
 }
 
@@ -14159,6 +14193,8 @@ dwarf2_add_field (struct field_info *fip, struct die_info *die,
       new_field = &fip->fields.back ();
     }
 
+  new_field->offset = die->sect_off;
+
   attr = dwarf2_attr (die, DW_AT_accessibility, cu);
   if (attr != nullptr)
     new_field->accessibility = DW_UNSND (attr);
@@ -14317,35 +14353,6 @@ dwarf2_add_field (struct field_info *fip, struct die_info *die,
       FIELD_TYPE (*fp) = die_type (die, cu);
       FIELD_NAME (*fp) = TYPE_NAME (fp->type);
     }
-  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>>";
-
-      /* Normally a DW_TAG_variant_part won't have a size, but our
- representation requires one, so set it to the maximum of the
- child sizes, being sure to account for the offset at which
- each child is seen.  */
-      if (TYPE_LENGTH (fp->type) == 0)
- {
-  unsigned max = 0;
-  for (int i = 0; i < TYPE_NFIELDS (fp->type); ++i)
-    {
-      unsigned len = ((TYPE_FIELD_BITPOS (fp->type, i) + 7) / 8
-      + TYPE_LENGTH (TYPE_FIELD_TYPE (fp->type, i)));
-      if (len > max)
- max = len;
-    }
-  TYPE_LENGTH (fp->type) = max;
- }
-    }
   else
     gdb_assert_not_reached ("missing case in dwarf2_add_field");
 }
@@ -14412,6 +14419,201 @@ dwarf2_add_type_defn (struct field_info *fip, struct die_info *die,
     fip->nested_types_list.push_back (fp);
 }
 
+/* A convenience typedef that's used when finding the discriminant
+   field for a variant part.  */
+typedef std::unordered_map<sect_offset, int> offset_map_type;
+
+/* Compute the discriminant range for a given variant.  OBSTACK is
+   where the results will be stored.  VARIANT is the variant to
+   process.  IS_UNSIGNED indicates whether the discriminant is signed
+   or unsigned.  */
+
+static const gdb::array_view<discriminant_range>
+convert_variant_range (struct obstack *obstack, const variant_field &variant,
+       bool is_unsigned)
+{
+  std::vector<discriminant_range> ranges;
+
+  if (variant.default_branch)
+    return {};
+
+  if (variant.discr_list_data == nullptr)
+    {
+      discriminant_range r
+ = {variant.discriminant_value, variant.discriminant_value};
+      ranges.push_back (r);
+    }
+  else
+    {
+      gdb::array_view<const gdb_byte> data (variant.discr_list_data->data,
+    variant.discr_list_data->size);
+      while (!data.empty ())
+ {
+  if (data[0] != DW_DSC_range && data[0] != DW_DSC_label)
+    {
+      complaint (_("invalid discriminant marker: %d"), data[0]);
+      break;
+    }
+  bool is_range = data[0] == DW_DSC_range;
+  data = data.slice (1);
+
+  ULONGEST low, high;
+  unsigned int bytes_read;
+
+  if (data.empty ())
+    {
+      complaint (_("DW_AT_discr_list missing low value"));
+      break;
+    }
+  if (is_unsigned)
+    low = read_unsigned_leb128 (nullptr, data.data (), &bytes_read);
+  else
+    low = (ULONGEST) read_signed_leb128 (nullptr, data.data (),
+ &bytes_read);
+  data = data.slice (bytes_read);
+
+  if (is_range)
+    {
+      if (data.empty ())
+ {
+  complaint (_("DW_AT_discr_list missing high value"));
+  break;
+ }
+      if (is_unsigned)
+ high = read_unsigned_leb128 (nullptr, data.data (),
+     &bytes_read);
+      else
+ high = (LONGEST) read_signed_leb128 (nullptr, data.data (),
+     &bytes_read);
+      data = data.slice (bytes_read);
+    }
+  else
+    high = low;
+
+  ranges.push_back ({ low, high });
+ }
+    }
+
+  discriminant_range *result = XOBNEWVEC (obstack, discriminant_range,
+  ranges.size ());
+  std::copy (ranges.begin (), ranges.end (), result);
+  return gdb::array_view<discriminant_range> (result, ranges.size ());
+}
+
+static const gdb::array_view<variant_part> create_variant_parts
+  (struct obstack *obstack,
+   const offset_map_type &offset_map,
+   struct field_info *fi,
+   const std::vector<variant_part_builder> &variant_parts);
+
+/* Fill in a "struct variant" for a given variant field.  RESULT is
+   the variant to fill in.  OBSTACK is where any needed allocations
+   will be done.  OFFSET_MAP holds the mapping from section offsets to
+   fields for the type.  FI describes the fields of the type we're
+   processing.  FIELD is the variant field we're converting.  */
+
+static void
+create_one_variant (variant &result, struct obstack *obstack,
+    const offset_map_type &offset_map,
+    struct field_info *fi, const variant_field &field)
+{
+  result.discriminants = convert_variant_range (obstack, field, false);
+  result.first_field = field.first_field + fi->baseclasses.size ();
+  result.last_field = field.last_field + fi->baseclasses.size ();
+  result.parts = create_variant_parts (obstack, offset_map, fi,
+       field.variant_parts);
+}
+
+/* Fill in a "struct variant_part" for a given variant part.  RESULT
+   is the variant part to fill in.  OBSTACK is where any needed
+   allocations will be done.  OFFSET_MAP holds the mapping from
+   section offsets to fields for the type.  FI describes the fields of
+   the type we're processing.  BUILDER is the variant part to be
+   converted.  */
+
+static void
+create_one_variant_part (variant_part &result,
+ struct obstack *obstack,
+ const offset_map_type &offset_map,
+ struct field_info *fi,
+ const variant_part_builder &builder)
+{
+  auto iter = offset_map.find (builder.discriminant_offset);
+  if (iter == offset_map.end ())
+    {
+      result.discriminant_index = -1;
+      /* Doesn't matter.  */
+      result.is_unsigned = false;
+    }
+  else
+    {
+      result.discriminant_index = iter->second;
+      result.is_unsigned
+ = TYPE_UNSIGNED (FIELD_TYPE
+ (fi->fields[result.discriminant_index].field));
+    }
+
+  size_t n = builder.variants.size ();
+  variant *output = new (obstack) variant[n];
+  for (size_t i = 0; i < n; ++i)
+    create_one_variant (output[i], obstack, offset_map, fi,
+ builder.variants[i]);
+
+  result.variants = gdb::array_view<variant> (output, n);
+}
+
+/* Create a vector of variant parts that can be attached to a type.
+   OBSTACK is where any needed allocations will be done.  OFFSET_MAP
+   holds the mapping from section offsets to fields for the type.  FI
+   describes the fields of the type we're processing.  VARIANT_PARTS
+   is the vector to convert.  */
+
+static const gdb::array_view<variant_part>
+create_variant_parts (struct obstack *obstack,
+      const offset_map_type &offset_map,
+      struct field_info *fi,
+      const std::vector<variant_part_builder> &variant_parts)
+{
+  if (variant_parts.empty ())
+    return {};
+
+  size_t n = variant_parts.size ();
+  variant_part *result = new (obstack) variant_part[n];
+  for (size_t i = 0; i < n; ++i)
+    create_one_variant_part (result[i], obstack, offset_map, fi,
+     variant_parts[i]);
+
+  return gdb::array_view<variant_part> (result, n);
+}
+
+/* Compute the variant part vector for FIP, attaching it to TYPE when
+   done.  */
+
+static void
+add_variant_property (struct field_info *fip, struct type *type,
+      struct dwarf2_cu *cu)
+{
+  /* Map section offsets of fields to their field index.  Note the
+     field index here does not take the number of baseclasses into
+     account.  */
+  offset_map_type offset_map;
+  for (int i = 0; i < fip->fields.size (); ++i)
+    offset_map[fip->fields[i].offset] = i;
+
+  struct objfile *objfile = cu->per_cu->dwarf2_per_objfile->objfile;
+  gdb::array_view<variant_part> parts
+    = create_variant_parts (&objfile->objfile_obstack, offset_map, fip,
+    fip->variant_parts);
+
+  struct dynamic_prop prop;
+  prop.kind = PROP_VARIANT_PARTS;
+  prop.data.variant_parts
+    = ((gdb::array_view<variant_part> *)
+       obstack_copy (&objfile->objfile_obstack, &parts, sizeof (parts)));
+
+  add_dyn_prop (DYN_PROP_VARIANT_PARTS, prop, type);
+}
+
 /* Create the vector of fields, and attach it to the type.  */
 
 static void
@@ -14457,22 +14659,8 @@ dwarf2_attach_fields_to_type (struct field_info *fip, struct type *type,
       TYPE_N_BASECLASSES (type) = fip->baseclasses.size ();
     }
 
-  if (TYPE_FLAG_DISCRIMINATED_UNION (type))
-    {
-      struct discriminant_info *di = alloc_discriminant_info (type, -1, -1);
-
-      for (int index = 0; index < nfields; ++index)
- {
-  struct nextfield &field = fip->fields[index];
-
-  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;
- }
-    }
+  if (!fip->variant_parts.empty ())
+    add_variant_property (fip, type, cu);
 
   /* Copy the saved-up fields into the field vector.  */
   for (int i = 0; i < nfields; ++i)
@@ -15042,11 +15230,6 @@ 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;
@@ -15120,6 +15303,130 @@ read_structure_type (struct die_info *die, struct dwarf2_cu *cu)
   return type;
 }
 
+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);
+
+/* A helper for handle_struct_member_die that handles
+   DW_TAG_variant_part.  */
+
+static void
+handle_variant_part (struct die_info *die, struct type *type,
+     struct field_info *fi,
+     std::vector<struct symbol *> *template_args,
+     struct dwarf2_cu *cu)
+{
+  variant_part_builder *new_part;
+  if (fi->current_variant_part == nullptr)
+    {
+      fi->variant_parts.emplace_back ();
+      new_part = &fi->variant_parts.back ();
+    }
+  else if (!fi->current_variant_part->processing_variant)
+    {
+      complaint (_("nested DW_TAG_variant_part seen "
+   "- DIE at %s [in module %s]"),
+ sect_offset_str (die->sect_off),
+ objfile_name (cu->per_cu->dwarf2_per_objfile->objfile));
+      return;
+    }
+  else
+    {
+      variant_field &current = fi->current_variant_part->variants.back ();
+      current.variant_parts.emplace_back ();
+      new_part = &current.variant_parts.back ();
+    }
+
+  /* When we recurse, we want callees to add to this new variant
+     part.  */
+  scoped_restore save_current_variant_part
+    = make_scoped_restore (&fi->current_variant_part, new_part);
+
+  struct attribute *discr = dwarf2_attr (die, DW_AT_discr, cu);
+  if (discr == NULL)
+    {
+      /* It's a univariant form, an extension we support.  */
+    }
+  else if (discr->form_is_ref ())
+    {
+      struct dwarf2_cu *target_cu = cu;
+      struct die_info *target_die = follow_die_ref (die, discr, &target_cu);
+
+      new_part->discriminant_offset = target_die->sect_off;
+    }
+  else
+    {
+      complaint (_("DW_AT_discr does not have DIE reference form"
+   " - DIE at %s [in module %s]"),
+ sect_offset_str (die->sect_off),
+ objfile_name (cu->per_cu->dwarf2_per_objfile->objfile));
+    }
+
+  for (die_info *child_die = die->child;
+       child_die != NULL;
+       child_die = child_die->sibling)
+    handle_struct_member_die (child_die, type, fi, template_args, cu);
+}
+
+/* A helper for handle_struct_member_die that handles
+   DW_TAG_variant.  */
+
+static void
+handle_variant (struct die_info *die, struct type *type,
+ struct field_info *fi,
+ std::vector<struct symbol *> *template_args,
+ struct dwarf2_cu *cu)
+{
+  if (fi->current_variant_part == nullptr)
+    {
+      complaint (_("saw DW_TAG_variant outside DW_TAG_variant_part "
+   "- DIE at %s [in module %s]"),
+ sect_offset_str (die->sect_off),
+ objfile_name (cu->per_cu->dwarf2_per_objfile->objfile));
+      return;
+    }
+  if (fi->current_variant_part->processing_variant)
+    {
+      complaint (_("nested DW_TAG_variant seen "
+   "- DIE at %s [in module %s]"),
+ sect_offset_str (die->sect_off),
+ objfile_name (cu->per_cu->dwarf2_per_objfile->objfile));
+      return;
+    }
+
+  scoped_restore save_processing_variant
+    = make_scoped_restore (&fi->current_variant_part->processing_variant,
+   true);
+
+  fi->current_variant_part->variants.emplace_back ();
+  variant_field &variant = fi->current_variant_part->variants.back ();
+  variant.first_field = fi->fields.size ();
+
+  /* In a variant we want to get the discriminant and also add a
+     field for our sole member child.  */
+  struct attribute *discr = dwarf2_attr (die, DW_AT_discr_value, cu);
+  if (discr == nullptr)
+    {
+      discr = dwarf2_attr (die, DW_AT_discr_list, cu);
+      if (discr == nullptr || DW_BLOCK (discr)->size == 0)
+ variant.default_branch = true;
+      else
+ variant.discr_list_data = DW_BLOCK (discr);
+    }
+  else
+    variant.discriminant_value = DW_UNSND (discr);
+
+  for (die_info *variant_child = die->child;
+       variant_child != NULL;
+       variant_child = variant_child->sibling)
+    handle_struct_member_die (variant_child, type, fi, template_args, cu);
+
+  variant.last_field = fi->fields.size ();
+}
+
 /* A helper for process_structure_scope that handles a single member
    DIE.  */
 
@@ -15130,8 +15437,7 @@ handle_struct_member_die (struct die_info *child_die, struct type *type,
   struct dwarf2_cu *cu)
 {
   if (child_die->tag == DW_TAG_member
-      || child_die->tag == DW_TAG_variable
-      || child_die->tag == DW_TAG_variant_part)
+      || 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
@@ -15168,41 +15474,10 @@ handle_struct_member_die (struct die_info *child_die, struct type *type,
       if (arg != NULL)
  template_args->push_back (arg);
     }
+  else if (child_die->tag == DW_TAG_variant_part)
+    handle_variant_part (child_die, type, fi, template_args, cu);
   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 (die_info *variant_child = child_die->child;
-   variant_child != NULL;
-   variant_child = variant_child->sibling)
- {
-  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 (_("DW_AT_discr_list is not supported yet"
-       " - DIE at %s [in module %s]"),
-     sect_offset_str (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.empty ());
-      if (discr == NULL)
- fi->fields.back ().variant.default_branch = true;
-      else
- fi->fields.back ().variant.discriminant_value = DW_UNSND (discr);
-    }
+    handle_variant (child_die, type, fi, template_args, cu);
 }
 
 /* Finish creating a structure or union type, including filling in
@@ -15219,39 +15494,7 @@ 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 {};
   bool has_template_parameters = false;
-
-  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 (discr->form_is_ref ())
- {
-  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 (_("DW_AT_discr does not have DIE reference form"
-       " - DIE at %s [in module %s]"),
-     sect_offset_str (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;
@@ -15262,10 +15505,6 @@ process_structure_scope (struct die_info *die, struct dwarf2_cu *cu)
       while (child_die && child_die->tag)
  {
   handle_struct_member_die (child_die, type, &fi, &template_args, cu);
-
-  if (is_variant_part && discr_offset == child_die->sect_off)
-    fi.fields.back ().variant.is_discriminant = true;
-
   child_die = child_die->sibling;
  }
 
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index f686e5407ba..134515845f2 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -319,14 +319,6 @@ 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.  */
 
@@ -482,39 +474,6 @@ struct variant_part : allocate_on_obstack
 };
 
 
-/* * 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.  */
@@ -591,9 +550,6 @@ 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,
-
   /* A property holding variant parts.  */
   DYN_PROP_VARIANT_PARTS,
 };
@@ -831,13 +787,6 @@ 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/rust-lang.c b/gdb/rust-lang.c
index 139e4c2f2ce..20661e48d96 100644
--- a/gdb/rust-lang.c
+++ b/gdb/rust-lang.c
@@ -68,38 +68,37 @@ rust_crate_for_block (const struct block *block)
    enum.  */
 
 static bool
-rust_enum_p (const struct type *type)
+rust_enum_p (struct type *type)
 {
-  return (TYPE_CODE (type) == TYPE_CODE_STRUCT
-  && TYPE_NFIELDS (type) == 1
-  && TYPE_FLAG_DISCRIMINATED_UNION (TYPE_FIELD_TYPE (type, 0)));
+  /* is_dynamic_type will return true if any field has a dynamic
+     attribute -- but we only want to check the top level.  */
+  return TYPE_HAS_VARIANT_PARTS (type);
 }
 
-/* Return true if TYPE, which must be an enum type, has no
-   variants.  */
+/* Return true if TYPE, which must be an already-resolved 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;
+  return TYPE_NFIELDS (type) == 0;
 }
 
-/* Given an enum type and contents, find which variant is active.  */
+/* Given an already-resolved enum type and contents, find which
+   variant is active.  */
 
-static struct field *
-rust_enum_variant (struct type *type, const gdb_byte *contents)
+static int
+rust_enum_variant (struct type *type)
 {
-  /* In Rust the enum always fills the containing structure.  */
-  gdb_assert (TYPE_FIELD_BITPOS (type, 0) == 0);
-
-  struct type *union_type = TYPE_FIELD_TYPE (type, 0);
+  /* The active variant is simply the first non-artificial field.  */
+  for (int i = 0; i < TYPE_NFIELDS (type); ++i)
+    if (!TYPE_FIELD_ARTIFICIAL (type, i))
+      return i;
 
-  int fieldno = value_union_variant (union_type, contents);
-  return &TYPE_FIELD (union_type, fieldno);
+  /* Perhaps we could get here by trying to print an Ada variant
+     record in Rust mode.  Unlikely, but an error is safer than an
+     assert.  */
+  error (_("Could not find active enum variant"));
 }
 
 /* See rust-lang.h.  */
@@ -471,6 +470,11 @@ rust_print_enum (struct value *val, struct ui_file *stream, int recurse,
 
   opts.deref_ref = 0;
 
+  gdb_assert (rust_enum_p (type));
+  gdb::array_view<const gdb_byte> view (value_contents_for_printing (val),
+ TYPE_LENGTH (value_type (val)));
+  type = resolve_dynamic_type (type, view, value_address (val));
+
   if (rust_empty_enum_p (type))
     {
       /* Print the enum type name here to be more clear.  */
@@ -480,9 +484,9 @@ rust_print_enum (struct value *val, struct ui_file *stream, int recurse,
       return;
     }
 
-  const gdb_byte *valaddr = value_contents_for_printing (val);
-  struct field *variant_field = rust_enum_variant (type, valaddr);
-  struct type *variant_type = FIELD_TYPE (*variant_field);
+  int variant_fieldno = rust_enum_variant (type);
+  val = value_field (val, variant_fieldno);
+  struct type *variant_type = TYPE_FIELD_TYPE (type, variant_fieldno);
 
   int nfields = TYPE_NFIELDS (variant_type);
 
@@ -505,10 +509,6 @@ rust_print_enum (struct value *val, struct ui_file *stream, int recurse,
       fprintf_filtered (stream, "{");
     }
 
-  struct value *union_value = value_field (val, 0);
-  int fieldno = (variant_field - &TYPE_FIELD (value_type (union_value), 0));
-  val = value_field (union_value, fieldno);
-
   bool first_field = true;
   for (int j = 0; j < TYPE_NFIELDS (variant_type); j++)
     {
@@ -698,8 +698,6 @@ rust_print_struct_def (struct type *type, const char *varstring,
   bool is_tuple = rust_tuple_type_p (type);
   bool is_enum = rust_enum_p (type);
 
-  int enum_discriminant_index = -1;
-
   if (for_rust_enum)
     {
       /* Already printing an outer enum, so nothing to print here.  */
@@ -710,25 +708,10 @@ 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
-    = get_dyn_prop (DYN_PROP_DISCRIMINATED, type);
-  struct discriminant_info *info
-    = (struct discriminant_info *) discriminant_prop->data.baton;
-  enum_discriminant_index = info->discriminant_index;
+  struct dynamic_prop *prop = get_dyn_prop (DYN_PROP_VARIANT_PARTS,
+    type);
+  if (prop != nullptr && prop->kind == PROP_TYPE)
+    type = prop->data.original_type;
  }
       else if (TYPE_CODE (type) == TYPE_CODE_STRUCT)
  fputs_filtered ("struct ", stream);
@@ -755,7 +738,7 @@ rust_print_struct_def (struct type *type, const char *varstring,
     {
       if (field_is_static (&TYPE_FIELD (type, i)))
  continue;
-      if (is_enum && i == enum_discriminant_index)
+      if (is_enum && TYPE_FIELD_ARTIFICIAL (type, i))
  continue;
       fields.push_back (i);
     }
@@ -772,7 +755,7 @@ rust_print_struct_def (struct type *type, const char *varstring,
       QUIT;
 
       gdb_assert (!field_is_static (&TYPE_FIELD (type, i)));
-      gdb_assert (! (is_enum && i == enum_discriminant_index));
+      gdb_assert (! (is_enum && TYPE_FIELD_ARTIFICIAL (type, i)));
 
       if (flags->print_offsets)
  podata->update (type, i, stream);
@@ -1679,20 +1662,16 @@ rust_evaluate_subexp (struct type *expect_type, struct expression *exp,
 
     if (rust_enum_p (type))
       {
+ gdb::array_view<const gdb_byte> view (value_contents (lhs),
+      TYPE_LENGTH (type));
+ type = resolve_dynamic_type (type, view, value_address (lhs));
+
  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);
-
- 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));
+ int fieldno = rust_enum_variant (type);
+ lhs = value_primitive_field (lhs, 0, fieldno, type);
  outer_type = type;
  type = value_type (lhs);
       }
@@ -1751,20 +1730,16 @@ tuple structs, and tuple-like enum variants"));
         type = value_type (lhs);
         if (TYPE_CODE (type) == TYPE_CODE_STRUCT && rust_enum_p (type))
   {
+    gdb::array_view<const gdb_byte> view (value_contents (lhs),
+  TYPE_LENGTH (type));
+    type = resolve_dynamic_type (type, view, value_address (lhs));
+
     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);
-
-    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));
+    int fieldno = rust_enum_variant (type);
+    lhs = value_primitive_field (lhs, 0, fieldno, type);
 
     struct type *outer_type = type;
     type = value_type (lhs);
diff --git a/gdb/valops.c b/gdb/valops.c
index 03c6482ee1e..33fe57562e4 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -2233,50 +2233,6 @@ 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 correctly 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 a reference to the fn_field list METHODS of
    overloaded instances defined in the source language.  If available
diff --git a/gdb/value.h b/gdb/value.h
index dfe3e8e3ed3..ae859ca7224 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -1226,14 +1226,6 @@ extern struct type *result_type_of_xmethod (struct value *method,
 extern struct value *call_xmethod (struct value *method,
    gdb::array_view<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);
-
 /* Destroy the values currently allocated.  This is called when GDB is
    exiting (e.g., on quit_force).  */
 extern void finalize_values ();
--
2.21.1

Reply | Threaded
Open this post in threaded view
|

[PATCH 06/11] Add support for dynamic type lengths

Tom Tromey-4
In reply to this post by Tom Tromey-4
In Ada, a type with variant parts can have a variable length.  This
patch adds support for this to gdb, by integrating the length
computation into the dynamic type resolution code.

gdb/ChangeLog
2020-04-08  Tom Tromey  <[hidden email]>

        * dwarf2/read.c (read_structure_type): Handle dynamic length.
        * gdbtypes.c (is_dynamic_type_internal): Check
        TYPE_HAS_DYNAMIC_LENGTH.
        (resolve_dynamic_type_internal): Use TYPE_DYNAMIC_LENGTH.
        * gdbtypes.h (TYPE_HAS_DYNAMIC_LENGTH, TYPE_DYNAMIC_LENGTH):
        New macros.
        (enum dynamic_prop_node_kind) <DYN_PROP_BYTE_SIZE>: New
        constant.

gdb/testsuite/ChangeLog
2020-04-08  Tom Tromey  <[hidden email]>

        * gdb.ada/variant.exp: New file
        * gdb.ada/variant/pkg.adb: New file
        * gdb.ada/variant/pck.adb: New file
---
 gdb/ChangeLog                         | 11 ++++++++
 gdb/dwarf2/read.c                     | 12 +++-----
 gdb/gdbtypes.c                        | 20 +++++++++++++-
 gdb/gdbtypes.h                        |  9 ++++++
 gdb/testsuite/ChangeLog               |  6 ++++
 gdb/testsuite/gdb.ada/variant.exp     | 40 +++++++++++++++++++++++++++
 gdb/testsuite/gdb.ada/variant/pck.ads | 37 +++++++++++++++++++++++++
 gdb/testsuite/gdb.ada/variant/pkg.adb | 30 ++++++++++++++++++++
 8 files changed, 156 insertions(+), 9 deletions(-)
 create mode 100644 gdb/testsuite/gdb.ada/variant.exp
 create mode 100644 gdb/testsuite/gdb.ada/variant/pck.ads
 create mode 100644 gdb/testsuite/gdb.ada/variant/pkg.adb

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index c0bced60384..31f732d29a3 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -15257,14 +15257,10 @@ read_structure_type (struct die_info *die, struct dwarf2_cu *cu)
         TYPE_LENGTH (type) = DW_UNSND (attr);
       else
  {
-  /* For the moment, dynamic type sizes are not supported
-     by GDB's struct type.  The actual size is determined
-     on-demand when resolving the type of a given object,
-     so set the type's length to zero for now.  Otherwise,
-     we record an expression as the length, and that expression
-     could lead to a very large value, which could eventually
-     lead to us trying to allocate that much memory when creating
-     a value of that type.  */
+  struct dynamic_prop prop;
+  if (attr_to_dynamic_prop (attr, die, cu, &prop,
+    cu->per_cu->addr_type ()))
+    add_dyn_prop (DYN_PROP_BYTE_SIZE, prop, type);
           TYPE_LENGTH (type) = 0;
  }
     }
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 9d0ebaa159e..4491c1832f1 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -1975,6 +1975,9 @@ is_dynamic_type_internal (struct type *type, int top_level)
   if (prop != nullptr && prop->kind != PROP_TYPE)
     return 1;
 
+  if (TYPE_HAS_DYNAMIC_LENGTH (type))
+    return 1;
+
   switch (TYPE_CODE (type))
     {
     case TYPE_CODE_RANGE:
@@ -2491,13 +2494,19 @@ resolve_dynamic_type_internal (struct type *type,
        int top_level)
 {
   struct type *real_type = check_typedef (type);
-  struct type *resolved_type = type;
+  struct type *resolved_type = nullptr;
   struct dynamic_prop *prop;
   CORE_ADDR value;
 
   if (!is_dynamic_type_internal (real_type, top_level))
     return type;
 
+  gdb::optional<CORE_ADDR> type_length;
+  prop = TYPE_DYNAMIC_LENGTH (type);
+  if (prop != NULL
+      && dwarf2_evaluate_property (prop, NULL, addr_stack, &value))
+    type_length = value;
+
   if (TYPE_CODE (type) == TYPE_CODE_TYPEDEF)
     {
       resolved_type = copy_type (type);
@@ -2553,6 +2562,15 @@ resolve_dynamic_type_internal (struct type *type,
  }
     }
 
+  if (resolved_type == nullptr)
+    return type;
+
+  if (type_length.has_value ())
+    {
+      TYPE_LENGTH (resolved_type) = *type_length;
+      remove_dyn_prop (DYN_PROP_BYTE_SIZE, resolved_type);
+    }
+
   /* Resolve data_location attribute.  */
   prop = TYPE_DATA_LOCATION (resolved_type);
   if (prop != NULL
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index 134515845f2..87b1bca3a22 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -355,6 +355,10 @@ DEF_ENUM_FLAGS_TYPE (enum type_instance_flag_value, type_instance_flags);
 #define TYPE_HAS_VARIANT_PARTS(t) \
   (get_dyn_prop (DYN_PROP_VARIANT_PARTS, t) != nullptr)
 
+/* * True if this type has a dynamic length.  */
+#define TYPE_HAS_DYNAMIC_LENGTH(t) \
+  (get_dyn_prop (DYN_PROP_BYTE_SIZE, t) != nullptr)
+
 /* * Instruction-space delimited type.  This is for Harvard architectures
    which have separate instruction and data address spaces (and perhaps
    others).
@@ -552,6 +556,9 @@ enum dynamic_prop_node_kind
 
   /* A property holding variant parts.  */
   DYN_PROP_VARIANT_PARTS,
+
+  /* A property holding the size of the type.  */
+  DYN_PROP_BYTE_SIZE,
 };
 
 /* * List for dynamic type attributes.  */
@@ -1445,6 +1452,8 @@ extern bool set_type_align (struct type *, ULONGEST);
   TYPE_DATA_LOCATION (thistype)->data.const_val
 #define TYPE_DATA_LOCATION_KIND(thistype) \
   TYPE_DATA_LOCATION (thistype)->kind
+#define TYPE_DYNAMIC_LENGTH(thistype) \
+  get_dyn_prop (DYN_PROP_BYTE_SIZE, thistype)
 
 /* Property accessors for the type allocated/associated.  */
 #define TYPE_ALLOCATED_PROP(thistype) \
diff --git a/gdb/testsuite/gdb.ada/variant.exp b/gdb/testsuite/gdb.ada/variant.exp
new file mode 100644
index 00000000000..b68bf60b192
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/variant.exp
@@ -0,0 +1,40 @@
+# Copyright 2020 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/>.
+
+load_lib "ada.exp"
+
+standard_ada_testfile pkg
+
+foreach_with_prefix scenario {none all minimal} {
+    set flags {debug}
+    if {$scenario != "none"} {
+ lappend flags additional_flags=-fgnat-encodings=$scenario
+    }
+
+    if {[gdb_compile_ada "${srcfile}" "${binfile}" executable $flags] != ""} {
+ return -1
+    }
+
+    clean_restart ${testfile}
+
+    set bp_location [gdb_get_line_number "STOP" ${testdir}/pkg.adb]
+    runto "pkg.adb:$bp_location"
+
+    gdb_test "print r" "= \\(c => 100 'd'\\)"
+    gdb_test "print q" " = \\(c => 0 '\\\[\"00\"\\\]', x_first => 27\\)"
+
+    gdb_test "print st1" " = \\(i => -4, one => 1, x => 2\\)"
+    gdb_test "print st2" " = \\(i => 99, one => 1, y => 77\\)"
+}
diff --git a/gdb/testsuite/gdb.ada/variant/pck.ads b/gdb/testsuite/gdb.ada/variant/pck.ads
new file mode 100644
index 00000000000..41b6efd4da8
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/variant/pck.ads
@@ -0,0 +1,37 @@
+--  Copyright 2020 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/>.
+
+package Pck is
+
+   type Rec_Type (C : Character := 'd') is record
+      case C is
+         when Character'First     => X_First : Integer;
+         when Character'Val (127) => X_127   : Integer;
+         when Character'Val (128) => X_128   : Integer;
+         when Character'Last      => X_Last  : Integer;
+         when others              => null;
+      end case;
+   end record;
+
+   type Second_Type (I : Integer) is record
+      One: Integer;
+      case I is
+         when -5 .. 5 =>
+   X : Integer;
+         when others =>
+   Y : Integer;
+      end case;
+   end record;
+end Pck;
diff --git a/gdb/testsuite/gdb.ada/variant/pkg.adb b/gdb/testsuite/gdb.ada/variant/pkg.adb
new file mode 100644
index 00000000000..0cc38f5b253
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/variant/pkg.adb
@@ -0,0 +1,30 @@
+--  Copyright 2020 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/>.
+
+with Pck; use Pck;
+
+procedure Pkg is
+
+   R, Q : Rec_Type;
+
+   ST1 : constant Second_Type := (I => -4, One => 1, X => 2);
+   ST2 : constant Second_Type := (I => 99, One => 1, Y => 77);
+
+begin
+   R := (C => 'd');
+   Q := (C => Character'First, X_First => 27);
+
+   null; -- STOP
+end Pkg;
--
2.21.1

Reply | Threaded
Open this post in threaded view
|

[PATCH 07/11] Add support for variable field offsets

Tom Tromey-4
In reply to this post by Tom Tromey-4
In Ada, a field can have a variable offset.  This patch adds support
for this case to gdb, using the existing dynamic type resolution code.

Doing just this, though, would break C++ virtual base handling.

It turns out that virtual base handling only worked by the ugliest of
hacks.  In particular, the DWARF reader would call decode_locdesc for
a virtual base location.  Here's an example of such an expression from
gdb's m-static test case:

    <241>   DW_AT_data_member_location: 6 byte block: 12 6 48 1c 6 22 (DW_OP_dup; DW_OP_deref; DW_OP_lit24; DW_OP_minus; DW_OP_deref; DW_OP_plus)

When examining this, decode_locdesc would treat DW_OP_deref as a no-op
and compute some answer (here, -24).  This would be stored as the
offset.

Later, in gnu-v3-abi.c, the real offset would be computed by digging
around in the vtable.

This patch cleans up this area.  In particular, it now evaluates the
location expression on demand.

Note there is a new FIXME in gnu-v3-abi.c.  I think some of the
callers are incorrect here, and have only worked because this member
is unused.  I will file a bug for this.  I didn't fix this problem in
this series because I felt it was already too complex.

gdb/ChangeLog
2020-04-08  Tom Tromey  <[hidden email]>

        * dwarf2/read.c (handle_data_member_location): New overload.
        (dwarf2_add_field): Use it.
        (decode_locdesc): Add "computed" parameter.  Update comment.
        * gdbtypes.c (is_dynamic_type_internal): Also look for
        FIELD_LOC_KIND_DWARF_BLOCK.
        (resolve_dynamic_struct): Handle FIELD_LOC_KIND_DWARF_BLOCK.
        * gdbtypes.c (is_dynamic_type_internal): Add special case for C++
        virtual base classes.
        * gnu-v3-abi.c (gnuv3_baseclass_offset): Handle
        FIELD_LOC_KIND_DWARF_BLOCK.

gdb/testsuite/ChangeLog
2020-04-08  Tom Tromey  <[hidden email]>

        * gdb.ada/variant.exp: Add dynamic field offset tests.
        * gdb.ada/variant/pck.ads (Nested_And_Variable): New type.
        * gdb.ada/variant/pkg.adb: Add new variables.
---
 gdb/ChangeLog                         |  13 +++
 gdb/dwarf2/read.c                     | 145 ++++++++++++++++++--------
 gdb/gdbtypes.c                        |  39 ++++++-
 gdb/gnu-v3-abi.c                      |  26 +++++
 gdb/testsuite/ChangeLog               |   6 ++
 gdb/testsuite/gdb.ada/variant.exp     |   6 ++
 gdb/testsuite/gdb.ada/variant/pck.ads |  17 +++
 gdb/testsuite/gdb.ada/variant/pkg.adb |  11 ++
 8 files changed, 218 insertions(+), 45 deletions(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 31f732d29a3..9d78527b56e 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -1435,7 +1435,8 @@ static const char *namespace_name (struct die_info *die,
 
 static void process_enumeration_scope (struct die_info *, struct dwarf2_cu *);
 
-static CORE_ADDR decode_locdesc (struct dwarf_block *, struct dwarf2_cu *);
+static CORE_ADDR decode_locdesc (struct dwarf_block *, struct dwarf2_cu *,
+ bool * = nullptr);
 
 static enum dwarf_array_dim_ordering read_array_order (struct die_info *,
        struct dwarf2_cu *);
@@ -14169,6 +14170,53 @@ handle_data_member_location (struct die_info *die, struct dwarf2_cu *cu,
   return 0;
 }
 
+/* Look for DW_AT_data_member_location and store the results in FIELD.  */
+
+static void
+handle_data_member_location (struct die_info *die, struct dwarf2_cu *cu,
+     struct field *field)
+{
+  struct attribute *attr;
+
+  attr = dwarf2_attr (die, DW_AT_data_member_location, cu);
+  if (attr != NULL)
+    {
+      if (attr->form_is_constant ())
+ {
+  LONGEST offset = attr->constant_value (0);
+  SET_FIELD_BITPOS (*field, offset * bits_per_byte);
+ }
+      else if (attr->form_is_section_offset ())
+ dwarf2_complex_location_expr_complaint ();
+      else if (attr->form_is_block ())
+ {
+  bool handled;
+  CORE_ADDR offset = decode_locdesc (DW_BLOCK (attr), cu, &handled);
+  if (handled)
+    SET_FIELD_BITPOS (*field, offset * bits_per_byte);
+  else
+    {
+      struct objfile *objfile
+ = cu->per_cu->dwarf2_per_objfile->objfile;
+      struct dwarf2_locexpr_baton *dlbaton
+ = XOBNEW (&objfile->objfile_obstack,
+  struct dwarf2_locexpr_baton);
+      dlbaton->data = DW_BLOCK (attr)->data;
+      dlbaton->size = DW_BLOCK (attr)->size;
+      /* When using this baton, we want to compute the address
+ of the field, not the value.  This is why
+ is_reference is set to false here.  */
+      dlbaton->is_reference = false;
+      dlbaton->per_cu = cu->per_cu;
+
+      SET_FIELD_DWARF_BLOCK (*field, dlbaton);
+    }
+ }
+      else
+ dwarf2_complex_location_expr_complaint ();
+    }
+}
+
 /* Add an aggregate field to the field list.  */
 
 static void
@@ -14213,8 +14261,6 @@ dwarf2_add_field (struct field_info *fip, struct die_info *die,
 
   if (die->tag == DW_TAG_member && ! die_is_declaration (die, cu))
     {
-      LONGEST offset;
-
       /* Data member other than a C++ static data member.  */
 
       /* Get type of field.  */
@@ -14234,8 +14280,7 @@ dwarf2_add_field (struct field_info *fip, struct die_info *die,
  }
 
       /* Get bit offset of field.  */
-      if (handle_data_member_location (die, cu, &offset))
- SET_FIELD_BITPOS (*fp, offset * bits_per_byte);
+      handle_data_member_location (die, cu, fp);
       attr = dwarf2_attr (die, DW_AT_bit_offset, cu);
       if (attr != nullptr)
  {
@@ -14344,11 +14389,8 @@ dwarf2_add_field (struct field_info *fip, struct die_info *die,
     }
   else if (die->tag == DW_TAG_inheritance)
     {
-      LONGEST offset;
-
       /* C++ base class field.  */
-      if (handle_data_member_location (die, cu, &offset))
- SET_FIELD_BITPOS (*fp, offset * bits_per_byte);
+      handle_data_member_location (die, cu, fp);
       FIELD_BITSIZE (*fp) = 0;
       FIELD_TYPE (*fp) = die_type (die, cu);
       FIELD_NAME (*fp) = TYPE_NAME (fp->type);
@@ -22612,27 +22654,13 @@ read_signatured_type (struct signatured_type *sig_type)
 
 /* Decode simple location descriptions.
    Given a pointer to a dwarf block that defines a location, compute
-   the location and return the value.
-
-   NOTE drow/2003-11-18: This function is called in two situations
-   now: for the address of static or global variables (partial symbols
-   only) and for offsets into structures which are expected to be
-   (more or less) constant.  The partial symbol case should go away,
-   and only the constant case should remain.  That will let this
-   function complain more accurately.  A few special modes are allowed
-   without complaint for global variables (for instance, global
-   register values and thread-local values).
-
-   A location description containing no operations indicates that the
-   object is optimized out.  The return value is 0 for that case.
-   FIXME drow/2003-11-16: No callers check for this case any more; soon all
-   callers will only want a very basic result and this can become a
-   complaint.
-
-   Note that stack[0] is unused except as a default error return.  */
+   the location and return the value.  If COMPUTED is non-null, it is
+   set to true to indicate that decoding was successful, and false
+   otherwise.  If COMPUTED is null, then this function may emit a
+   complaint.  */
 
 static CORE_ADDR
-decode_locdesc (struct dwarf_block *blk, struct dwarf2_cu *cu)
+decode_locdesc (struct dwarf_block *blk, struct dwarf2_cu *cu, bool *computed)
 {
   struct objfile *objfile = cu->per_cu->dwarf2_per_objfile->objfile;
   size_t i;
@@ -22643,6 +22671,9 @@ decode_locdesc (struct dwarf_block *blk, struct dwarf2_cu *cu)
   unsigned int bytes_read, unsnd;
   gdb_byte op;
 
+  if (computed != nullptr)
+    *computed = false;
+
   i = 0;
   stacki = 0;
   stack[stacki] = 0;
@@ -22722,7 +22753,12 @@ decode_locdesc (struct dwarf_block *blk, struct dwarf2_cu *cu)
  case DW_OP_reg31:
   stack[++stacki] = op - DW_OP_reg0;
   if (i < size)
-    dwarf2_complex_location_expr_complaint ();
+    {
+      if (computed == nullptr)
+ dwarf2_complex_location_expr_complaint ();
+      else
+ return 0;
+    }
   break;
 
  case DW_OP_regx:
@@ -22730,7 +22766,12 @@ decode_locdesc (struct dwarf_block *blk, struct dwarf2_cu *cu)
   i += bytes_read;
   stack[++stacki] = unsnd;
   if (i < size)
-    dwarf2_complex_location_expr_complaint ();
+    {
+      if (computed == nullptr)
+ dwarf2_complex_location_expr_complaint ();
+      else
+ return 0;
+    }
   break;
 
  case DW_OP_addr:
@@ -22812,7 +22853,12 @@ decode_locdesc (struct dwarf_block *blk, struct dwarf2_cu *cu)
      global symbols, although the variable's address will be bogus
      in the psymtab.  */
   if (i < size)
-    dwarf2_complex_location_expr_complaint ();
+    {
+      if (computed == nullptr)
+ dwarf2_complex_location_expr_complaint ();
+      else
+ return 0;
+    }
   break;
 
         case DW_OP_GNU_push_tls_address:
@@ -22826,11 +22872,18 @@ decode_locdesc (struct dwarf_block *blk, struct dwarf2_cu *cu)
      non-zero to not look as a variable garbage collected by linker
      which have DW_OP_addr 0.  */
   if (i < size)
-    dwarf2_complex_location_expr_complaint ();
+    {
+      if (computed == nullptr)
+ dwarf2_complex_location_expr_complaint ();
+      else
+ return 0;
+    }
   stack[stacki]++;
           break;
 
  case DW_OP_GNU_uninit:
+  if (computed != nullptr)
+    return 0;
   break;
 
  case DW_OP_addrx:
@@ -22842,16 +22895,17 @@ decode_locdesc (struct dwarf_block *blk, struct dwarf2_cu *cu)
   break;
 
  default:
-  {
-    const char *name = get_DW_OP_name (op);
+  if (computed == nullptr)
+    {
+      const char *name = get_DW_OP_name (op);
 
-    if (name)
-      complaint (_("unsupported stack op: '%s'"),
- name);
-    else
-      complaint (_("unsupported stack op: '%02x'"),
- op);
-  }
+      if (name)
+ complaint (_("unsupported stack op: '%s'"),
+   name);
+      else
+ complaint (_("unsupported stack op: '%02x'"),
+   op);
+    }
 
   return (stack[stacki]);
  }
@@ -22860,16 +22914,21 @@ decode_locdesc (struct dwarf_block *blk, struct dwarf2_cu *cu)
          outside of the allocated space.  Also enforce minimum>0.  */
       if (stacki >= ARRAY_SIZE (stack) - 1)
  {
-  complaint (_("location description stack overflow"));
+  if (computed == nullptr)
+    complaint (_("location description stack overflow"));
   return 0;
  }
 
       if (stacki <= 0)
  {
-  complaint (_("location description stack underflow"));
+  if (computed == nullptr)
+    complaint (_("location description stack underflow"));
   return 0;
  }
     }
+
+  if (computed != nullptr)
+    *computed = true;
   return (stack[stacki]);
 }
 
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 4491c1832f1..01d838381ba 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -2015,10 +2015,27 @@ is_dynamic_type_internal (struct type *type, int top_level)
       {
  int i;
 
+ bool is_cplus = HAVE_CPLUS_STRUCT (type);
+
  for (i = 0; i < TYPE_NFIELDS (type); ++i)
-  if (!field_is_static (&TYPE_FIELD (type, i))
-      && is_dynamic_type_internal (TYPE_FIELD_TYPE (type, i), 0))
+  {
+    /* Static fields can be ignored here.  */
+    if (field_is_static (&TYPE_FIELD (type, i)))
+      continue;
+    /* If the field has dynamic type, then so does TYPE.  */
+    if (is_dynamic_type_internal (TYPE_FIELD_TYPE (type, i), 0))
+      return 1;
+    /* If the field is at a fixed offset, then it is not
+       dynamic.  */
+    if (TYPE_FIELD_LOC_KIND (type, i) != FIELD_LOC_KIND_DWARF_BLOCK)
+      continue;
+    /* Do not consider C++ virtual base types to be dynamic
+       due to the field's offset being dynamic; these are
+       handled via other means.  */
+    if (is_cplus && BASETYPE_VIA_VIRTUAL (type, i))
+      continue;
     return 1;
+  }
       }
       break;
     }
@@ -2430,6 +2447,24 @@ resolve_dynamic_struct (struct type *type,
       if (field_is_static (&TYPE_FIELD (resolved_type, i)))
  continue;
 
+      if (TYPE_FIELD_LOC_KIND (resolved_type, i) == FIELD_LOC_KIND_DWARF_BLOCK)
+ {
+  struct dwarf2_property_baton baton;
+  baton.property_type
+    = lookup_pointer_type (TYPE_FIELD_TYPE (resolved_type, i));
+  baton.locexpr = *TYPE_FIELD_DWARF_BLOCK (resolved_type, i);
+
+  struct dynamic_prop prop;
+  prop.kind = PROP_LOCEXPR;
+  prop.data.baton = &baton;
+
+  CORE_ADDR addr;
+  if (dwarf2_evaluate_property (&prop, nullptr, addr_stack, &addr,
+ true))
+    SET_FIELD_BITPOS (TYPE_FIELD (resolved_type, i),
+      TARGET_CHAR_BIT * (addr - addr_stack->addr));
+ }
+
       /* As we know this field is not a static field, the field's
  field_loc_kind should be FIELD_LOC_KIND_BITPOS.  Verify
  this is the case, but only trigger a simple error rather
diff --git a/gdb/gnu-v3-abi.c b/gdb/gnu-v3-abi.c
index 89574ec9ed5..70558437f9c 100644
--- a/gdb/gnu-v3-abi.c
+++ b/gdb/gnu-v3-abi.c
@@ -30,6 +30,7 @@
 #include "typeprint.h"
 #include <algorithm>
 #include "cli/cli-style.h"
+#include "dwarf2/loc.h"
 
 static struct cp_abi_ops gnu_v3_abi_ops;
 
@@ -461,6 +462,31 @@ gnuv3_baseclass_offset (struct type *type, int index,
   if (!BASETYPE_VIA_VIRTUAL (type, index))
     return TYPE_BASECLASS_BITPOS (type, index) / 8;
 
+  /* If we have a DWARF expression for the offset, evaluate it.  */
+  if (TYPE_FIELD_LOC_KIND (type, index) == FIELD_LOC_KIND_DWARF_BLOCK)
+    {
+      struct dwarf2_property_baton baton;
+      baton.property_type
+ = lookup_pointer_type (TYPE_FIELD_TYPE (type, index));
+      baton.locexpr = *TYPE_FIELD_DWARF_BLOCK (type, index);
+
+      struct dynamic_prop prop;
+      prop.kind = PROP_LOCEXPR;
+      prop.data.baton = &baton;
+
+      struct property_addr_info addr_stack;
+      addr_stack.type = type;
+      /* Note that we don't set "valaddr" here.  Doing so causes
+ regressions.  FIXME.  */
+      addr_stack.addr = address + embedded_offset;
+      addr_stack.next = nullptr;
+
+      CORE_ADDR result;
+      if (dwarf2_evaluate_property (&prop, nullptr, &addr_stack, &result,
+    true))
+ return (int) (result - addr_stack.addr);
+    }
+
   /* To access a virtual base, we need to use the vbase offset stored in
      our vtable.  Recent GCC versions provide this information.  If it isn't
      available, we could get what we needed from RTTI, or from drawing the
diff --git a/gdb/testsuite/gdb.ada/variant.exp b/gdb/testsuite/gdb.ada/variant.exp
index b68bf60b192..490956a2666 100644
--- a/gdb/testsuite/gdb.ada/variant.exp
+++ b/gdb/testsuite/gdb.ada/variant.exp
@@ -37,4 +37,10 @@ foreach_with_prefix scenario {none all minimal} {
 
     gdb_test "print st1" " = \\(i => -4, one => 1, x => 2\\)"
     gdb_test "print st2" " = \\(i => 99, one => 1, y => 77\\)"
+
+    gdb_test "print nav1" " = \\(one => 0, two => 93, str => \"\"\\)"
+    gdb_test "print nav2" \
+ " = \\(one => 3, two => 0, str => \"zzz\", onevalue => 33, str2 => \"\"\\)"
+    gdb_test "print nav3" \
+ " = \\(one => 3, two => 7, str => \"zzz\", onevalue => 33, str2 => \"qqqqqqq\", twovalue => 88\\)"
 }
diff --git a/gdb/testsuite/gdb.ada/variant/pck.ads b/gdb/testsuite/gdb.ada/variant/pck.ads
index 41b6efd4da8..3895b9c48eb 100644
--- a/gdb/testsuite/gdb.ada/variant/pck.ads
+++ b/gdb/testsuite/gdb.ada/variant/pck.ads
@@ -34,4 +34,21 @@ package Pck is
    Y : Integer;
       end case;
    end record;
+
+   type Nested_And_Variable (One, Two: Integer) is record
+       Str : String (1 .. One);
+       case One is
+          when 0 =>
+     null;
+          when others =>
+     OneValue : Integer;
+             Str2 : String (1 .. Two);
+             case Two is
+        when 0 =>
+   null;
+ when others =>
+   TwoValue : Integer;
+             end case;
+       end case;
+   end record;
 end Pck;
diff --git a/gdb/testsuite/gdb.ada/variant/pkg.adb b/gdb/testsuite/gdb.ada/variant/pkg.adb
index 0cc38f5b253..91cf080ed1d 100644
--- a/gdb/testsuite/gdb.ada/variant/pkg.adb
+++ b/gdb/testsuite/gdb.ada/variant/pkg.adb
@@ -22,6 +22,17 @@ procedure Pkg is
    ST1 : constant Second_Type := (I => -4, One => 1, X => 2);
    ST2 : constant Second_Type := (I => 99, One => 1, Y => 77);
 
+   NAV1 : constant Nested_And_Variable := (One => 0, Two => 93,
+                                           Str => (others => 'z'));
+   NAV2 : constant Nested_And_Variable := (One => 3, OneValue => 33,
+                                           Str => (others => 'z'),
+                                           Str2 => (others => 'q'),
+                                           Two => 0);
+   NAV3 : constant Nested_And_Variable := (One => 3, OneValue => 33,
+                                           Str => (others => 'z'),
+                                           Str2 => (others => 'q'),
+                                           Two => 7, TwoValue => 88);
+
 begin
    R := (C => 'd');
    Q := (C => Character'First, X_First => 27);
--
2.21.1

Reply | Threaded
Open this post in threaded view
|

[PATCH 08/11] Update Ada ptype support for dynamic types

Tom Tromey-4
In reply to this post by Tom Tromey-4
The DWARF reader was updated to handle variant parts and some other
selected features for Ada; but the Ada "ptype" code was not touched.
This patch changes the Ada ptype code to handle the new types
properly.

Test cases for this and for some of the other code in this series are
in a separate patch.

gdb/ChangeLog
2020-04-08  Tom Tromey  <[hidden email]>

        * ada-typeprint.c (print_choices, print_variant_part)
        (print_record_field_types_dynamic): New functions.
        (print_record_field_types): Use print_record_field_types_dynamic.
---
 gdb/ChangeLog       |   6 +++
 gdb/ada-typeprint.c | 129 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 135 insertions(+)

diff --git a/gdb/ada-typeprint.c b/gdb/ada-typeprint.c
index 9f0ac259a16..83972fe125d 100644
--- a/gdb/ada-typeprint.c
+++ b/gdb/ada-typeprint.c
@@ -654,6 +654,120 @@ print_selected_record_field_types (struct type *type, struct type *outer_type,
   return flds;
 }
 
+static void print_record_field_types_dynamic
+  (const gdb::array_view<variant_part> &parts,
+   int from, int to, struct type *type, struct ui_file *stream,
+   int show, int level, const struct type_print_options *flags);
+
+/* Print the choices encoded by VARIANT on STREAM.  LEVEL is the
+   indentation level.  The type of the discriminant for VARIANT is
+   given by DISR_TYPE.  */
+
+static void
+print_choices (struct type *discr_type, const variant &variant,
+       struct ui_file *stream, int level)
+{
+  fprintf_filtered (stream, "\n%*swhen ", level, "");
+  if (variant.is_default ())
+    fprintf_filtered (stream, "others");
+  else
+    {
+      bool first = true;
+      for (const discriminant_range &range : variant.discriminants)
+ {
+  if (!first)
+    fprintf_filtered (stream, " | ");
+  first = false;
+
+  ada_print_scalar (discr_type, range.low, stream);
+  if (range.low != range.high)
+    ada_print_scalar (discr_type, range.high, stream);
+ }
+    }
+
+  fprintf_filtered (stream, " =>");
+}
+
+/* Print a single variant part, PART, on STREAM.  TYPE is the
+   enclosing type.  SHOW, LEVEL, and FLAGS are the usual type-printing
+   settings.  This prints information about PART and the fields it
+   controls.  It returns the index of the next field that should be
+   shown -- that is, one after the last field printed by this
+   call.  */
+
+static int
+print_variant_part (const variant_part &part,
+    struct type *type, struct ui_file *stream,
+    int show, int level,
+    const struct type_print_options *flags)
+{
+  struct type *discr_type = nullptr;
+  const char *name;
+  if (part.discriminant_index == -1)
+    name = "?";
+  else
+    {
+      name = TYPE_FIELD_NAME (type, part.discriminant_index);
+      discr_type = TYPE_FIELD_TYPE (type, part.discriminant_index);
+    }
+
+  fprintf_filtered (stream, "\n%*scase %s is", level + 4, "", name);
+
+  int last_field = -1;
+  for (const variant &variant : part.variants)
+    {
+      print_choices (discr_type, variant, stream, level + 8);
+
+      if (variant.first_field == variant.last_field)
+ fprintf_filtered (stream, " null;");
+      else
+ {
+  print_record_field_types_dynamic (variant.parts,
+    variant.first_field,
+    variant.last_field, type, stream,
+    show, level + 8, flags);
+  last_field = variant.last_field;
+ }
+    }
+
+  fprintf_filtered (stream, "\n%*send case;", level + 4, "");
+
+  return last_field;
+}
+
+/* Print some fields of TYPE to STREAM.  SHOW, LEVEL, and FLAGS are
+   the usual type-printing settings.  PARTS is the array of variant
+   parts that correspond to the range of fields to be printed.  FROM
+   and TO are the range of fields to print.  */
+
+static void
+print_record_field_types_dynamic (const gdb::array_view<variant_part> &parts,
+  int from, int to,
+  struct type *type, struct ui_file *stream,
+  int show, int level,
+  const struct type_print_options *flags)
+{
+  int field = from;
+
+  for (const variant_part &part : parts)
+    {
+      if (part.variants.empty ())
+ continue;
+
+      /* Print any non-varying fields.  */
+      int first_varying = part.variants[0].first_field;
+      print_selected_record_field_types (type, type, field,
+ first_varying - 1, stream,
+ show, level, flags);
+
+      field = print_variant_part (part, type, stream, show, level, flags);
+    }
+
+  /* Print any trailing fields that we were asked to print.  */
+  print_selected_record_field_types (type, type, field, to - 1, stream, show,
+     level, flags);
+}
+
 /* Print a description on STREAM of all fields of record or union type
    TYPE, as for print_selected_record_field_types, above.  */
 
@@ -662,6 +776,21 @@ print_record_field_types (struct type *type, struct type *outer_type,
   struct ui_file *stream, int show, int level,
   const struct type_print_options *flags)
 {
+  struct dynamic_prop *prop = get_dyn_prop (DYN_PROP_VARIANT_PARTS, type);
+  if (prop != nullptr)
+    {
+      if (prop->kind == PROP_TYPE)
+ {
+  type = prop->data.original_type;
+  prop = get_dyn_prop (DYN_PROP_VARIANT_PARTS, type);
+ }
+      gdb_assert (prop->kind == PROP_VARIANT_PARTS);
+      print_record_field_types_dynamic (*prop->data.variant_parts,
+ 0, TYPE_NFIELDS (type),
+ type, stream, show, level, flags);
+      return TYPE_NFIELDS (type);
+    }
+
   return print_selected_record_field_types (type, outer_type,
     0, TYPE_NFIELDS (type) - 1,
     stream, show, level, flags);
--
2.21.1

Reply | Threaded
Open this post in threaded view
|

[PATCH 09/11] Add tests for Ada changes

Tom Tromey-4
In reply to this post by Tom Tromey-4
The previous patches largely came without test cases.  This was done
to make the patches easier to review; as most of the patches were
needed before existing tests could be updated.

This patch adds a new test and updates some existing tests to test all
the settings of -fgnat-encodings.  This ensures that tests are run
both with the old-style "magic symbol name" encoding, and the
new-style DWARF encoding.

Note that in one case, a test is modified to be more lax.  See the
comment in mi_var_array.exp.  I didn't want to fix this in this
series, as it's already complicated enough.  However, I think it could
be fixed; I will file a bug for it.

gdb/testsuite/ChangeLog
2020-04-08  Tom Tromey  <[hidden email]>

        * gdb.ada/mi_var_array.exp: Try all -fgnat-encodings settings.
        Make array type matching more lax.
        * gdb.ada/mi_var_union.exp: Try all -fgnat-encodings settings.
        * gdb.ada/mi_variant.exp: New file.
        * gdb.ada/mi_variant/pck.ads: New file.
        * gdb.ada/mi_variant/pkg.adb: New file.
        * gdb.ada/packed_tagged.exp: Try all -fgnat-encodings settings.
        * gdb.ada/unchecked_union.exp: Try all -fgnat-encodings settings.
---
 gdb/testsuite/ChangeLog                   | 11 ++++
 gdb/testsuite/gdb.ada/mi_var_array.exp    | 69 +++++++++++++----------
 gdb/testsuite/gdb.ada/mi_var_union.exp    | 65 +++++++++++----------
 gdb/testsuite/gdb.ada/mi_variant.exp      | 65 +++++++++++++++++++++
 gdb/testsuite/gdb.ada/mi_variant/pck.ads  | 54 ++++++++++++++++++
 gdb/testsuite/gdb.ada/mi_variant/pkg.adb  | 28 +++++++++
 gdb/testsuite/gdb.ada/packed_tagged.exp   | 41 ++++++++------
 gdb/testsuite/gdb.ada/unchecked_union.exp | 29 ++++++----
 8 files changed, 276 insertions(+), 86 deletions(-)
 create mode 100644 gdb/testsuite/gdb.ada/mi_variant.exp
 create mode 100644 gdb/testsuite/gdb.ada/mi_variant/pck.ads
 create mode 100644 gdb/testsuite/gdb.ada/mi_variant/pkg.adb

diff --git a/gdb/testsuite/gdb.ada/mi_var_array.exp b/gdb/testsuite/gdb.ada/mi_var_array.exp
index e0980c6a2d6..646ebd196f6 100644
--- a/gdb/testsuite/gdb.ada/mi_var_array.exp
+++ b/gdb/testsuite/gdb.ada/mi_var_array.exp
@@ -17,36 +17,47 @@ load_lib "ada.exp"
 
 standard_ada_testfile bar
 
-if {[gdb_compile_ada "${srcfile}" "${binfile}" executable [list debug]] != "" } {
-  return -1
-}
-
 load_lib mi-support.exp
 set MIFLAGS "-i=mi"
 
-gdb_exit
-if [mi_gdb_start] {
-    continue
-}
-
-mi_delete_breakpoints
-mi_gdb_reinitialize_dir $srcdir/$subdir
-mi_gdb_load ${binfile}
-
-if ![mi_run_to_main] then {
-   fail "cannot run to main, testcase aborted"
-   return 0
+foreach_with_prefix scenario {none all minimal} {
+    set flags {debug}
+    if {$scenario != "none"} {
+ lappend flags additional_flags=-fgnat-encodings=$scenario
+    }
+
+    if {[gdb_compile_ada "${srcfile}" "${binfile}" executable $flags] != "" } {
+ return -1
+    }
+
+    gdb_exit
+    if [mi_gdb_start] {
+ continue
+    }
+
+    mi_delete_breakpoints
+    mi_gdb_reinitialize_dir $srcdir/$subdir
+    mi_gdb_load ${binfile}
+
+    if ![mi_run_to_main] then {
+ fail "cannot run to main, testcase aborted"
+ return 0
+    }
+
+    set bp_location [gdb_get_line_number "STOP" ${testdir}/bar.adb]
+    mi_continue_to_line \
+ "bar.adb:$bp_location" \
+ "stop at start of main Ada procedure"
+
+    mi_gdb_test "-var-create vta * vta" \
+ "\\^done,name=\"vta\",numchild=\"2\",.*" \
+ "create bt varobj"
+
+    # In the "minimal" mode, we don't currently have the ability to
+    # print the subrange type properly.  So, we just allow anything
+    # for the array range here.  The correct result would be to fix
+    # this to read "(1 .. n)".
+    mi_gdb_test "-var-list-children vta" \
+ "\\^done,numchild=\"2\",children=\\\[child={name=\"vta.n\",exp=\"n\",numchild=\"0\",type=\"bar\\.int\",thread-id=\"$decimal\"},child={name=\"vta.f\",exp=\"f\",numchild=\"0\",type=\"array .* of character\",thread-id=\"$decimal\"}\\\],.*" \
+ "list vta's children"
 }
-
-set bp_location [gdb_get_line_number "STOP" ${testdir}/bar.adb]
-mi_continue_to_line \
-    "bar.adb:$bp_location" \
-    "stop at start of main Ada procedure"
-
-mi_gdb_test "-var-create vta * vta" \
-    "\\^done,name=\"vta\",numchild=\"2\",.*" \
-    "create bt varobj"
-
-mi_gdb_test "-var-list-children vta" \
-    "\\^done,numchild=\"2\",children=\\\[child={name=\"vta.n\",exp=\"n\",numchild=\"0\",type=\"bar\\.int\",thread-id=\"$decimal\"},child={name=\"vta.f\",exp=\"f\",numchild=\"0\",type=\"array \\(1 .. n\\) of character\",thread-id=\"$decimal\"}\\\],.*" \
-    "list vta's children"
diff --git a/gdb/testsuite/gdb.ada/mi_var_union.exp b/gdb/testsuite/gdb.ada/mi_var_union.exp
index c5f43b4c5d2..7619d86d273 100644
--- a/gdb/testsuite/gdb.ada/mi_var_union.exp
+++ b/gdb/testsuite/gdb.ada/mi_var_union.exp
@@ -17,38 +17,45 @@ load_lib "ada.exp"
 
 standard_ada_testfile bar
 
-if {[gdb_compile_ada "${srcfile}" "${binfile}" executable [list debug]] != "" } {
-  return -1
-}
-
 load_lib mi-support.exp
 set MIFLAGS "-i=mi"
 
-gdb_exit
-if [mi_gdb_start] {
-    continue
-}
-
 set float "\\-?((\[0-9\]+(\\.\[0-9\]+)?(e\[-+\]\[0-9\]+)?)|(nan\\($hex\\)))"
 
-mi_delete_breakpoints
-mi_gdb_reinitialize_dir $srcdir/$subdir
-mi_gdb_load ${binfile}
-
-if ![mi_run_to_main] then {
-   fail "cannot run to main, testcase aborted"
-   return 0
+foreach_with_prefix scenario {none all minimal} {
+    set flags {debug}
+    if {$scenario != "none"} {
+ lappend flags additional_flags=-fgnat-encodings=$scenario
+    }
+
+    if {[gdb_compile_ada "${srcfile}" "${binfile}" executable $flags] != "" } {
+ return -1
+    }
+
+    gdb_exit
+    if [mi_gdb_start] {
+ continue
+    }
+
+    mi_delete_breakpoints
+    mi_gdb_reinitialize_dir $srcdir/$subdir
+    mi_gdb_load ${binfile}
+
+    if ![mi_run_to_main] then {
+ fail "cannot run to main, testcase aborted"
+ return 0
+    }
+
+    set bp_location [gdb_get_line_number "STOP" ${testdir}/bar.adb]
+    mi_continue_to_line \
+ "bar.adb:$bp_location" \
+ "stop at start of main Ada procedure"
+
+    mi_gdb_test "-var-create var1 * Ut" \
+ "\\^done,name=\"var1\",numchild=\"2\",.*" \
+ "Create var1 varobj"
+
+    mi_gdb_test "-var-list-children 1 var1" \
+ "\\^done,numchild=\"2\",children=\\\[child={name=\"var1.b\",exp=\"b\",numchild=\"0\",value=\"3\",type=\"integer\",thread-id=\"$decimal\"},child={name=\"var1.c\",exp=\"c\",numchild=\"0\",value=\"$float\",type=\"float\",thread-id=\"$decimal\"}\\\],has_more=\"0\"" \
+ "list var1's children"
 }
-
-set bp_location [gdb_get_line_number "STOP" ${testdir}/bar.adb]
-mi_continue_to_line \
-    "bar.adb:$bp_location" \
-    "stop at start of main Ada procedure"
-
-mi_gdb_test "-var-create var1 * Ut" \
-    "\\^done,name=\"var1\",numchild=\"2\",.*" \
-    "Create var1 varobj"
-
-mi_gdb_test "-var-list-children 1 var1" \
-    "\\^done,numchild=\"2\",children=\\\[child={name=\"var1.b\",exp=\"b\",numchild=\"0\",value=\"3\",type=\"integer\",thread-id=\"$decimal\"},child={name=\"var1.c\",exp=\"c\",numchild=\"0\",value=\"$float\",type=\"float\",thread-id=\"$decimal\"}\\\],has_more=\"0\"" \
-    "list var1's children"
diff --git a/gdb/testsuite/gdb.ada/mi_variant.exp b/gdb/testsuite/gdb.ada/mi_variant.exp
new file mode 100644
index 00000000000..ac9ece7303c
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/mi_variant.exp
@@ -0,0 +1,65 @@
+# Copyright 2020 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/>.
+
+load_lib "ada.exp"
+load_lib "gdb-python.exp"
+
+standard_ada_testfile pkg
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+foreach_with_prefix scenario {none all minimal} {
+    set flags {debug}
+    if {$scenario != "none"} {
+ lappend flags additional_flags=-fgnat-encodings=$scenario
+    }
+
+    if {[gdb_compile_ada "${srcfile}" "${binfile}" executable $flags] != ""} {
+ return -1
+    }
+
+    gdb_exit
+    if [mi_gdb_start] {
+ continue
+    }
+
+    mi_delete_breakpoints
+    mi_gdb_reinitialize_dir $srcdir/$subdir
+    mi_gdb_load ${binfile}
+
+    if ![mi_run_to_main] then {
+ fail "cannot run to main, testcase aborted"
+ return 0
+    }
+
+    set bp_location [gdb_get_line_number "STOP" ${testdir}/pkg.adb]
+    mi_continue_to_line \
+ "pkg.adb:$bp_location" \
+ "stop at start of main Ada procedure"
+
+    mi_gdb_test "-var-create r * r" \
+ "\\^done,name=\"r\",numchild=\"1\",.*" \
+ "create r varobj"
+
+    set bp_location [gdb_get_line_number "STOP2" ${testdir}/pkg.adb]
+    mi_continue_to_line \
+ "pkg.adb:$bp_location" \
+ "stop at second breakpoint"
+
+    mi_gdb_test "-var-update 1 r" \
+ "\\^done.*name=\"r\",.*new_num_children=\"2\",.*" \
+ "update r varobj"
+}
diff --git a/gdb/testsuite/gdb.ada/mi_variant/pck.ads b/gdb/testsuite/gdb.ada/mi_variant/pck.ads
new file mode 100644
index 00000000000..3895b9c48eb
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/mi_variant/pck.ads
@@ -0,0 +1,54 @@
+--  Copyright 2020 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/>.
+
+package Pck is
+
+   type Rec_Type (C : Character := 'd') is record
+      case C is
+         when Character'First     => X_First : Integer;
+         when Character'Val (127) => X_127   : Integer;
+         when Character'Val (128) => X_128   : Integer;
+         when Character'Last      => X_Last  : Integer;
+         when others              => null;
+      end case;
+   end record;
+
+   type Second_Type (I : Integer) is record
+      One: Integer;
+      case I is
+         when -5 .. 5 =>
+   X : Integer;
+         when others =>
+   Y : Integer;
+      end case;
+   end record;
+
+   type Nested_And_Variable (One, Two: Integer) is record
+       Str : String (1 .. One);
+       case One is
+          when 0 =>
+     null;
+          when others =>
+     OneValue : Integer;
+             Str2 : String (1 .. Two);
+             case Two is
+        when 0 =>
+   null;
+ when others =>
+   TwoValue : Integer;
+             end case;
+       end case;
+   end record;
+end Pck;
diff --git a/gdb/testsuite/gdb.ada/mi_variant/pkg.adb b/gdb/testsuite/gdb.ada/mi_variant/pkg.adb
new file mode 100644
index 00000000000..ffa8e5e070b
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/mi_variant/pkg.adb
@@ -0,0 +1,28 @@
+--  Copyright 2020 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/>.
+
+with Pck; use Pck;
+
+procedure Pkg is
+
+   R : Rec_Type;
+
+begin
+   R := (C => 'd');
+   null; -- STOP
+
+   R := (C => Character'First, X_First => 27);
+   null; -- STOP2
+end Pkg;
diff --git a/gdb/testsuite/gdb.ada/packed_tagged.exp b/gdb/testsuite/gdb.ada/packed_tagged.exp
index 2670dad6046..72ae29c08d4 100644
--- a/gdb/testsuite/gdb.ada/packed_tagged.exp
+++ b/gdb/testsuite/gdb.ada/packed_tagged.exp
@@ -17,24 +17,31 @@ load_lib "ada.exp"
 
 standard_ada_testfile comp_bug
 
-if {[gdb_compile_ada "${srcfile}" "${binfile}" executable [list debug]] != "" } {
-  return -1
-}
+foreach_with_prefix scenario {none all minimal} {
+    set flags {debug}
+    if {$scenario != "none"} {
+ lappend flags additional_flags=-fgnat-encodings=$scenario
+    }
+
+    if {[gdb_compile_ada "${srcfile}" "${binfile}" executable $flags] != ""} {
+ return -1
+    }
 
-clean_restart ${testfile}
+    clean_restart ${testfile}
 
-set bp_location [gdb_get_line_number "STOP" ${testdir}/comp_bug.adb]
-runto "comp_bug.adb:$bp_location"
+    set bp_location [gdb_get_line_number "STOP" ${testdir}/comp_bug.adb]
+    runto "comp_bug.adb:$bp_location"
 
-gdb_test "print x" \
-         "= \\(exists => true, value => 10\\)"
+    gdb_test "print x" \
+ "= \\(exists => true, value => 10\\)"
 
-gdb_test "ptype x" \
-         [multi_line "type = record" \
-                     "    exists: (boolean|range false \\.\\. true);" \
-                     "    case exists is" \
-                     "        when true =>" \
-                     "            value: range 0 \\.\\. 255;" \
-                     "        when others => null;" \
-                     "    end case;" \
-                     "end record" ]
+    gdb_test "ptype x" \
+ [multi_line "type = record" \
+     "    exists: (boolean|range false \\.\\. true);" \
+     "    case exists is" \
+     "        when true =>" \
+     "            value: range 0 \\.\\. 255;" \
+     "        when others => null;" \
+     "    end case;" \
+     "end record" ]
+}
diff --git a/gdb/testsuite/gdb.ada/unchecked_union.exp b/gdb/testsuite/gdb.ada/unchecked_union.exp
index 87a27d286c7..c85d7c33153 100644
--- a/gdb/testsuite/gdb.ada/unchecked_union.exp
+++ b/gdb/testsuite/gdb.ada/unchecked_union.exp
@@ -19,15 +19,6 @@ load_lib "ada.exp"
 
 standard_ada_testfile unchecked_union
 
-if {[gdb_compile_ada "${srcfile}" "${binfile}" executable {debug}] != ""} {
-  return -1
-}
-
-clean_restart ${testfile}
-
-set bp_location [gdb_get_line_number "BREAK" ${testdir}/unchecked_union.adb]
-runto "unchecked_union.adb:$bp_location"
-
 proc multi_line_string {str} {
     set result {}
     foreach line [split $str \n] {
@@ -54,5 +45,21 @@ set pair_string {    case ? is
 }
 set pair_full "type = record\n${inner_string}${pair_string}end record"
 
-gdb_test "ptype Pair" [multi_line_string $pair_full]
-gdb_test "ptype Inner" [multi_line_string $inner_full]
+foreach_with_prefix scenario {none all minimal} {
+    set flags {debug}
+    if {$scenario != "none"} {
+ lappend flags additional_flags=-fgnat-encodings=$scenario
+    }
+
+    if {[gdb_compile_ada "${srcfile}" "${binfile}" executable $flags] != ""} {
+ return -1
+    }
+
+    clean_restart ${testfile}
+
+    set bp_location [gdb_get_line_number "BREAK" ${testdir}/unchecked_union.adb]
+    runto "unchecked_union.adb:$bp_location"
+
+    gdb_test "ptype Pair" [multi_line_string $pair_full]
+    gdb_test "ptype Inner" [multi_line_string $inner_full]
+}
--
2.21.1

Reply | Threaded
Open this post in threaded view
|

[PATCH 10/11] Add Python support for dynamic types

Tom Tromey-4
In reply to this post by Tom Tromey-4
This changes the gdb Python API to add support for dynamic types.  In
particular, this adds an attribute to gdb.Type, and updates some
attributes to reflect dynamic sizes and field offsets.

There's still no way to get the dynamic type from one of its concrete
instances.  This could perhaps be added if needed.

gdb/ChangeLog
2020-04-08  Tom Tromey  <[hidden email]>

        PR python/23662:
        * python/py-type.c (convert_field): Handle
        FIELD_LOC_KIND_DWARF_BLOCK.
        (typy_get_sizeof): Handle TYPE_HAS_DYNAMIC_LENGTH.
        (typy_get_dynamic): Nw function.
        (type_object_getset): Add "dynamic".
        * NEWS: Add entry.

gdb/doc/ChangeLog
2020-04-08  Tom Tromey  <[hidden email]>

        PR python/23662:
        * python.texi (Types In Python): Document new features.

gdb/testsuite/ChangeLog
2020-04-08  Tom Tromey  <[hidden email]>

        PR python/23662:
        * gdb.ada/variant.exp: Add Python checks.
        * gdb.rust/simple.exp: Add dynamic type checks.
---
 gdb/ChangeLog                     | 10 +++++++++
 gdb/NEWS                          |  5 +++++
 gdb/doc/ChangeLog                 |  5 +++++
 gdb/doc/python.texi               | 14 +++++++++++--
 gdb/python/py-type.c              | 35 +++++++++++++++++++++++++++++--
 gdb/testsuite/ChangeLog           |  6 ++++++
 gdb/testsuite/gdb.ada/variant.exp | 10 +++++++++
 gdb/testsuite/gdb.rust/simple.exp | 10 +++++++++
 8 files changed, 91 insertions(+), 4 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 6657f6fadce..01e73c9e5ea 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -68,6 +68,11 @@ GNU/Linux/RISC-V (gdbserver) riscv*-*-linux*
   ** gdb.register_window_type can be used to implement new TUI windows
      in Python.
 
+  ** Dynamic types can now be queried.  gdb.Type has a new attribute,
+     "dynamic", and gdb.Type.sizeof can be None for a dynamic type.  A
+     field of a dynamic type may have None for its "bitpos" attribute
+     as well.
+
 *** Changes in GDB 9
 
 * 'thread-exited' event is now available in the annotations interface.
diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index 76cdf7f5419..bcc27f6a224 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -1068,6 +1068,12 @@ The type code for this type.  The type code will be one of the
 @code{TYPE_CODE_} constants defined below.
 @end defvar
 
+@defvar Type.dynamic
+A boolean indicating whether this type is dynamic.  In some
+situations, such as Rust @code{enum} types or Ada variant records, the
+concrete type of a value may vary depending on its contents.
+@end defvar
+
 @defvar Type.name
 The name of this type.  If this type has no name, then @code{None}
 is returned.
@@ -1076,7 +1082,9 @@ is returned.
 @defvar Type.sizeof
 The size of this type, in target @code{char} units.  Usually, a
 target's @code{char} type will be an 8-bit byte.  However, on some
-unusual platforms, this type may have a different size.
+unusual platforms, this type may have a different size.  A dynamic
+type may not have a fixed size; in this case, this attribute's value
+will be @code{None}.
 @end defvar
 
 @defvar Type.tag
@@ -1106,7 +1114,9 @@ Each field is a @code{gdb.Field} object, with some pre-defined attributes:
 @item bitpos
 This attribute is not available for @code{enum} or @code{static}
 (as in C@t{++}) fields.  The value is the position, counting
-in bits, from the start of the containing type.
+in bits, from the start of the containing type.  Note that, in a
+dynamic type, the position of a field may not be constant.  In this
+case, the value will be @code{None}.
 
 @item enumval
 This attribute is only available for @code{enum} fields, and its value
diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c
index b19cad098a4..acdd80fccc3 100644
--- a/gdb/python/py-type.c
+++ b/gdb/python/py-type.c
@@ -189,8 +189,11 @@ convert_field (struct type *type, int field)
  }
       else
  {
-  arg.reset (gdb_py_long_from_longest (TYPE_FIELD_BITPOS (type,
-  field)));
+  if (TYPE_FIELD_LOC_KIND (type, field) == FIELD_LOC_KIND_DWARF_BLOCK)
+    arg = gdbpy_ref<>::new_reference (Py_None);
+  else
+    arg.reset (gdb_py_long_from_longest (TYPE_FIELD_BITPOS (type,
+    field)));
   attrstring = "bitpos";
  }
 
@@ -710,9 +713,12 @@ typy_get_sizeof (PyObject *self, void *closure)
 {
   struct type *type = ((type_object *) self)->type;
 
+  bool size_varies = false;
   try
     {
       check_typedef (type);
+
+      size_varies = TYPE_HAS_DYNAMIC_LENGTH (type);
     }
   catch (const gdb_exception &except)
     {
@@ -720,6 +726,8 @@ typy_get_sizeof (PyObject *self, void *closure)
 
   /* Ignore exceptions.  */
 
+  if (size_varies)
+    Py_RETURN_NONE;
   return gdb_py_long_from_longest (TYPE_LENGTH (type));
 }
 
@@ -744,6 +752,27 @@ typy_get_alignof (PyObject *self, void *closure)
   return gdb_py_object_from_ulongest (align).release ();
 }
 
+/* Return whether or not the type is dynamic.  */
+static PyObject *
+typy_get_dynamic (PyObject *self, void *closure)
+{
+  struct type *type = ((type_object *) self)->type;
+
+  bool result = false;
+  try
+    {
+      result = is_dynamic_type (type);
+    }
+  catch (const gdb_exception &except)
+    {
+      /* Ignore exceptions.  */
+    }
+
+  if (result)
+    Py_RETURN_TRUE;
+  Py_RETURN_FALSE;
+}
+
 static struct type *
 typy_lookup_typename (const char *type_name, const struct block *block)
 {
@@ -1436,6 +1465,8 @@ static gdb_PyGetSetDef type_object_getset[] =
     "The alignment of this type, in bytes.", NULL },
   { "code", typy_get_code, NULL,
     "The code for this type.", NULL },
+  { "dynamic", typy_get_dynamic, NULL,
+    "Whether this type is dynamic.", NULL },
   { "name", typy_get_name, NULL,
     "The name for this type, or None.", NULL },
   { "sizeof", typy_get_sizeof, NULL,
diff --git a/gdb/testsuite/gdb.ada/variant.exp b/gdb/testsuite/gdb.ada/variant.exp
index 490956a2666..da51f7ba2e8 100644
--- a/gdb/testsuite/gdb.ada/variant.exp
+++ b/gdb/testsuite/gdb.ada/variant.exp
@@ -14,6 +14,7 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 load_lib "ada.exp"
+load_lib "gdb-python.exp"
 
 standard_ada_testfile pkg
 
@@ -43,4 +44,13 @@ foreach_with_prefix scenario {none all minimal} {
  " = \\(one => 3, two => 0, str => \"zzz\", onevalue => 33, str2 => \"\"\\)"
     gdb_test "print nav3" \
  " = \\(one => 3, two => 7, str => \"zzz\", onevalue => 33, str2 => \"qqqqqqq\", twovalue => 88\\)"
+
+    # This is only supported for the DWARF encoding.
+    if {$scenario == "minimal" && ![skip_python_tests]} {
+ gdb_test_no_output \
+    "python t = gdb.lookup_type('nested_and_variable')" \
+    "fetch type for python"
+ gdb_test "python print(t.dynamic)" "True"
+ gdb_test "python print(t\['onevalue'\].bitpos)" "None"
+    }
 }
diff --git a/gdb/testsuite/gdb.rust/simple.exp b/gdb/testsuite/gdb.rust/simple.exp
index 92b3666386b..6daaf8415c5 100644
--- a/gdb/testsuite/gdb.rust/simple.exp
+++ b/gdb/testsuite/gdb.rust/simple.exp
@@ -364,3 +364,13 @@ if {[skip_python_tests]} {
 }
 
 gdb_test "python print(gdb.lookup_type('simple::HiBob'))" "simple::HiBob"
+
+gdb_test_no_output "python e = gdb.parse_and_eval('e')" \
+    "get value of e for python"
+gdb_test "python print(len(e.type.fields()))" "2"
+gdb_test "python print(e.type.fields()\[0\].artificial)" "True"
+gdb_test "python print(e.type.fields()\[1\].name)" "Two"
+
+gdb_test "python print(e.type.dynamic)" "False"
+gdb_test "python print(gdb.lookup_type('simple::MoreComplicated').dynamic)" \
+    "True"
--
2.21.1

Reply | Threaded
Open this post in threaded view
|

[PATCH 11/11] Update test cases that work with minimal encodings

Tom Tromey-4
In reply to this post by Tom Tromey-4
Some test cases already work fine with minimal encodings (in some
cases perhaps due to the variant parts series) This patch updates
these tests as appropriate.

gdb/testsuite/ChangeLog
2020-04-08  Tom Tromey  <[hidden email]>

        * gdb.ada/frame_arg_lang.exp: Run with multiple -fgnat-encodings
        values.
        * gdb.ada/funcall_ref.exp: Run with multiple -fgnat-encodings
        values.  Update test for minimal encodings.
        * gdb.ada/lang_switch.exp: Update test for minimal encodings.
        * gdb.ada/var_rec_arr.exp: Run with multiple -fgnat-encodings
        values.  Update test for minimal encodings.
---
 gdb/testsuite/ChangeLog                  | 10 +++
 gdb/testsuite/gdb.ada/frame_arg_lang.exp | 93 +++++++++++++---------
 gdb/testsuite/gdb.ada/funcall_ref.exp    | 98 +++++++++++++++---------
 gdb/testsuite/gdb.ada/lang_switch.exp    |  5 +-
 gdb/testsuite/gdb.ada/packed_tagged.exp  | 46 ++++++++++-
 gdb/testsuite/gdb.ada/var_rec_arr.exp    | 73 +++++++++++-------
 6 files changed, 221 insertions(+), 104 deletions(-)

diff --git a/gdb/testsuite/gdb.ada/frame_arg_lang.exp b/gdb/testsuite/gdb.ada/frame_arg_lang.exp
index 92baf7dfa10..dc08d261334 100644
--- a/gdb/testsuite/gdb.ada/frame_arg_lang.exp
+++ b/gdb/testsuite/gdb.ada/frame_arg_lang.exp
@@ -21,53 +21,70 @@ set csrcfile ${srcdir}/${subdir}/${testdir}/${cfile}.c
 set cobject [standard_output_file ${cfile}.o]
 
 gdb_compile "${csrcfile}" "${cobject}" object [list debug]
-if {[gdb_compile_ada "${srcfile}" "${binfile}" executable [list debug additional_flags=-largs additional_flags=${cobject} additional_flags=-margs]] != "" } {
-  return -1
-}
 
-clean_restart ${testfile}
+# Note we don't test the "none" (no -fgnat-encodings option) scenario
+# here, because "all" and "minimal" cover the cases, and this way we
+# don't have to update the test when gnat changes its default.
+foreach_with_prefix scenario {all minimal} {
+    set flags [list debug additional_flags=-largs \
+   additional_flags=${cobject} \
+   additional_flags=-margs \
+   additional_flags=-fgnat-encodings=$scenario]
+
+    if {[gdb_compile_ada "${srcfile}" "${binfile}" executable $flags] != ""} {
+ return -1
+    }
 
-set bp_location [gdb_get_line_number "STOP" ${testdir}/foo.c]
-runto "foo.c:$bp_location"
+    clean_restart ${testfile}
 
-gdb_test_no_output "set print frame-arguments all"
+    set bp_location [gdb_get_line_number "STOP" ${testdir}/foo.c]
+    runto "foo.c:$bp_location"
 
-# Here is the scenario:
-#  - Once stopped in a C function, with language_mode set to auto, print
-#    backtrace, we should see the Ada frame arguments printed using Ada
-#    syntax.
-#  - Set language to C, then check that printing backtrace shows the Ada
-#    frame arguments using C syntax.
-#  - Set language back to auto, check language mode value, then print
-#    backtrace, we should see Ada frame arguments printed using Ada C
-#    syntax.
+    gdb_test_no_output "set print frame-arguments all"
 
-gdb_test "show lang" \
-         "The current source language is \"auto; currently c\"." \
-         "show language when set to 'auto; c'"
+    # Here is the scenario:
+    #  - Once stopped in a C function, with language_mode set to auto, print
+    #    backtrace, we should see the Ada frame arguments printed using Ada
+    #    syntax.
+    #  - Set language to C, then check that printing backtrace shows the Ada
+    #    frame arguments using C syntax.
+    #  - Set language back to auto, check language mode value, then print
+    #    backtrace, we should see Ada frame arguments printed using Ada C
+    #    syntax.
 
-gdb_test "bt" \
-         "#1  $hex in pck\\.call_me \\(s=\"test\"\\).*" \
-         "backtrace with auto: c"
+    gdb_test "show lang" \
+ "The current source language is \"auto; currently c\"." \
+ "show language when set to 'auto; c'"
 
-gdb_test_no_output "set language c" \
-                   "Set current source language to \"manual; currently c\"."
+    gdb_test "bt" \
+ "#1  $hex in pck\\.call_me \\(s=\"test\"\\).*" \
+ "backtrace with auto: c"
 
-gdb_test "show lang" \
-         "The current source language is \"c\"." \
-         "show language when set to 'c'"
+    gdb_test_no_output "set language c" \
+ "Set current source language to \"manual; currently c\"."
 
-gdb_test "bt" \
-         "#1  $hex in pck\\.call_me \\(s={P_ARRAY = $hex, P_BOUNDS = $hex}\\).*" \
-         "backtrace with language forced to 'c'"
+    gdb_test "show lang" \
+ "The current source language is \"c\"." \
+ "show language when set to 'c'"
 
-gdb_test_no_output "set language auto" \
-                   "Set current source language to \"auto; currently c\"."
+    # With -fgnat-encodings=minimal, this works properly in C as well.
+    if {$scenario == "minimal"} {
+ set expected "\"test\""
+    } else {
+ set expected "{P_ARRAY = $hex, P_BOUNDS = $hex}"
+    }
+    gdb_test "bt" \
+ "#1  $hex in pck\\.call_me \\(s=$expected\\).*" \
+ "backtrace with language forced to 'c'"
 
-gdb_test "show lang" \
-         "The current source language is \"auto; currently c\"." \
-         "show language when set back to 'auto; c'"
+    gdb_test_no_output "set language auto" \
+ "Set current source language to \"auto; currently c\"."
 
-gdb_test "bt" \
-         "#1  $hex in pck\\.call_me \\(s=\"test\"\\).*" \
-         "backtrace with language back to 'auto; c'"
+    gdb_test "show lang" \
+ "The current source language is \"auto; currently c\"." \
+ "show language when set back to 'auto; c'"
+
+    gdb_test "bt" \
+ "#1  $hex in pck\\.call_me \\(s=\"test\"\\).*" \
+ "backtrace with language back to 'auto; c'"
+}
diff --git a/gdb/testsuite/gdb.ada/funcall_ref.exp b/gdb/testsuite/gdb.ada/funcall_ref.exp
index 02664f6ad32..e260e908643 100644
--- a/gdb/testsuite/gdb.ada/funcall_ref.exp
+++ b/gdb/testsuite/gdb.ada/funcall_ref.exp
@@ -17,43 +17,71 @@ load_lib "ada.exp"
 
 standard_ada_testfile foo
 
-if {[gdb_compile_ada "${srcfile}" "${binfile}" executable \
-                     [list debug]] != "" } {
-  return -1
-}
+# Note we don't test the "none" (no -fgnat-encodings option) scenario
+# here, because "all" and "minimal" cover the cases, and this way we
+# don't have to update the test when gnat changes its default.
+foreach_with_prefix scenario {all minimal} {
+    set flags [list debug additional_flags=-fgnat-encodings=$scenario]
 
-clean_restart ${testfile}
-
-set bp_location [gdb_get_line_number "STOP" ${testdir}/foo.adb]
-runto "foo.adb:$bp_location"
-
-# Test printing and type-printing of a discriminated record that a function
-# returns by reference.
-
-# Currently, GCC describes such functions as returning pointers (instead of
-# references).
-set pass_re [multi_line "type = <ref> record" \
- "    n: natural;" \
- "    s: access array \\(1 \\.\\. n\\) of character;" \
- "end record"]
-set unsupported_re [multi_line "type = access record" \
- "    n: natural;" \
- "    s: access array \\(1 \\.\\. n\\) of character;" \
- "end record"]
-set supported 1
-gdb_test_multiple "ptype get (\"Hello world!\")" "" {
-    -re -wrap $pass_re {
- pass $gdb_test_name
+    if {[gdb_compile_ada "${srcfile}" "${binfile}" executable $flags] != ""} {
+ return -1
     }
-    -re -wrap $unsupported_re {
- unsupported $gdb_test_name
- set supported 0
+
+    clean_restart ${testfile}
+
+    set bp_location [gdb_get_line_number "STOP" ${testdir}/foo.adb]
+    runto "foo.adb:$bp_location"
+
+    # Test printing and type-printing of a discriminated record that a function
+    # returns by reference.
+
+    # Currently, GCC describes such functions as returning pointers (instead of
+    # references).
+    set pass_re [multi_line "type = <ref> record" \
+     "    n: natural;" \
+     "    s: access array \\(1 \\.\\. n\\) of character;" \
+     "end record"]
+    # With DWARF we get debuginfo that could in theory show "1..n" for
+    # the range:
+    #     <3><1230>: Abbrev Number: 15 (DW_TAG_member)
+    #     <1231>   DW_AT_name        : n
+    # ...
+    #  <4><1257>: Abbrev Number: 18 (DW_TAG_subrange_type)
+    #     <1258>   DW_AT_type        : <0x126e>
+    #     <125c>   DW_AT_upper_bound : <0x1230>
+    # However, we don't currently record the needed information in the
+    # location batons.  In the meantime, we accept and kfail the
+    # compromise output.
+    set dwarf_kfail_re [multi_line "type = <ref> record" \
+    "    n: natural;" \
+    "    s: array \\(<>\\) of character;" \
+    "end record"]
+    set unsupported_re [multi_line "type = access record" \
+    "    n: natural;" \
+    "    s: access array \\(1 \\.\\. n\\) of character;" \
+    "end record"]
+    set supported 1
+    gdb_test_multiple "ptype get (\"Hello world!\")" "" {
+ -re -wrap $pass_re {
+    pass $gdb_test_name
+ }
+ -re -wrap $dwarf_kfail_re {
+    if {$scenario == "minimal"} {
+ setup_kfail "symbolic names in location batons" *-*-*
+    }
+    fail $gdb_test_name
+    set supported 0
+ }
+ -re -wrap $unsupported_re {
+    unsupported $gdb_test_name
+    set supported 0
+ }
     }
-}
 
-if { $supported == 0 } {
-    return 0
-}
+    if { $supported == 0 } {
+ return 0
+    }
 
-gdb_test "p get (\"Hello world!\")" \
-    "= \\(n => 12, s => \"Hello world!\"\\)"
+    gdb_test "p get (\"Hello world!\")" \
+ "= \\(n => 12, s => \"Hello world!\"\\)"
+}
diff --git a/gdb/testsuite/gdb.ada/lang_switch.exp b/gdb/testsuite/gdb.ada/lang_switch.exp
index 006cae05913..7d9bd617504 100644
--- a/gdb/testsuite/gdb.ada/lang_switch.exp
+++ b/gdb/testsuite/gdb.ada/lang_switch.exp
@@ -41,6 +41,9 @@ gdb_test "bt" \
 # Now, make sure that the language doesn't get automatically switched
 # if the current language is not "auto".
 gdb_test "set lang c"
+# This gives different output with -fgnat-encodings=minimal and
+# -fgnat-encodings=all, but since we don't care so much about the
+# precise details here, we just accept anything.
 gdb_test "bt" \
-         ".*#1.*lang_switch\\.ada_procedure\\s*\\(msg=(@$hex: +)?{.*\\).*" \
+         ".*#1.*lang_switch\\.ada_procedure\\s*\\(msg=(@$hex: +)?.*\\).*" \
          "backtrace with lang set to C"
diff --git a/gdb/testsuite/gdb.ada/packed_tagged.exp b/gdb/testsuite/gdb.ada/packed_tagged.exp
index 72ae29c08d4..d6ee8454c5a 100644
--- a/gdb/testsuite/gdb.ada/packed_tagged.exp
+++ b/gdb/testsuite/gdb.ada/packed_tagged.exp
@@ -17,7 +17,10 @@ load_lib "ada.exp"
 
 standard_ada_testfile comp_bug
 
-foreach_with_prefix scenario {none all minimal} {
+# Note we don't test the "none" (no -fgnat-encodings option) scenario
+# here, because "all" and "minimal" cover the cases, and this way we
+# don't have to update the test when gnat changes its default.
+foreach_with_prefix scenario {all minimal} {
     set flags {debug}
     if {$scenario != "none"} {
  lappend flags additional_flags=-fgnat-encodings=$scenario
@@ -32,10 +35,25 @@ foreach_with_prefix scenario {none all minimal} {
     set bp_location [gdb_get_line_number "STOP" ${testdir}/comp_bug.adb]
     runto "comp_bug.adb:$bp_location"
 
-    gdb_test "print x" \
+    set pass_re \
  "= \\(exists => true, value => 10\\)"
+    # There is a compiler bug that causes this output.
+    set kfail_re \
+ "= \\(exists => true\\)"
 
-    gdb_test "ptype x" \
+    gdb_test_multiple "print x" "" {
+ -re -wrap $pass_re {
+    pass $gdb_test_name
+ }
+ -re -wrap $kfail_re {
+    if {$scenario == "minimal"} {
+ setup_kfail "gnat compiler bug" *-*-*
+    }
+    fail $gdb_test_name
+ }
+    }
+
+    set pass_re \
  [multi_line "type = record" \
      "    exists: (boolean|range false \\.\\. true);" \
      "    case exists is" \
@@ -44,4 +62,26 @@ foreach_with_prefix scenario {none all minimal} {
      "        when others => null;" \
      "    end case;" \
      "end record" ]
+    # There is a compiler bug that causes this output.
+    set kfail_re \
+ [multi_line "type = record" \
+     "    exists: (boolean|range false \\.\\. true);" \
+     "    case \\? is" \
+     "        when others =>" \
+     "            value: range 0 \\.\\. 255;" \
+     "        when others => null;" \
+     "    end case;" \
+     "end record" ]
+
+    gdb_test_multiple "ptype x" "" {
+ -re -wrap $pass_re {
+    pass $gdb_test_name
+ }
+ -re -wrap $kfail_re {
+    if {$scenario == "minimal"} {
+ setup_kfail "gnat compiler bug" *-*-*
+    }
+    fail $gdb_test_name
+ }
+    }
 }
diff --git a/gdb/testsuite/gdb.ada/var_rec_arr.exp b/gdb/testsuite/gdb.ada/var_rec_arr.exp
index 146ad5e8dc0..80ec32616a9 100644
--- a/gdb/testsuite/gdb.ada/var_rec_arr.exp
+++ b/gdb/testsuite/gdb.ada/var_rec_arr.exp
@@ -17,41 +17,60 @@ load_lib "ada.exp"
 
 standard_ada_testfile foo_na09_042
 
-if {[gdb_compile_ada "${srcfile}" "${binfile}" executable [list debug]] != "" } {
-  return -1
-}
+# Note we don't test the "none" (no -fgnat-encodings option) scenario
+# here, because "all" and "minimal" cover the cases, and this way we
+# don't have to update the test when gnat changes its default.
+foreach_with_prefix scenario {all minimal} {
+    set flags [list debug additional_flags=-fgnat-encodings=$scenario]
+
+    if {[gdb_compile_ada "${srcfile}" "${binfile}" executable $flags] != ""} {
+ return -1
+    }
 
-clean_restart ${testfile}
+    clean_restart ${testfile}
 
-set bp_location [gdb_get_line_number "STOP" ${testdir}/foo_na09_042.adb]
-runto "foo_na09_042.adb:$bp_location"
+    set bp_location [gdb_get_line_number "STOP" ${testdir}/foo_na09_042.adb]
+    runto "foo_na09_042.adb:$bp_location"
 
-gdb_test "print a1" \
-         " = \\(\\(i => 0, s => \"\"\\), \\(i => 1, s => \"A\"\\), \\(i => 2, s => \"AB\"\\)\\)"
+    gdb_test "print a1" \
+ " = \\(\\(i => 0, s => \"\"\\), \\(i => 1, s => \"A\"\\), \\(i => 2, s => \"AB\"\\)\\)"
 
-gdb_test "print a1(1)" \
-         " = \\(i => 0, s => \"\"\\)"
+    gdb_test "print a1(1)" \
+ " = \\(i => 0, s => \"\"\\)"
 
-gdb_test "print a1(2)" \
-         " = \\(i => 1, s => \"A\"\\)"
+    gdb_test "print a1(2)" \
+ " = \\(i => 1, s => \"A\"\\)"
 
-gdb_test "print a1(3)" \
-         " = \\(i => 2, s => \"AB\"\\)"
+    gdb_test "print a1(3)" \
+ " = \\(i => 2, s => \"AB\"\\)"
 
-gdb_test "print a2" \
-         " = \\(\\(i => 2, s => \"AB\"\\), \\(i => 1, s => \"A\"\\), \\(i => 0, s => \"\"\\)\\)"
+    gdb_test "print a2" \
+ " = \\(\\(i => 2, s => \"AB\"\\), \\(i => 1, s => \"A\"\\), \\(i => 0, s => \"\"\\)\\)"
 
-gdb_test "print a2(1)" \
-         " = \\(i => 2, s => \"AB\"\\)"
+    gdb_test "print a2(1)" \
+ " = \\(i => 2, s => \"AB\"\\)"
 
-gdb_test "print a2(2)" \
-         " = \\(i => 1, s => \"A\"\\)"
+    gdb_test "print a2(2)" \
+ " = \\(i => 1, s => \"A\"\\)"
 
-gdb_test "print a2(3)" \
-         " = \\(i => 0, s => \"\"\\)"
+    gdb_test "print a2(3)" \
+ " = \\(i => 0, s => \"\"\\)"
 
-gdb_test "ptype a1(1)" \
-         [multi_line "type = record" \
-                     "    i: pck\\.small_type;" \
-                     "    s: access array \\((<>|1 \\.\\. i)\\) of character;" \
-                     "end record"]
+    # Note that the "access" is only printed when the gnat encodings
+    # are used.  This is due to how the encodings work -- the type
+    # doesn't actually have the "access", and so here the DWARF
+    # encoding is more correct.
+    if {$scenario == "all"} {
+ set ex [multi_line "type = record" \
+    "    i: pck\\.small_type;" \
+    "    s: access array \\((<>|1 \\.\\. i)\\) of character;" \
+    "end record"]
+    } else {
+ set ex [multi_line "type = record" \
+    "    i: pck\\.small_type;" \
+    "    s: array \\((<>|1 \\.\\. i)\\) of character;" \
+    "end record"]
+    }
+
+    gdb_test "ptype a1(1)" $ex
+}
--
2.21.1

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 10/11] Add Python support for dynamic types

Eli Zaretskii
In reply to this post by Tom Tromey-4
> From: Tom Tromey <[hidden email]>
> Date: Wed,  8 Apr 2020 11:54:51 -0600
> Cc: Tom Tromey <[hidden email]>
>
> This changes the gdb Python API to add support for dynamic types.  In
> particular, this adds an attribute to gdb.Type, and updates some
> attributes to reflect dynamic sizes and field offsets.
>
> There's still no way to get the dynamic type from one of its concrete
> instances.  This could perhaps be added if needed.
>
> gdb/ChangeLog
> 2020-04-08  Tom Tromey  <[hidden email]>
>
> PR python/23662:
> * python/py-type.c (convert_field): Handle
> FIELD_LOC_KIND_DWARF_BLOCK.
> (typy_get_sizeof): Handle TYPE_HAS_DYNAMIC_LENGTH.
> (typy_get_dynamic): Nw function.
> (type_object_getset): Add "dynamic".
> * NEWS: Add entry.
>
> gdb/doc/ChangeLog
> 2020-04-08  Tom Tromey  <[hidden email]>
>
> PR python/23662:
> * python.texi (Types In Python): Document new features.
>
> gdb/testsuite/ChangeLog
> 2020-04-08  Tom Tromey  <[hidden email]>
>
> PR python/23662:
> * gdb.ada/variant.exp: Add Python checks.
> * gdb.rust/simple.exp: Add dynamic type checks.

Thanks, the documentation parts are OK.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 10/11] Add Python support for dynamic types

Sourceware - gdb-patches mailing list
In reply to this post by Tom Tromey-4
On Wed, Apr 8, 2020 at 12:55 PM Tom Tromey <[hidden email]> wrote:

>
> This changes the gdb Python API to add support for dynamic types.  In
> particular, this adds an attribute to gdb.Type, and updates some
> attributes to reflect dynamic sizes and field offsets.
>
> There's still no way to get the dynamic type from one of its concrete
> instances.  This could perhaps be added if needed.
>
> gdb/ChangeLog
> 2020-04-08  Tom Tromey  <[hidden email]>
>
>         PR python/23662:
>         * python/py-type.c (convert_field): Handle
>         FIELD_LOC_KIND_DWARF_BLOCK.
>         (typy_get_sizeof): Handle TYPE_HAS_DYNAMIC_LENGTH.
>         (typy_get_dynamic): Nw function.
>         (type_object_getset): Add "dynamic".
>         * NEWS: Add entry.
>
> gdb/doc/ChangeLog
> 2020-04-08  Tom Tromey  <[hidden email]>
>
>         PR python/23662:
>         * python.texi (Types In Python): Document new features.
>
> gdb/testsuite/ChangeLog
> 2020-04-08  Tom Tromey  <[hidden email]>
>
>         PR python/23662:
>         * gdb.ada/variant.exp: Add Python checks.
>         * gdb.rust/simple.exp: Add dynamic type checks.
> ---
>  gdb/ChangeLog                     | 10 +++++++++
>  gdb/NEWS                          |  5 +++++
>  gdb/doc/ChangeLog                 |  5 +++++
>  gdb/doc/python.texi               | 14 +++++++++++--
>  gdb/python/py-type.c              | 35 +++++++++++++++++++++++++++++--
>  gdb/testsuite/ChangeLog           |  6 ++++++
>  gdb/testsuite/gdb.ada/variant.exp | 10 +++++++++
>  gdb/testsuite/gdb.rust/simple.exp | 10 +++++++++
>  8 files changed, 91 insertions(+), 4 deletions(-)
>
> diff --git a/gdb/NEWS b/gdb/NEWS
> index 6657f6fadce..01e73c9e5ea 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -68,6 +68,11 @@ GNU/Linux/RISC-V (gdbserver) riscv*-*-linux*
>    ** gdb.register_window_type can be used to implement new TUI windows
>       in Python.
>
> +  ** Dynamic types can now be queried.  gdb.Type has a new attribute,
> +     "dynamic", and gdb.Type.sizeof can be None for a dynamic type.  A
> +     field of a dynamic type may have None for its "bitpos" attribute
> +     as well.
> +
>  *** Changes in GDB 9
>
>  * 'thread-exited' event is now available in the annotations interface.
> diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
> index 76cdf7f5419..bcc27f6a224 100644
> --- a/gdb/doc/python.texi
> +++ b/gdb/doc/python.texi
> @@ -1068,6 +1068,12 @@ The type code for this type.  The type code will be one of the
>  @code{TYPE_CODE_} constants defined below.
>  @end defvar
>
> +@defvar Type.dynamic
> +A boolean indicating whether this type is dynamic.  In some
> +situations, such as Rust @code{enum} types or Ada variant records, the
> +concrete type of a value may vary depending on its contents.
> +@end defvar
> +
>  @defvar Type.name
>  The name of this type.  If this type has no name, then @code{None}
>  is returned.
> @@ -1076,7 +1082,9 @@ is returned.
>  @defvar Type.sizeof
>  The size of this type, in target @code{char} units.  Usually, a
>  target's @code{char} type will be an 8-bit byte.  However, on some
> -unusual platforms, this type may have a different size.
> +unusual platforms, this type may have a different size.  A dynamic
> +type may not have a fixed size; in this case, this attribute's value
> +will be @code{None}.
>  @end defvar
>
>  @defvar Type.tag
> @@ -1106,7 +1114,9 @@ Each field is a @code{gdb.Field} object, with some pre-defined attributes:
>  @item bitpos
>  This attribute is not available for @code{enum} or @code{static}
>  (as in C@t{++}) fields.  The value is the position, counting
> -in bits, from the start of the containing type.
> +in bits, from the start of the containing type.  Note that, in a
> +dynamic type, the position of a field may not be constant.  In this
> +case, the value will be @code{None}.

Just to make sure I understand, if I have a type like this (using
Haskell-like syntax, I hope this makes sense):

Type t =
  Int x |
  Float y |
  (String a, Float b)

This would be reflected in Python as the type having fields x, y, a
and b, all of which have a None position?

Is there a way to access these fields, and is there a way to see which
specific type a certain variable value is?

Christian

>
>  @item enumval
>  This attribute is only available for @code{enum} fields, and its value
> diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c
> index b19cad098a4..acdd80fccc3 100644
> --- a/gdb/python/py-type.c
> +++ b/gdb/python/py-type.c
> @@ -189,8 +189,11 @@ convert_field (struct type *type, int field)
>         }
>        else
>         {
> -         arg.reset (gdb_py_long_from_longest (TYPE_FIELD_BITPOS (type,
> -                                                                 field)));
> +         if (TYPE_FIELD_LOC_KIND (type, field) == FIELD_LOC_KIND_DWARF_BLOCK)
> +           arg = gdbpy_ref<>::new_reference (Py_None);
> +         else
> +           arg.reset (gdb_py_long_from_longest (TYPE_FIELD_BITPOS (type,
> +                                                                   field)));
>           attrstring = "bitpos";
>         }
>
> @@ -710,9 +713,12 @@ typy_get_sizeof (PyObject *self, void *closure)
>  {
>    struct type *type = ((type_object *) self)->type;
>
> +  bool size_varies = false;
>    try
>      {
>        check_typedef (type);
> +
> +      size_varies = TYPE_HAS_DYNAMIC_LENGTH (type);
>      }
>    catch (const gdb_exception &except)
>      {
> @@ -720,6 +726,8 @@ typy_get_sizeof (PyObject *self, void *closure)
>
>    /* Ignore exceptions.  */
>
> +  if (size_varies)
> +    Py_RETURN_NONE;
>    return gdb_py_long_from_longest (TYPE_LENGTH (type));
>  }
>
> @@ -744,6 +752,27 @@ typy_get_alignof (PyObject *self, void *closure)
>    return gdb_py_object_from_ulongest (align).release ();
>  }
>
> +/* Return whether or not the type is dynamic.  */
> +static PyObject *
> +typy_get_dynamic (PyObject *self, void *closure)
> +{
> +  struct type *type = ((type_object *) self)->type;
> +
> +  bool result = false;
> +  try
> +    {
> +      result = is_dynamic_type (type);
> +    }
> +  catch (const gdb_exception &except)
> +    {
> +      /* Ignore exceptions.  */
> +    }
> +
> +  if (result)
> +    Py_RETURN_TRUE;
> +  Py_RETURN_FALSE;
> +}
> +
>  static struct type *
>  typy_lookup_typename (const char *type_name, const struct block *block)
>  {
> @@ -1436,6 +1465,8 @@ static gdb_PyGetSetDef type_object_getset[] =
>      "The alignment of this type, in bytes.", NULL },
>    { "code", typy_get_code, NULL,
>      "The code for this type.", NULL },
> +  { "dynamic", typy_get_dynamic, NULL,
> +    "Whether this type is dynamic.", NULL },
>    { "name", typy_get_name, NULL,
>      "The name for this type, or None.", NULL },
>    { "sizeof", typy_get_sizeof, NULL,
> diff --git a/gdb/testsuite/gdb.ada/variant.exp b/gdb/testsuite/gdb.ada/variant.exp
> index 490956a2666..da51f7ba2e8 100644
> --- a/gdb/testsuite/gdb.ada/variant.exp
> +++ b/gdb/testsuite/gdb.ada/variant.exp
> @@ -14,6 +14,7 @@
>  # along with this program.  If not, see <http://www.gnu.org/licenses/>.
>
>  load_lib "ada.exp"
> +load_lib "gdb-python.exp"
>
>  standard_ada_testfile pkg
>
> @@ -43,4 +44,13 @@ foreach_with_prefix scenario {none all minimal} {
>         " = \\(one => 3, two => 0, str => \"zzz\", onevalue => 33, str2 => \"\"\\)"
>      gdb_test "print nav3" \
>         " = \\(one => 3, two => 7, str => \"zzz\", onevalue => 33, str2 => \"qqqqqqq\", twovalue => 88\\)"
> +
> +    # This is only supported for the DWARF encoding.
> +    if {$scenario == "minimal" && ![skip_python_tests]} {
> +       gdb_test_no_output \
> +           "python t = gdb.lookup_type('nested_and_variable')" \
> +           "fetch type for python"
> +       gdb_test "python print(t.dynamic)" "True"
> +       gdb_test "python print(t\['onevalue'\].bitpos)" "None"
> +    }
>  }
> diff --git a/gdb/testsuite/gdb.rust/simple.exp b/gdb/testsuite/gdb.rust/simple.exp
> index 92b3666386b..6daaf8415c5 100644
> --- a/gdb/testsuite/gdb.rust/simple.exp
> +++ b/gdb/testsuite/gdb.rust/simple.exp
> @@ -364,3 +364,13 @@ if {[skip_python_tests]} {
>  }
>
>  gdb_test "python print(gdb.lookup_type('simple::HiBob'))" "simple::HiBob"
> +
> +gdb_test_no_output "python e = gdb.parse_and_eval('e')" \
> +    "get value of e for python"
> +gdb_test "python print(len(e.type.fields()))" "2"
> +gdb_test "python print(e.type.fields()\[0\].artificial)" "True"
> +gdb_test "python print(e.type.fields()\[1\].name)" "Two"
> +
> +gdb_test "python print(e.type.dynamic)" "False"
> +gdb_test "python print(gdb.lookup_type('simple::MoreComplicated').dynamic)" \
> +    "True"
> --
> 2.21.1
>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 10/11] Add Python support for dynamic types

Tom Tromey-2
>>>>> "Christian" == Christian Biesinger via Gdb-patches <[hidden email]> writes:

>> This attribute is not available for @code{enum} or @code{static}
>> (as in C@t{++}) fields.  The value is the position, counting
>> -in bits, from the start of the containing type.
>> +in bits, from the start of the containing type.  Note that, in a
>> +dynamic type, the position of a field may not be constant.  In this
>> +case, the value will be @code{None}.

Christian> Just to make sure I understand, if I have a type like this (using
Christian> Haskell-like syntax, I hope this makes sense):

Christian> Type t =
Christian>   Int x |
Christian>   Float y |
Christian>   (String a, Float b)

Christian> This would be reflected in Python as the type having fields x, y, a
Christian> and b, all of which have a None position?

IIUC this would have field x, y, and a field with an anonymous structure
type, that in turn has fields a and b.  But otherwise yes.

Christian> Is there a way to access these fields, and is there a way to see which
Christian> specific type a certain variable value is?

Yes, the type of a value will be a concrete instance of such a dynamic
type.

The Rust test is similar:

>> +gdb_test "python print(len(e.type.fields()))" "2"
>> +gdb_test "python print(e.type.fields()\[0\].artificial)" "True"
>> +gdb_test "python print(e.type.fields()\[1\].name)" "Two"
>> +
>> +gdb_test "python print(e.type.dynamic)" "False"
>> +gdb_test "python print(gdb.lookup_type('simple::MoreComplicated').dynamic)" \
>> +    "True"

This is inspecting a dynamic type "MoreComplicated" and a value of that
type, "e".

MoreComplicated is an enum, which is the Rust equivalent of your Haskell
example.

    enum MoreComplicated {
        One,
        Two(i32),
        Three(HiBob),
        Four{this: bool, is: u8, a: char, struct_: u64, variant: u32},
    }

The value comes from:

    let e = MoreComplicated::Two(73);

So, if we have the dynamic type, "dynamic" is True and all the fields
are available (not shown in the test).  If we have the value "e", only
the applicable fields for the variant are available -- the
compiler-provided discriminant, and the "Two" payload.

Tom
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 10/11] Add Python support for dynamic types

Sourceware - gdb-patches mailing list
On Thu, Apr 9, 2020 at 1:44 PM Tom Tromey <[hidden email]> wrote:

>
> >>>>> "Christian" == Christian Biesinger via Gdb-patches <[hidden email]> writes:
>
> >> This attribute is not available for @code{enum} or @code{static}
> >> (as in C@t{++}) fields.  The value is the position, counting
> >> -in bits, from the start of the containing type.
> >> +in bits, from the start of the containing type.  Note that, in a
> >> +dynamic type, the position of a field may not be constant.  In this
> >> +case, the value will be @code{None}.
>
> Christian> Just to make sure I understand, if I have a type like this (using
> Christian> Haskell-like syntax, I hope this makes sense):
>
> Christian> Type t =
> Christian>   Int x |
> Christian>   Float y |
> Christian>   (String a, Float b)
>
> Christian> This would be reflected in Python as the type having fields x, y, a
> Christian> and b, all of which have a None position?
>
> IIUC this would have field x, y, and a field with an anonymous structure
> type, that in turn has fields a and b.  But otherwise yes.
>
> Christian> Is there a way to access these fields, and is there a way to see which
> Christian> specific type a certain variable value is?
>
> Yes, the type of a value will be a concrete instance of such a dynamic
> type.
>
> The Rust test is similar:
>
> >> +gdb_test "python print(len(e.type.fields()))" "2"
> >> +gdb_test "python print(e.type.fields()\[0\].artificial)" "True"
> >> +gdb_test "python print(e.type.fields()\[1\].name)" "Two"
> >> +
> >> +gdb_test "python print(e.type.dynamic)" "False"
> >> +gdb_test "python print(gdb.lookup_type('simple::MoreComplicated').dynamic)" \
> >> +    "True"
>
> This is inspecting a dynamic type "MoreComplicated" and a value of that
> type, "e".
>
> MoreComplicated is an enum, which is the Rust equivalent of your Haskell
> example.
>
>     enum MoreComplicated {
>         One,
>         Two(i32),
>         Three(HiBob),
>         Four{this: bool, is: u8, a: char, struct_: u64, variant: u32},
>     }
>
> The value comes from:
>
>     let e = MoreComplicated::Two(73);
>
> So, if we have the dynamic type, "dynamic" is True and all the fields
> are available (not shown in the test).  If we have the value "e", only
> the applicable fields for the variant are available -- the
> compiler-provided discriminant, and the "Two" payload.

Thanks for the detailed explanation! I would suggest two things:
- For your note in the documentation that "the position of a field may
not be constant", maybe add that the field may not exist in a specific
value, and
- mention somewhere that value.type will be different from
gdb.lookup_type(value.type.name), and lets you access the concrete
fields

The "dynamic type" name is unfortunate, since it is unrelated to
Value.dynamic_type AFAICT. I thought discriminated/tagged union was a
more common name for this :(

Thanks,
Christian
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 0/11] Variant part support, plus more

Tom Tromey-4
In reply to this post by Tom Tromey-4
>>>>> "Tom" == Tom Tromey <[hidden email]> writes:

Tom> Currently, the Ada support in gdb relies on what are called "gnat
Tom> encodings" -- which really just means that some debug info is
Tom> represented using magic symbol names, attached to other debug objects.
Tom> A long term goal here is to slowly remove gnat encodings, in favor of
Tom> using standard DWARF.

Tom> This series is one step toward that goal.  It adds support for DWARF
Tom> variant parts to gdb.  It also brings in support for fields that have
Tom> dynamic offsets, and types that have dynamic lengths.  It also adds a
Tom> Python API to let pretty-printers know a bit more about dynamic types.

I'm going to check this in now.

Tom
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 10/11] Add Python support for dynamic types

Tom Tromey-2
In reply to this post by Sourceware - gdb-patches mailing list
>>>>> "Christian" == Christian Biesinger via Gdb-patches <[hidden email]> writes:

Christian> Thanks for the detailed explanation! I would suggest two things:
Christian> - For your note in the documentation that "the position of a field may
Christian> not be constant", maybe add that the field may not exist in a specific
Christian> value, and
Christian> - mention somewhere that value.type will be different from
Christian> gdb.lookup_type(value.type.name), and lets you access the concrete
Christian> fields

I'll write a new doc patch for this.

Christian> The "dynamic type" name is unfortunate, since it is unrelated to
Christian> Value.dynamic_type AFAICT. I thought discriminated/tagged union was a
Christian> more common name for this :(

Yeah, though this code doesn't really only apply to variant parts /
discriminated unions.  It applies to any type that might have some kind
of dynamically-resolved attribute.  For example, this can be used by
structures that contain dynamically-sized arrays.

Tom
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 10/11] Add Python support for dynamic types

Tom Tromey-2
In reply to this post by Sourceware - gdb-patches mailing list
>>>>> "Christian" == Christian Biesinger via Gdb-patches <[hidden email]> writes:

Christian> The "dynamic type" name is unfortunate, since it is unrelated to
Christian> Value.dynamic_type AFAICT. I thought discriminated/tagged union was a
Christian> more common name for this :(

When I sent my earlier note, this hadn't fully sunk in I guess.  Maybe
we ought to rename the "dynamic" type business.  It isn't too late, even
though that's checked in.

What do you think would be better?

Tom
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 10/11] Add Python support for dynamic types

Sourceware - gdb-patches mailing list
On Fri, Apr 24, 2020 at 8:38 PM Tom Tromey <[hidden email]> wrote:

>
> >>>>> "Christian" == Christian Biesinger via Gdb-patches <[hidden email]> writes:
>
> Christian> The "dynamic type" name is unfortunate, since it is unrelated to
> Christian> Value.dynamic_type AFAICT. I thought discriminated/tagged union was a
> Christian> more common name for this :(
>
> When I sent my earlier note, this hadn't fully sunk in I guess.  Maybe
> we ought to rename the "dynamic" type business.  It isn't too late, even
> though that's checked in.
>
> What do you think would be better?
>
> Tom

I know that Harper has used the terminology "Extensible", and the term
"Index", is commonly the generalization of Tag.
first paragraph from the paper by Licata & Harper, An Extensible
Theory of Indexed Types
http://www.cs.cmu.edu/~rwh/papers/extidx/paper.pdf

"Indexed families of types are a way of associating run-time data with
compile-time abstractions that can be used to reason about them. We
propose an extensible theory of indexed types, in which programmers
can define the index data appropriate to their programs and use them
to track properties of run-time code."

Extensible seems alright to me for the bool, in that the concrete data
obtained from the type is extensible beyond that what is statically
known about the type?

shrug
12