Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [patch] Fix DW_AT_lower_bound DWARF-4+ defaults
@ 2012-04-26 15:08 Jan Kratochvil
  2012-04-26 16:09 ` Tom Tromey
  0 siblings, 1 reply; 6+ messages in thread
From: Jan Kratochvil @ 2012-04-26 15:08 UTC (permalink / raw)
  To: gdb-patches

Hi,

with GCC defaults changed to DWARF-4 today
	http://gcc.gnu.org/viewcvs?view=revision&revision=186835

I have found GDB has the defaults for DW_AT_lower_bound in DWARF-4+
wrong/unimplemented.

As it is a large Ada regression I plan to check it in soon, it is also mostly
[obv]ious patch.

No regressions for DWARF 2, 3, 4 (4 both with and without .debug_types) on
{x86_64,x86_64-m32,i686}-fedora17-linux-gnu and also for x86_64,x86_64-m32
with gcc (GCC) 4.8.0 20120426 (experimental).


Thanks,
Jan


-FAIL: gdb.ada/array_return.exp: value printed by finish of Create_Small
+PASS: gdb.ada/array_return.exp: value printed by finish of Create_Small
-FAIL: gdb.ada/array_return.exp: value printed by finish of Create_Large
+PASS: gdb.ada/array_return.exp: value printed by finish of Create_Large
-FAIL: gdb.ada/array_return.exp: value printed by finish of Create_Small_Float_Vector
+PASS: gdb.ada/array_return.exp: value printed by finish of Create_Small_Float_Vector
-FAIL: gdb.ada/arrayidx.exp: print one_two_three, indexes off
+PASS: gdb.ada/arrayidx.exp: print one_two_three, indexes off
-FAIL: gdb.ada/arrayidx.exp: print few_reps, indexes off
-FAIL: gdb.ada/arrayidx.exp: print many_reps, indexes off
-FAIL: gdb.ada/arrayidx.exp: print empty, indexes off
+PASS: gdb.ada/arrayidx.exp: print few_reps, indexes off
+PASS: gdb.ada/arrayidx.exp: print many_reps, indexes off
+PASS: gdb.ada/arrayidx.exp: print empty, indexes off
-FAIL: gdb.ada/arrayidx.exp: print one_two_three
+PASS: gdb.ada/arrayidx.exp: print one_two_three
-FAIL: gdb.ada/arrayidx.exp: print few_reps
-FAIL: gdb.ada/arrayidx.exp: print many_reps
-FAIL: gdb.ada/arrayidx.exp: print empty
+PASS: gdb.ada/arrayidx.exp: print few_reps
+PASS: gdb.ada/arrayidx.exp: print many_reps
+PASS: gdb.ada/arrayidx.exp: print empty
-FAIL: gdb.ada/arrayptr.exp: print arr_ptr (2)
-FAIL: gdb.ada/arrayptr.exp: print arr_ptr (3..4)
+PASS: gdb.ada/arrayptr.exp: print arr_ptr (2)
+PASS: gdb.ada/arrayptr.exp: print arr_ptr (3..4)
-FAIL: gdb.ada/homonym.exp: ptype local_type at BREAK_2
-FAIL: gdb.ada/homonym.exp: ptype local_type_subtype at BREAK_2
-FAIL: gdb.ada/homonym.exp: ptype lcl at BREAK_2
+PASS: gdb.ada/homonym.exp: ptype local_type at BREAK_2
+PASS: gdb.ada/homonym.exp: ptype local_type_subtype at BREAK_2
+PASS: gdb.ada/homonym.exp: ptype lcl at BREAK_2
-FAIL: gdb.ada/mod_from_name.exp: print xp
+PASS: gdb.ada/mod_from_name.exp: print xp
-FAIL: gdb.ada/null_array.exp: print my_matrix
+PASS: gdb.ada/null_array.exp: print my_matrix
-FAIL: gdb.ada/ptr_typedef.exp: print my_blob.bees(1)
+PASS: gdb.ada/ptr_typedef.exp: print my_blob.bees(1)
-FAIL: gdb.ada/rec_return.exp: print bar
+PASS: gdb.ada/rec_return.exp: print bar
-FAIL: gdb.ada/set_pckd_arr_elt.exp: continue to update_small
+PASS: gdb.ada/set_pckd_arr_elt.exp: continue to update_small
-FAIL: gdb.ada/str_ref_cmp.exp: print String_Var (1 .. 3) = "Hel"
+PASS: gdb.ada/str_ref_cmp.exp: print String_Var (1 .. 3) = "Hel"
-FAIL: gdb.ada/type_coercion.exp: p q
+PASS: gdb.ada/type_coercion.exp: p q
-FAIL: gdb.ada/type_coercion.exp: p q
+PASS: gdb.ada/type_coercion.exp: p q
-FAIL: gdb.ada/widewide.exp: print my_wws
-FAIL: gdb.ada/widewide.exp: print my_wws(1)
-FAIL: gdb.ada/widewide.exp: print my_wws(2)
+PASS: gdb.ada/widewide.exp: print my_wws
+PASS: gdb.ada/widewide.exp: print my_wws(1)
+PASS: gdb.ada/widewide.exp: print my_wws(2)


gdb/
2012-04-26  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Fix DW_AT_lower_bound defaults for DWARF-4+.
	* dwarf2read.c (read_subrange_type): Remove initialization of low and
	high.  Implement DWARF-4+ DW_AT_lower_bound defaults.  Print complaint
	for DW_AT_lower_bound with no default by the DWARF standard.

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index bd2d6f2..cfd12e5 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -9086,8 +9086,7 @@ read_subrange_type (struct die_info *die, struct dwarf2_cu *cu)
   struct type *base_type;
   struct type *range_type;
   struct attribute *attr;
-  LONGEST low = 0;
-  LONGEST high = -1;
+  LONGEST low, high;
   char *name;
   LONGEST negative_mask;
 
@@ -9100,18 +9099,47 @@ read_subrange_type (struct die_info *die, struct dwarf2_cu *cu)
   if (range_type)
     return range_type;
 
-  if (cu->language == language_fortran)
-    {
-      /* FORTRAN implies a lower bound of 1, if not given.  */
-      low = 1;
-    }
-
   /* FIXME: For variable sized arrays either of these could be
      a variable rather than a constant value.  We'll allow it,
      but we don't know how to handle it.  */
   attr = dwarf2_attr (die, DW_AT_lower_bound, cu);
   if (attr)
     low = dwarf2_get_attr_constant_value (attr, 0);
+  else
+    {
+      low = -1;
+      switch (cu->language)
+	{
+	case language_c:
+	case language_cplus:
+	  low = 0;
+	  break;
+	case language_fortran:
+	  low = 1;
+	  break;
+	case language_d:
+	case language_java:
+	case language_objc:
+	  if (cu->header.version >= 4)
+	    low = 0;
+	  break;
+	case language_ada:
+	case language_m2:
+	case language_pascal:
+	  if (cu->header.version >= 4)
+	    low = 1;
+	  break;
+	default:
+	  break;
+	}
+      if (low == -1)
+	{
+	  complaint (&symfile_complaints, _("Missing DW_AT_lower_bound "
+					    "- DIE at 0x%x [in module %s]"),
+		     die->offset.sect_off, cu->objfile->name);
+	  low = 0;
+	}
+    }
 
   attr = dwarf2_attr (die, DW_AT_upper_bound, cu);
   if (attr)


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [patch] Fix DW_AT_lower_bound DWARF-4+ defaults
  2012-04-26 16:09 ` Tom Tromey
