Re: [RFA] dwarf2read.c: Avoid complaint for char array of unspecified size

Re: [RFA] dwarf2read.c: Avoid complaint for char array of unspecified size

[Tom Tromey]

>>>>> "Pierre" == Pierre Muller <pierre.muller@ics-cnrs.unistra.fr> writes:

Pierre>   This code:
Pierre> external char gdbint [];
Pierre> generated a DW_TAG_subrange_type with zero attributes
Pierre> to describe the `[]' part.
Pierre>  (compiled with  gcc (Debian 4.3.2-1.1) 4.3.2.).

I think this kind of DIE is explicitly ok, according to DWARF.
See the paragraph toward the end of section 5.11 that describes what to
do when a DW_TAG_subrange_type has no basis type.

So, I think this complaint is incorrect.  It would be better to simply
remove it.

Tom

RE: [RFA] dwarf2read.c: Avoid complaint for char array of unspecified size

[Pierre Muller]

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Tom Tromey
> Envoyé : Friday, May 21, 2010 11:43 PM
> À : Pierre Muller
> Cc : gdb-patches@sourceware.org
> Objet : Re: [RFA] dwarf2read.c: Avoid complaint for char array of
> unspecified size
> 
> >>>>> "Pierre" == Pierre Muller <pierre.muller@ics-cnrs.unistra.fr>
> writes:
> 
> Pierre>   This code:
> Pierre> external char gdbint [];
> Pierre> generated a DW_TAG_subrange_type with zero attributes
> Pierre> to describe the `[]' part.
> Pierre>  (compiled with  gcc (Debian 4.3.2-1.1) 4.3.2.).
> 
> I think this kind of DIE is explicitly ok, according to DWARF.
> See the paragraph toward the end of section 5.11 that describes what to
> do when a DW_TAG_subrange_type has no basis type.
 For dwarf2, in it on 5.10, page number 47 (49 of pdf file).
  If I understand this correctly, without anything it should be
a subrange of type "singed integer" having the same size as an address
for that target machine. This is not exactly what is done in the Irix
code I modified...
  But I still don't get the bounds that we should use:
  lower bound is 0, or 1 for fortran (default value),
but should upper bound be equal to lower bound or lower bound -1?

> So, I think this complaint is incorrect.  It would be better to simply
> remove it.
  This would mean that we should fix the code inside read_subrange_type, no?

  Do you think that this would be the correct approach?

Pierre

Re: [RFA] dwarf2read.c: Avoid complaint for char array of unspecified size

[Tom Tromey]

>>>>> "Pierre" == Pierre Muller <pierre.muller@ics-cnrs.unistra.fr> writes:

Pierre>   But I still don't get the bounds that we should use:
Pierre>   lower bound is 0, or 1 for fortran (default value),
Pierre> but should upper bound be equal to lower bound or lower bound -1?

Whatever means "unknown bounds" inside gdb.
I don't remember offhand what this is.

Pierre>   This would mean that we should fix the code inside
Pierre>   read_subrange_type, no?

Pierre>   Do you think that this would be the correct approach?

Yeah, I do.

Tom

[RFA-v2] dwarf2read.c: Avoid complaint for char array of unspecified size

[Pierre Muller]

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Tom Tromey
> Envoyé : Monday, May 24, 2010 5:30 PM
> À : Pierre Muller
> Cc : gdb-patches@sourceware.org
> Objet : Re: [RFA] dwarf2read.c: Avoid complaint for char array of
> unspecified size
> 
> >>>>> "Pierre" == Pierre Muller <pierre.muller@ics-cnrs.unistra.fr>
> writes:
> 
> Pierre>   But I still don't get the bounds that we should use:
> Pierre>   lower bound is 0, or 1 for fortran (default value),
> Pierre> but should upper bound be equal to lower bound or lower bound -
> 1?
> 
> Whatever means "unknown bounds" inside gdb.
> I don't remember offhand what this is.
> 
> Pierre>   This would mean that we should fix the code inside
> Pierre>   read_subrange_type, no?
> 
> Pierre>   Do you think that this would be the correct approach?
> 
> Yeah, I do.
> 
> Tom


2010-05-26  Pierre Muller  <muller@ics.u-strasbg.fr>

	* dwarf2read.c (read_subrange_type): Create default subrange
	type if DIE->NUM_ATTRS is zero.

