RFC: fix another regression w/ gcc svn trunk

RFC: fix another regression w/ gcc svn trunk

[Tom Tromey]

I would appreciate comments on this patch.

GCC svn trunk apparently emits DW_FORM_data1 a little more aggressively
now.  This caused a regression in scope.exp:

    print foo::funclocal_ro
    $15 = -53
    (gdb) FAIL: gdb.base/scope.exp: print foo::funclocal_ro

The correct answer is 203, but gdb sign-extends this to get -53.

Both GCC and elfutils agreed that DW_FORM_data* are "just bits" -- no
sign extension is to be applied.  This is not clear from the DWARF
specification, so I asked on the DWARF list, and the answer there was
that this is unspecified.  (I would link to the list archives here, but
they are closed to non-subscribers -- and if you are a subscriber you
already saw or have the thread.)

Since GCC has apparently been assuming its interpretation for some time,
I think it is best to simply follow GCC.  That is what this patch does.

Built and regtested on x86-64 (compile farm).  Also I tested this test
case locally using gcc svn.  I plan to run the whole gdb test suite
again with this fix in place.

In the absence of comments I am going to check this in.
I am undecided as to whether this belongs in 7.2.

Tom

2010-07-27  Tom Tromey  <tromey@redhat.com>

	* dwarf2read.c (dwarf2_const_value_data): Never sign extend.

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 0d63fe3..de516dc 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -10157,8 +10157,8 @@ dwarf2_const_value (struct attribute *attr, struct symbol *sym,
 }
 
 
-/* Given an attr with a DW_FORM_dataN value in host byte order, sign-
-   or zero-extend it as appropriate for the symbol's type.  */
+/* Given an attr with a DW_FORM_dataN value in host byte order,
+   zero-extend it as appropriate for the symbol's type.  */
 static void
 dwarf2_const_value_data (struct attribute *attr,
 			 struct symbol *sym,
@@ -10167,12 +10167,7 @@ dwarf2_const_value_data (struct attribute *attr,
   LONGEST l = DW_UNSND (attr);
 
   if (bits < sizeof (l) * 8)
-    {
-      if (TYPE_UNSIGNED (SYMBOL_TYPE (sym)))
-	l &= ((LONGEST) 1 << bits) - 1;
-      else
-	l = (l << (sizeof (l) * 8 - bits)) >> (sizeof (l) * 8 - bits);
-    }
+    l &= ((LONGEST) 1 << bits) - 1;
 
   SYMBOL_VALUE (sym) = l;
   SYMBOL_CLASS (sym) = LOC_CONST;

Re: RFC: fix another regression w/ gcc svn trunk

[Joel Brobecker]

> In the absence of comments I am going to check this in.
> I am undecided as to whether this belongs in 7.2.
> 
> Tom
> 
> 2010-07-27  Tom Tromey  <tromey@redhat.com>
> 
> 	* dwarf2read.c (dwarf2_const_value_data): Never sign extend.

This looks reasonable to me. For 7.2, I'm also 50/50, so I propose
the following compromise: Since it is mostly affecting an unreleased
version of GCC, we can leave this out of 7.2, and reconsider for 7.2.1
after an observation period on the head?

> +/* Given an attr with a DW_FORM_dataN value in host byte order,
> +   zero-extend it as appropriate for the symbol's type.  */

I think it would be nice to explain in the comment the reason for
zero-extending as opposed to sign-extending? In particular, mention
the fact the expected behavior, according to the DWARF committee is
not precisely defined, and the reason for choosing zero-extension
is because GCC has been following that convention for a while.

-- 
Joel

Re: RFC: fix another regression w/ gcc svn trunk

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel> This looks reasonable to me. For 7.2, I'm also 50/50, so I propose
Joel> the following compromise: Since it is mostly affecting an unreleased
Joel> version of GCC, we can leave this out of 7.2, and reconsider for 7.2.1
Joel> after an observation period on the head?

Sounds reasonable to me.

>> +/* Given an attr with a DW_FORM_dataN value in host byte order,
>> +   zero-extend it as appropriate for the symbol's type.  */

Joel> I think it would be nice to explain in the comment the reason for
Joel> zero-extending as opposed to sign-extending?

Good idea.  I have appended what I am checking in.

Tom

2010-07-27  Tom Tromey  <tromey@redhat.com>

	* dwarf2read.c (dwarf2_const_value_data): Never sign extend.

Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.430
diff -u -r1.430 dwarf2read.c
--- dwarf2read.c	28 Jul 2010 16:23:58 -0000	1.430
+++ dwarf2read.c	28 Jul 2010 19:58:30 -0000
@@ -10446,8 +10446,13 @@
 }
 
 
-/* Given an attr with a DW_FORM_dataN value in host byte order, sign-
-   or zero-extend it as appropriate for the symbol's type.  */
+/* Given an attr with a DW_FORM_dataN value in host byte order,
+   zero-extend it as appropriate for the symbol's type.  The DWARF
+   standard (v4) is not entirely clear about the meaning of using
+   DW_FORM_dataN for a constant with a signed type, where the type is
+   wider than the data.  The conclusion of a discussion on the DWARF
+   list was that this is unspecified.  We choose to always zero-extend
+   because that is the interpretation long in use by GCC.  */
 static void
 dwarf2_const_value_data (struct attribute *attr,
 			 struct symbol *sym,
@@ -10456,12 +10461,7 @@
   LONGEST l = DW_UNSND (attr);
 
   if (bits < sizeof (l) * 8)
-    {
-      if (TYPE_UNSIGNED (SYMBOL_TYPE (sym)))
-	l &= ((LONGEST) 1 << bits) - 1;
-      else
-	l = (l << (sizeof (l) * 8 - bits)) >> (sizeof (l) * 8 - bits);
-    }
+    l &= ((LONGEST) 1 << bits) - 1;
 
   SYMBOL_VALUE (sym) = l;
   SYMBOL_CLASS (sym) = LOC_CONST;