@ 2012-04-26 15:39   ` Jan Kratochvil
  2012-04-26 15:55     ` Tom Tromey
  0 siblings, 1 reply; 6+ messages in thread
From: Jan Kratochvil @ 2012-04-26 15:39 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On Thu, 26 Apr 2012 17:29:36 +0200, Tom Tromey wrote:
> I looked at the DWARF 3 spec and it says:
> 
>     If the lower bound value is missing, the value is assumed to be a
>     language-dependent default constant. The default lower bound value for C
>     or C++ is 0. For Fortran, it is 1.
> 
> So it seems wrong to complain here;

All of DWARF 2, 3 and 4 have there also the paragraph:
	No other default lower bound values are currently defined.

Which makes the complaint IMNSHO right.


> defaults is informative, not exhaustive, and we should just pick a
> reasonable default and not complain.

Some reasonable default is picked, maybe we could discuss if this fallback
could be improved but I find 0 to be good enough for invalid DWARF input.


Thanks,
Jan


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [patch] Fix DW_AT_lower_bound DWARF-4+ defaults
  2012-04-26 15:39   ` Jan Kratochvil
@ 2012-04-26 15:55     ` Tom Tromey
  2012-04-26 16:16       ` Jan Kratochvil
  0 siblings, 1 reply; 6+ messages in thread
From: Tom Tromey @ 2012-04-26 15:55 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: gdb-patches

Jan> All of DWARF 2, 3 and 4 have there also the paragraph:
Jan> 	No other default lower bound values are currently defined.
Jan> Which makes the complaint IMNSHO right.

Crud, I missed that somehow.
I agree.

>> defaults is informative, not exhaustive, and we should just pick a
>> reasonable default and not complain.

Jan> Some reasonable default is picked, maybe we could discuss if this fallback
Jan> could be improved but I find 0 to be good enough for invalid DWARF input.

I think the nicest thing would be to pick the DWARF 4 defaults, and
complain for other versions.

Tom


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [patch] Fix DW_AT_lower_bound DWARF-4+ defaults
  2012-04-26 15:08 [patch] Fix DW_AT_lower_bound DWARF-4+ defaults Jan Kratochvil
@ 2012-04-26 16:09 ` Tom Tromey
  2012-04-26 15:39   ` Jan Kratochvil
  0 siblings, 1 reply; 6+ messages in thread
From: Tom Tromey @ 2012-04-26 16:09 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: gdb-patches

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Jan> +	case language_d:
Jan> +	case language_java:
Jan> +	case language_objc:
Jan> +	  if (cu->header.version >= 4)
Jan> +	    low = 0;
Jan> +	  break;

I looked at the DWARF 3 spec and it says:

    If the lower bound value is missing, the value is assumed to be a
    language-dependent default constant. The default lower bound value for C
    or C++ is 0. For Fortran, it is 1.

So it seems wrong to complain here; at least for Java and ObjC, where
there is only a single sensible default.  I can't speak for the other
languages, but even there I think the DWARF text mentioning specific
defaults is informative, not exhaustive, and we should just pick a
reasonable default and not complain.

Tom


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [patch] Fix DW_AT_lower_bound DWARF-4+ defaults
  2012-04-26 15:55     ` Tom Tromey