Index: src/gdb/dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.388
diff -u -p -r1.388 dwarf2read.c
--- src/gdb/dwarf2read.c	21 May 2010 20:45:19 -0000	1.388
+++ src/gdb/dwarf2read.c	26 May 2010 08:41:36 -0000
@@ -6129,7 +6129,20 @@ read_subrange_type (struct die_info *die
   LONGEST high = -1;
   char *name;
   LONGEST negative_mask;
-  
+
+  /* Dwarf-2 specifications explicitly allows to create subrange types
+     without speciying anything, no base_type, no lower and no upper
+     bound.  This kind of empty subrange die is created for instance by GCC
+     for the following C code:
+     extern char gdb_int [];
+  */
+  if (die->num_attrs == 0)
+    {
+      base_type = objfile_type (cu->objfile)->builtin_int;
+      range_type = create_range_type (NULL, base_type, 0, -1); 
+      return set_die_type (die, range_type, cu);
+    }
+
   base_type = die_type (die, cu);
   if (TYPE_CODE (base_type) == TYPE_CODE_VOID)
     {

Re: [RFA-v2] dwarf2read.c: Avoid complaint for char array of unspecified size

[Tom Tromey]

>>>>> "Pierre" == Pierre Muller <pierre.muller@ics-cnrs.unistra.fr> writes:

Pierre> 2010-05-26  Pierre Muller  <muller@ics.u-strasbg.fr>
Pierre> 	* dwarf2read.c (read_subrange_type): Create default subrange
Pierre> 	type if DIE->NUM_ATTRS is zero.

I think this is an improvement, but it would be preferable to just
directly implement what DWARF specifies:

    If the subrange entry has no type attribute describing the basis
    type, the basis type is assumed to be the same as the object
    described by the lower bound attribute (if it references an
    object). If there is no lower bound attribute, or that attribute
    does not reference an object, the basis type is the type of the
    upper bound or count attribute (if either of them references an
    object). If there is no upper bound or count attribute, or neither
    references an object, the type is assumed to be the same type, in
    the source language of the compilation unit containing the subrange
    entry, as a signed integer with the same size as an address on the
    target machine.

Pierre> +  if (die->num_attrs == 0)

A DW_TAG_subrange_type might have attributes that we ignore.  So, I
think there is no need for a check that is this specific; just determine
the type according to the above algorithm, and remove the existing
complaint code.

Tom

[RFA-v3] dwarf2read.c: Avoid complaint for char array of unspecified size

[Pierre Muller]

> Pierre> +  if (die->num_attrs == 0)
> 
> A DW_TAG_subrange_type might have attributes that we ignore.  So, I
> think there is no need for a check that is this specific; just
> determine
> the type according to the above algorithm, and remove the existing
> complaint code.
> 
> Tom

  OK, I tried to follow that route,
the tricky part was that we don't really know which objfile type
is the same size as an address, but the patch is a bit lengthy...
  At the same time, I discovered that DW_AT_count was not supported,
(maybe no compiler uses that, but it can't hurt to add
support for this).


Pierre

2010-05-28  Pierre Muller  <muller@ics.u-strasbg.fr>

	* dwarf2read.c (read_subrange_type): Handle missing base type
	according to Dwarf-2 specifications.
	Also handle DW_AT_count attribute.

Index: src/gdb/dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.388
diff -u -p -r1.388 dwarf2read.c
--- src/gdb/dwarf2read.c	21 May 2010 20:45:19 -0000	1.388
+++ src/gdb/dwarf2read.c	28 May 2010 08:39:58 -0000
@@ -6129,16 +6131,8 @@ read_subrange_type (struct die_info *die
   LONGEST high = -1;
   char *name;
   LONGEST negative_mask;
-  
+
   base_type = die_type (die, cu);
-  if (TYPE_CODE (base_type) == TYPE_CODE_VOID)
-    {
-      complaint (&symfile_complaints,
-                _("DW_AT_type missing from DW_TAG_subrange_type"));
-      base_type
-	= init_type (TYPE_CODE_INT, gdbarch_addr_bit (gdbarch) / 8,
-		     0, NULL, cu->objfile);
-    }
 
   if (cu->language == language_fortran)
     { 
@@ -6156,10 +6150,10 @@ read_subrange_type (struct die_info *die
   attr = dwarf2_attr (die, DW_AT_upper_bound, cu);
   if (attr)
     {       
-      if (attr->form == DW_FORM_block1)
+      if (attr->form == DW_FORM_block1 || is_ref_attr (attr))
         {
           /* GCC encodes arrays with unspecified or dynamic length
-             with a DW_FORM_block1 attribute.
+             with a DW_FORM_block1 attribute or a reference attribute.
              FIXME: GDB does not yet know how to handle dynamic
              arrays properly, treat them as arrays with unspecified
              length for now.
@@ -6174,6 +6168,48 @@ read_subrange_type (struct die_info *die
       else
         high = dwarf2_get_attr_constant_value (attr, 1);
     }
+  else
+    {
+      attr = dwarf2_attr (die, DW_AT_count, cu);
+      if (attr)
+	{
+	  int count = dwarf2_get_attr_constant_value (attr, 1);
+	  high = low + count - 1;
+	}
+    }
+
+  /* Dwarf-2 specifications explicitly allows to create subrange types
+     without specifying a base type.
+     In that case, the base type must be set to the type of
+     the lower bound, upper bound or count, in that order, if any of these
+     three attributes references an object that has a type.
+     If no base type is found, the Dwarf-2 specifications say that
+     a signed integer type of size equal to the size of an address should
+     be used.
+     For the following C code: `extern char gdb_int [];'
+     GCC produces an empty range DIE.
+     FIXME: muller/2010-05-28: Possible references to object for low bound,
+     high bound or count are not yet handled by this code.
+  */
+  if (TYPE_CODE (base_type) == TYPE_CODE_VOID)
+    {
+      struct objfile *objfile = cu->objfile;
+      struct gdbarch *gdbarch = get_objfile_arch (objfile);
+      int addr_size = gdbarch_addr_bit (gdbarch) /8;
+      struct type *int_type = objfile_type (objfile)->builtin_long_long;
+
+      /* Test "long long int", "long int", and "int" objfile types,
+	 and select the last one having a size above or equal to the
+	 architecture address size.  */
+      if (int_type && TYPE_LENGTH (int_type) >= addr_size)
+	base_type = int_type;
+      int_type = objfile_type (objfile)->builtin_long;
+      if (int_type && TYPE_LENGTH (int_type) >= addr_size)
+	base_type = int_type;
+      int_type = objfile_type (objfile)->builtin_int;
+      if (int_type && TYPE_LENGTH (int_type) >= addr_size)
+	base_type = int_type;
+    }
 
   negative_mask = 
     (LONGEST) -1 << (TYPE_LENGTH (base_type) * TARGET_CHAR_BIT - 1);

Re: [RFA-v3] dwarf2read.c: Avoid complaint for char array of unspecified size

[Tom Tromey]

>>>>> "Pierre" == Pierre Muller <pierre.muller@ics-cnrs.unistra.fr> writes:

Pierre>   OK, I tried to follow that route,
Pierre> the tricky part was that we don't really know which objfile type
Pierre> is the same size as an address, but the patch is a bit lengthy...

Yeah, I don't know this either.  Your approach seems reasonable.

Pierre>   At the same time, I discovered that DW_AT_count was not supported,
Pierre> (maybe no compiler uses that, but it can't hurt to add
Pierre> support for this).

Thanks for doing this.

Pierre> +      /* Test "long long int", "long int", and "int" objfile types,
Pierre> +	 and select the last one having a size above or equal to the
Pierre> +	 architecture address size.  */

I think you should test these in the reverse order: int, then long, then
long long.  This assures you will find the smallest type that matches.

This patch is ok with that change.

Tom

RE: [RFA-v3] dwarf2read.c: Avoid complaint for char array of unspecified size

[Pierre Muller]

> Pierre> +      /* Test "long long int", "long int", and "int" objfile
> types,
> Pierre> +	 and select the last one having a size above or equal to
> the
> Pierre> +	 architecture address size.  */
> 
> I think you should test these in the reverse order: int, then long,
> then
> long long.  This assures you will find the smallest type that matches.
> 
> This patch is ok with that change.

  Thanks for the approval,
I modified as you suggested, even if I don't think that it makes
any difference in the chosen integer type...

Pierre


For the record, here is what I checked in rev 1.390 of dwarf2-read.c:


ChangeLog entry:

2010-06-02  Pierre Muller  <muller@ics.u-strasbg.fr>

	* dwarf2read.c (read_subrange_type): Handle missing base type
	according to Dwarf-2 specifications.

Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.389
diff -u -p -r1.389 dwarf2read.c
--- dwarf2read.c	1 Jun 2010 21:34:15 -0000	1.389
+++ dwarf2read.c	2 Jun 2010 06:19:23 -0000
@@ -6131,16 +6131,8 @@ read_subrange_type (struct die_info *die
   LONGEST high = -1;
   char *name;
   LONGEST negative_mask;
-  
+
   base_type = die_type (die, cu);
-  if (TYPE_CODE (base_type) == TYPE_CODE_VOID)
-    {
-      complaint (&symfile_complaints,
-                _("DW_AT_type missing from DW_TAG_subrange_type"));
-      base_type
-	= init_type (TYPE_CODE_INT, gdbarch_addr_bit (gdbarch) / 8,
-		     0, NULL, cu->objfile);
-    }
 
   if (cu->language == language_fortran)
     { 
@@ -6158,10 +6150,10 @@ read_subrange_type (struct die_info *die
   attr = dwarf2_attr (die, DW_AT_upper_bound, cu);
   if (attr)
     {       
-      if (attr->form == DW_FORM_block1)
+      if (attr->form == DW_FORM_block1 || is_ref_attr (attr))
         {
           /* GCC encodes arrays with unspecified or dynamic length
-             with a DW_FORM_block1 attribute.
+             with a DW_FORM_block1 attribute or a reference attribute.
              FIXME: GDB does not yet know how to handle dynamic
              arrays properly, treat them as arrays with unspecified
              length for now.
@@ -6176,6 +6168,54 @@ read_subrange_type (struct die_info *die
       else
         high = dwarf2_get_attr_constant_value (attr, 1);
     }
+  else
+    {
+      attr = dwarf2_attr (die, DW_AT_count, cu);
+      if (attr)
+	{
+	  int count = dwarf2_get_attr_constant_value (attr, 1);
+	  high = low + count - 1;
+	}
+    }
+
+  /* Dwarf-2 specifications explicitly allows to create subrange types
+     without specifying a base type.
+     In that case, the base type must be set to the type of
+     the lower bound, upper bound or count, in that order, if any of these
+     three attributes references an object that has a type.
+     If no base type is found, the Dwarf-2 specifications say that
+     a signed integer type of size equal to the size of an address should
+     be used.
+     For the following C code: `extern char gdb_int [];'
+     GCC produces an empty range DIE.
+     FIXME: muller/2010-05-28: Possible references to object for low bound,
+     high bound or count are not yet handled by this code.
+  */
+  if (TYPE_CODE (base_type) == TYPE_CODE_VOID)
+    {
+      struct objfile *objfile = cu->objfile;
+      struct gdbarch *gdbarch = get_objfile_arch (objfile);
+      int addr_size = gdbarch_addr_bit (gdbarch) /8;
+      struct type *int_type = objfile_type (objfile)->builtin_int;
+
+      /* Test "int", "long int", and "long long int" objfile types,
+	 and select the first one having a size above or equal to the
+	 architecture address size.  */
+      if (int_type && TYPE_LENGTH (int_type) >= addr_size)
+	base_type = int_type;
+      else
+	{
+	  int_type = objfile_type (objfile)->builtin_long;
+	  if (int_type && TYPE_LENGTH (int_type) >= addr_size)
+	    base_type = int_type;
+	  else
+	    {
+	      int_type = objfile_type (objfile)->builtin_long_long;
+	      if (int_type && TYPE_LENGTH (int_type) >= addr_size)
+		base_type = int_type;
+	    }
+	}
+    }
 
   negative_mask = 
     (LONGEST) -1 << (TYPE_LENGTH (base_type) * TARGET_CHAR_BIT - 1);

Re: [RFA-v3] dwarf2read.c: Avoid complaint for char array of unspecified size

[Tom Tromey]

>>>>> "Pierre" == Pierre Muller <pierre.muller@ics-cnrs.unistra.fr> writes:

Pierre>   Thanks for the approval,
Pierre> I modified as you suggested, even if I don't think that it makes
Pierre> any difference in the chosen integer type...

Yeah, I think I misunderstood the code, sorry about that.

Tom