@ 2012-04-26 16:16       ` Jan Kratochvil
  2012-04-26 20:35         ` [commit] " Jan Kratochvil
  0 siblings, 1 reply; 6+ messages in thread
From: Jan Kratochvil @ 2012-04-26 16:16 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On Thu, 26 Apr 2012 17:43:34 +0200, Tom Tromey wrote:
> I think the nicest thing would be to pick the DWARF 4 defaults, and
> complain for other versions.

OK.


Thanks,
Jan


gdb/
2012-04-26  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Fix DW_AT_lower_bound defaults for DWARF-4+.
	* dwarf2read.c (read_subrange_type): Remove initialization of low and
	high.  New variable low_default_is_valid.  Implement DWARF-4+
	DW_AT_lower_bound defaults.  Print complaint for DW_AT_lower_bound with
	no default by the DWARF standard.

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index bd2d6f2..99bba9f 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -9086,8 +9086,8 @@ read_subrange_type (struct die_info *die, struct dwarf2_cu *cu)
   struct type *base_type;
   struct type *range_type;
   struct attribute *attr;
-  LONGEST low = 0;
-  LONGEST high = -1;
+  LONGEST low, high;
+  int low_default_is_valid;
   char *name;
   LONGEST negative_mask;
 
@@ -9100,10 +9100,35 @@ read_subrange_type (struct die_info *die, struct dwarf2_cu *cu)
   if (range_type)
     return range_type;
 
-  if (cu->language == language_fortran)
+  /* Set LOW_DEFAULT_IS_VALID if current language and DWARF version allow
+     omitting DW_AT_lower_bound.  */
+  switch (cu->language)
     {
-      /* FORTRAN implies a lower bound of 1, if not given.  */
+    case language_c:
+    case language_cplus:
+      low = 0;
+      low_default_is_valid = 1;
+      break;
+    case language_fortran:
+      low = 1;
+      low_default_is_valid = 1;
+      break;
+    case language_d:
+    case language_java:
+    case language_objc:
+      low = 0;
+      low_default_is_valid = (cu->header.version >= 4);
+      break;
+    case language_ada:
+    case language_m2:
+    case language_pascal:
       low = 1;
+      low_default_is_valid = (cu->header.version >= 4);
+      break;
+    default:
+      low = 0;
+      low_default_is_valid = 0;
+      break;
     }
 
   /* FIXME: For variable sized arrays either of these could be
@@ -9111,7 +9136,11 @@ read_subrange_type (struct die_info *die, struct dwarf2_cu *cu)
      but we don't know how to handle it.  */
   attr = dwarf2_attr (die, DW_AT_lower_bound, cu);
   if (attr)
-    low = dwarf2_get_attr_constant_value (attr, 0);
+    low = dwarf2_get_attr_constant_value (attr, low);
+  else if (!low_default_is_valid)
+    complaint (&symfile_complaints, _("Missing DW_AT_lower_bound "
+				      "- DIE at 0x%x [in module %s]"),
+	       die->offset.sect_off, cu->objfile->name);
 
   attr = dwarf2_attr (die, DW_AT_upper_bound, cu);
   if (attr)


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [commit] [patch] Fix DW_AT_lower_bound DWARF-4+ defaults
  2012-04-26 16:16       ` Jan Kratochvil
@ 2012-04-26 20:35         ` Jan Kratochvil
  0 siblings, 0 replies; 6+ messages in thread
From: Jan Kratochvil @ 2012-04-26 20:35 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On Thu, 26 Apr 2012 18:09:52 +0200, Jan Kratochvil wrote:
> gdb/
> 2012-04-26  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	Fix DW_AT_lower_bound defaults for DWARF-4+.
> 	* dwarf2read.c (read_subrange_type): Remove initialization of low and
> 	high.  New variable low_default_is_valid.  Implement DWARF-4+
> 	DW_AT_lower_bound defaults.  Print complaint for DW_AT_lower_bound with
> 	no default by the DWARF standard.

Checked it in to fix the testsuite results regressed by GCC:
	http://sourceware.org/ml/gdb-cvs/2012-04/msg00219.html


Thanks,
Jan


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2012-04-26 20:33 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-04-26 15:08 [patch] Fix DW_AT_lower_bound DWARF-4+ defaults Jan Kratochvil
2012-04-26 16:09 ` Tom Tromey
2012-04-26 15:39   ` Jan Kratochvil
2012-04-26 15:55     ` Tom Tromey
2012-04-26 16:16       ` Jan Kratochvil
2012-04-26 20:35         ` [commit] " Jan Kratochvil

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox