[PATCH] Fix up typed DWARF stack support for POINTERS_EXTEND_UNSIGNED targets (PR debug/48853)

[PATCH] Fix up typed DWARF stack support for POINTERS_EXTEND_UNSIGNED targets (PR debug/48853)

[Jakub Jelinek]

Hi!

My typed DWARF stack changes apparently broke ia64-hpux and H.J.'s out of
tree x32 target.  There are several issues:
1) for SUBREG mem_loc_descriptor's 3rd argument was wrong, found by code
   inspection
2) CONST/SYMBOL_REF/LABEL_REF when in MEM addresses on POINTERS_EXTEND_UNSIGNED
   targets are often Pmode, which is unfortunately larger than DWARF2_ADDR_SIZE
   and my conditional would just return NULL in that case instead of
   emitting DW_OP_addr.
3) and, when mem_loc_descriptor is called from unwind code, Pmodes larger
   than DWARF2_ADDR_SIZE would result in the new DW_OP_GNU_*_type etc. ops
   which are not allowed in .eh_frame/.debug_frame
The following patch ought to fix that, bootstrapped/regtested on
x86_64-linux and i686-linux and Steve tested it on ia64-hpux and H.J. on his
port.  Ok for trunk?

2011-05-05  Jakub Jelinek  <jakub@redhat.com>

	PR debug/48853
	* dwarf2out.c (mem_loc_descriptor) <case SUBREG>: Pass mem_mode
	instead of mode as 3rd argument to recursive call.
	(mem_loc_descriptor) <case REG>: If POINTERS_EXTEND_UNSIGNED, don't
	emit DW_OP_GNU_regval_type if mode is Pmode and mem_mode is not
	VOIDmode.
	(mem_loc_descriptor) <case SYMBOL_REF>: If POINTERS_EXTEND_UNSIGNED,
	don't give up if mode is Pmode and mem_mode is not VOIDmode.
	(mem_loc_descriptor) <case CONST_INT>: If POINTERS_EXTEND_UNSIGNED,
	use int_loc_descriptor if mode is Pmode and mem_mode is not VOIDmode.

--- gcc/dwarf2out.c.jj	2011-05-04 10:14:08.000000000 +0200
+++ gcc/dwarf2out.c	2011-05-04 19:08:22.000000000 +0200
@@ -13883,7 +13883,7 @@ mem_loc_descriptor (rtx rtl, enum machin
 
 	  mem_loc_result = mem_loc_descriptor (SUBREG_REG (rtl),
 					       GET_MODE (SUBREG_REG (rtl)),
-					       mode, initialized);
+					       mem_mode, initialized);
 	  if (mem_loc_result == NULL)
 	    break;
 	  type_die = base_type_for_mode (mode, 0);
@@ -13906,7 +13906,13 @@ mem_loc_descriptor (rtx rtl, enum machin
 
     case REG:
       if (GET_MODE_CLASS (mode) != MODE_INT
-	  || GET_MODE_SIZE (mode) > DWARF2_ADDR_SIZE)
+	  || (GET_MODE_SIZE (mode) > DWARF2_ADDR_SIZE
+#ifdef POINTERS_EXTEND_UNSIGNED
+	      && (mode != Pmode
+		  || GET_MODE_SIZE (ptr_mode) != DWARF2_ADDR_SIZE
+		  || mem_mode == VOIDmode)
+#endif
+	      ))
 	{
 	  dw_die_ref type_die;
 
@@ -14049,9 +14055,18 @@ mem_loc_descriptor (rtx rtl, enum machin
 	 pool.  */
     case CONST:
     case SYMBOL_REF:
+      if (GET_MODE_CLASS (mode) != MODE_INT)
+	break;
+#ifndef POINTERS_EXTEND_UNSIGNED
+      if (GET_MODE_SIZE (mode) > DWARF2_ADDR_SIZE)
+	break;
+#else
       if (GET_MODE_SIZE (mode) > DWARF2_ADDR_SIZE
-	  || GET_MODE_CLASS (mode) != MODE_INT)
+	  && (mode != Pmode
+	      || GET_MODE_SIZE (ptr_mode) != DWARF2_ADDR_SIZE
+	      || mem_mode == VOIDmode))
 	break;
+#endif
       if (GET_CODE (rtl) == SYMBOL_REF
 	  && SYMBOL_REF_TLS_MODEL (rtl) != TLS_MODEL_NONE)
 	{
@@ -14288,7 +14303,14 @@ mem_loc_descriptor (rtx rtl, enum machin
       break;
 
     case CONST_INT:
-      if (GET_MODE_SIZE (mode) <= DWARF2_ADDR_SIZE)
+      if (GET_MODE_SIZE (mode) <= DWARF2_ADDR_SIZE
+#ifdef POINTERS_EXTEND_UNSIGNED
+	  || (mode == Pmode
+	      && GET_MODE_SIZE (ptr_mode) == DWARF2_ADDR_SIZE
+	      && mem_mode != VOIDmode
+	      && trunc_int_for_mode (INTVAL (rtl), ptr_mode) == INTVAL (rtl))
+#endif
+	  )
 	{
 	  mem_loc_result = int_loc_descriptor (INTVAL (rtl));
 	  break;

	Jakub

Re: [PATCH] Fix up typed DWARF stack support for POINTERS_EXTEND_UNSIGNED targets (PR debug/48853)

[Jason Merrill]

> +	      && (mode != Pmode
> +		  || GET_MODE_SIZE (ptr_mode) != DWARF2_ADDR_SIZE

This test seems overly cautious; it seems implausible to have 
DWARF2_ADDR_SIZE smaller than GET_MODE_SIZE (ptr_mode), and having it be 
larger doesn't seem like a problem.

> +		  || mem_mode == VOIDmode)

We can't handle Pmode void* like other pointers?

Jason

Re: [PATCH] Fix up typed DWARF stack support for POINTERS_EXTEND_UNSIGNED targets (PR debug/48853)

[Jakub Jelinek]

On Thu, May 05, 2011 at 10:25:53AM -0400, Jason Merrill wrote:
> >+	      && (mode != Pmode
> >+		  || GET_MODE_SIZE (ptr_mode) != DWARF2_ADDR_SIZE
> 
> This test seems overly cautious; it seems implausible to have
> DWARF2_ADDR_SIZE smaller than GET_MODE_SIZE (ptr_mode), and having
> it be larger doesn't seem like a problem.

Ok, I guess I can leave those ptr_mode GET_MODE_SIZE tests out.

> >+		  || mem_mode == VOIDmode)
> 
> We can't handle Pmode void* like other pointers?

mem_mode == VOIDmode doesn't mean a void* pointer, mem_mode != VOIDmode
means mem_loc_descriptor is called on some MEM's address and gives the
mode of the MEM, while VOIDmode mem_mode means it isn't a memory address,
but just a random rtl that we wish to translate into DWARF location
expression.

If it is not a MEM address (i.e. mem_mode is VOIDmode) and
Pmode is wider than DWARF2_ADDR_SIZE, then how do you otherwise find out
if it is a pointer and you leave it up to the debug info consumer to do
some zero/sign/something else extension from DWARF2_ADDR_SIZE to
Pmode size, or if it is any other DImode quantity, which needs to be
represented using DW_OP_GNU_*_type?
Say for i?86 -m32
unsigned long long x = (uintptr_t) &y + 0x123456789abcULL;
(plus:DI (zero_extend:DI (symbol_ref:SI "y")) (const_double 0x123456789abc))
using untyped ops would be incorrect.

We could have some flag/global var whether mem_loc_descriptor is called
from unwind info code or not, and just assume untyped (ptr_mode) arithmetics
is always fine in that case, and just use always DW_OP_GNU_*_type
on POINTERS_EXTEND_UNSIGNED targets for Pmode elsewhere, the problem with
that is that the generated debug info for -gdwarf-strict would be very bad
and debug info would be unnecessarily large.
And we'd still need to represent Pmode SYMBOL_REF/CONST/LABEL_REF somehow
with the zero/sign extension being explicit (like
DW_OP_addr <foo + 6> DW_OP_GNU_convert <unsigned int> DW_OP_GNU_convert <long long>
for zero-extension, s/unsigned int/int/ for sign extension).  Unfortunately
POINTERS_EXTEND_UNSIGNED can be also -1 which means target has special
instruction to do the extension, but nothing says what that insn actually
performs.  So I think it is better to use untyped ops for memory addresses
and typed ops outside of memory addresses when the mode is larger than
DWARF2_ADDR_SIZE.

	Jakub

Re: [PATCH] Fix up typed DWARF stack support for POINTERS_EXTEND_UNSIGNED targets (PR debug/48853)

[Jason Merrill]

On 05/05/2011 10:56 AM, Jakub Jelinek wrote:
>> We can't handle Pmode void* like other pointers?
>
> mem_mode == VOIDmode doesn't mean a void* pointer, mem_mode != VOIDmode
> means mem_loc_descriptor is called on some MEM's address and gives the
> mode of the MEM, while VOIDmode mem_mode means it isn't a memory address,
> but just a random rtl that we wish to translate into DWARF location
> expression.

Ah, OK.

Jason

Re: [PATCH] Fix up typed DWARF stack support for POINTERS_EXTEND_UNSIGNED targets (PR debug/48853)

[H.J. Lu]

On Thu, May 5, 2011 at 2:20 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> My typed DWARF stack changes apparently broke ia64-hpux and H.J.'s out of
> tree x32 target.  There are several issues:
> 1) for SUBREG mem_loc_descriptor's 3rd argument was wrong, found by code
>   inspection
> 2) CONST/SYMBOL_REF/LABEL_REF when in MEM addresses on POINTERS_EXTEND_UNSIGNED
>   targets are often Pmode, which is unfortunately larger than DWARF2_ADDR_SIZE
>   and my conditional would just return NULL in that case instead of
>   emitting DW_OP_addr.
> 3) and, when mem_loc_descriptor is called from unwind code, Pmodes larger
>   than DWARF2_ADDR_SIZE would result in the new DW_OP_GNU_*_type etc. ops
>   which are not allowed in .eh_frame/.debug_frame
> The following patch ought to fix that, bootstrapped/regtested on
> x86_64-linux and i686-linux and Steve tested it on ia64-hpux and H.J. on his
> port.  Ok for trunk?
>
> 2011-05-05  Jakub Jelinek  <jakub@redhat.com>
>
>        PR debug/48853
>        * dwarf2out.c (mem_loc_descriptor) <case SUBREG>: Pass mem_mode
>        instead of mode as 3rd argument to recursive call.
>        (mem_loc_descriptor) <case REG>: If POINTERS_EXTEND_UNSIGNED, don't
>        emit DW_OP_GNU_regval_type if mode is Pmode and mem_mode is not
>        VOIDmode.
>        (mem_loc_descriptor) <case SYMBOL_REF>: If POINTERS_EXTEND_UNSIGNED,
>        don't give up if mode is Pmode and mem_mode is not VOIDmode.
>        (mem_loc_descriptor) <case CONST_INT>: If POINTERS_EXTEND_UNSIGNED,
>        use int_loc_descriptor if mode is Pmode and mem_mode is not VOIDmode.
>

Here is the missing patch for case SUBREG.  OK for trunk if there is
no regressions?

Thanks.


H.J.
----
2011-05-06  H.J. Lu  <hongjiu.lu@intel.com>

	PR debug/48853
	* dwarf2out.c (mem_loc_descriptor) <case SUBREG>: If
	POINTERS_EXTEND_UNSIGNED, don't give up if mode is Pmode and
	mem_mode is not VOIDmode.

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 026e4a7..049ca8e 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -13892,7 +13892,11 @@ mem_loc_descriptor (rtx rtl, enum machine_mode mode,
 	break;
       if (GET_MODE_CLASS (mode) == MODE_INT
 	  && GET_MODE_CLASS (GET_MODE (SUBREG_REG (rtl))) == MODE_INT
-	  && GET_MODE_SIZE (mode) <= DWARF2_ADDR_SIZE
+	  && (GET_MODE_SIZE (mode) <= DWARF2_ADDR_SIZE
+#ifdef POINTERS_EXTEND_UNSIGNED
+	      || (mode == Pmode && mem_mode != VOIDmode)
+#endif
+	     )
 	  && GET_MODE_SIZE (GET_MODE (SUBREG_REG (rtl))) <= DWARF2_ADDR_SIZE)
 	{
 	  mem_loc_result = mem_loc_descriptor (SUBREG_REG (rtl),

Re: [PATCH] Fix up typed DWARF stack support for POINTERS_EXTEND_UNSIGNED targets (PR debug/48853)

[Jason Merrill]

OK.

Jason

Re: [PATCH] Fix up typed DWARF stack support for POINTERS_EXTEND_UNSIGNED targets (PR debug/48853)

[H.J. Lu]

[-- Attachment #1: Type: text/plain, Size: 4773 bytes --]

On Thu, May 5, 2011 at 2:20 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> My typed DWARF stack changes apparently broke ia64-hpux and H.J.'s out of
> tree x32 target.  There are several issues:
> 1) for SUBREG mem_loc_descriptor's 3rd argument was wrong, found by code
>   inspection
> 2) CONST/SYMBOL_REF/LABEL_REF when in MEM addresses on POINTERS_EXTEND_UNSIGNED
>   targets are often Pmode, which is unfortunately larger than DWARF2_ADDR_SIZE
>   and my conditional would just return NULL in that case instead of
>   emitting DW_OP_addr.
> 3) and, when mem_loc_descriptor is called from unwind code, Pmodes larger
>   than DWARF2_ADDR_SIZE would result in the new DW_OP_GNU_*_type etc. ops
>   which are not allowed in .eh_frame/.debug_frame
> The following patch ought to fix that, bootstrapped/regtested on
> x86_64-linux and i686-linux and Steve tested it on ia64-hpux and H.J. on his
> port.  Ok for trunk?
>
> 2011-05-05  Jakub Jelinek  <jakub@redhat.com>
>
>        PR debug/48853
>        * dwarf2out.c (mem_loc_descriptor) <case SUBREG>: Pass mem_mode
>        instead of mode as 3rd argument to recursive call.
>        (mem_loc_descriptor) <case REG>: If POINTERS_EXTEND_UNSIGNED, don't
>        emit DW_OP_GNU_regval_type if mode is Pmode and mem_mode is not
>        VOIDmode.
>        (mem_loc_descriptor) <case SYMBOL_REF>: If POINTERS_EXTEND_UNSIGNED,
>        don't give up if mode is Pmode and mem_mode is not VOIDmode.
>        (mem_loc_descriptor) <case CONST_INT>: If POINTERS_EXTEND_UNSIGNED,
>        use int_loc_descriptor if mode is Pmode and mem_mode is not VOIDmode.
>

Another problem.  This patch

http://gcc.gnu.org/ml/gcc-patches/2011-03/msg01730.html

has
@@ -14562,7 +15110,10 @@ loc_descriptor (rtx rtl, enum machine_mo
 	 up an entire register.  For now, just assume that it is
 	 legitimate to make the Dwarf info refer to the whole register which
 	 contains the given subreg.  */
-      loc_result = loc_descriptor (SUBREG_REG (rtl), mode, initialized);
+      if (REG_P (SUBREG_REG (rtl)) && subreg_lowpart_p (rtl))
+	loc_result = loc_descriptor (SUBREG_REG (rtl), mode, initialized);
+      else
+	goto do_default;
       break;

     case REG:

It doesn't work with Pmode != ptr_mode.  I got

Breakpoint 5, loc_descriptor (rtl=0x7ffff0acc900, mode=SImode,
    initialized=VAR_INIT_STATUS_INITIALIZED)
    at /export/gnu/import/git/gcc-x32/gcc/dwarf2out.c:15027
15027	      if ((REG_P (SUBREG_REG (rtl))
(gdb) bt
#0  loc_descriptor (rtl=0x7ffff0acc900, mode=SImode,
    initialized=VAR_INIT_STATUS_INITIALIZED)
    at /export/gnu/import/git/gcc-x32/gcc/dwarf2out.c:15027
#1  0x00000000006dae2a in loc_descriptor (rtl=0x7ffff0a6c660, mode=SImode,
    initialized=VAR_INIT_STATUS_INITIALIZED)
    at /export/gnu/import/git/gcc-x32/gcc/dwarf2out.c:15071
#2  0x00000000006dbc69 in dw_loc_list_1 (loc=0x7ffff0a87320,
    varloc=0x7ffff0a6c660, want_address=2,
    initialized=VAR_INIT_STATUS_INITIALIZED)
    at /export/gnu/import/git/gcc-x32/gcc/dwarf2out.c:15346
#3  0x00000000006dc5b2 in dw_loc_list (loc_list=0x7ffff0a6c800,
    decl=0x7ffff0a87320, want_address=2)
    at /export/gnu/import/git/gcc-x32/gcc/dwarf2out.c:15602
#4  0x00000000006dd920 in loc_list_from_tree (loc=0x7ffff0a87320,
    want_address=2) at /export/gnu/import/git/gcc-x32/gcc/dwarf2out.c:15990
#5  0x00000000006e3d81 in add_location_or_const_value_attribute (
    die=0x7ffff0acd640, decl=0x7ffff0a87320, cache_p=0 '\000',
    attr=DW_AT_location)
    at /export/gnu/import/git/gcc-x32/gcc/dwarf2out.c:17500
#6  0x00000000006eae13 in gen_formal_parameter_die (node=0x7ffff0a87320,
    origin=0x7ffff0b71d48, emit_name_p=1 '\001', context_die=0x7ffff0acd5a0)
    at /export/gnu/import/git/gcc-x32/gcc/dwarf2out.c:19244
#7  0x00000000006f6432 in gen_decl_die (decl=0x7ffff0a87320, origin=0x0,
---Type <return> to continue, or q <return> to quit---q
contextQuit
(gdb) call debug_rtx (rtl)
(subreg:SI (symbol_ref:DI ("a") [flags 0x2] <var_decl 0x7ffff0a87000 a>) 0)
(gdb) f 1
#1  0x00000000006dae2a in loc_descriptor (rtl=0x7ffff0a6c660, mode=SImode,
    initialized=VAR_INIT_STATUS_INITIALIZED)
    at /export/gnu/import/git/gcc-x32/gcc/dwarf2out.c:15071
15071		  loc_result = loc_descriptor (loc, mode, initialized);
(gdb) call debug_rtx (rtl)
(var_location xxxxx (subreg:SI (symbol_ref:DI ("a") [flags 0x2]
<var_decl 0x7ffff0a87000 a>) 0))

This patch restores the old behavior for Pmode. OK for trunk if there
are no regressions?

Thanks.


-- 
H.J.
2011-05-11  H.J. Lu  <hongjiu.lu@intel.com>

	PR debug/48853
	* dwarf2out.c (loc_descriptor) <case SUBREG>: If
	POINTERS_EXTEND_UNSIGNED is defined, don't give up if mode of
	SUBREG is Pmode.

[-- Attachment #2: gcc-pr48853-2.patch --]
[-- Type: text/x-diff, Size: 875 bytes --]

2011-05-11  H.J. Lu  <hongjiu.lu@intel.com>

	PR debug/48853
	* dwarf2out.c (loc_descriptor) <case SUBREG>: If
	POINTERS_EXTEND_UNSIGNED is defined, don't give up if mode of
	SUBREG is Pmode.

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index b85a55e..03d12de 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -15024,7 +15024,12 @@ loc_descriptor (rtx rtl, enum machine_mode mode,
 	 up an entire register.  For now, just assume that it is
 	 legitimate to make the Dwarf info refer to the whole register which
 	 contains the given subreg.  */
-      if (REG_P (SUBREG_REG (rtl)) && subreg_lowpart_p (rtl))
+      if ((REG_P (SUBREG_REG (rtl))
+#ifdef POINTERS_EXTEND_UNSIGNED
+	   || GET_MODE (SUBREG_REG (rtl)) == Pmode
+#endif
+	  )
+	  && subreg_lowpart_p (rtl))
 	loc_result = loc_descriptor (SUBREG_REG (rtl), mode, initialized);
       else
 	goto do_default;

Re: [PATCH] Fix up typed DWARF stack support for POINTERS_EXTEND_UNSIGNED targets (PR debug/48853)

[Jakub Jelinek]

On Wed, May 11, 2011 at 12:28:18PM -0700, H.J. Lu wrote:
> This patch restores the old behavior for Pmode. OK for trunk if there
> are no regressions?

That is IMHO wrong, ignoring subregs is a very bad idea.
While you can workaround generation of the DW_OP_GNU_convert that way
(why do you want that?, just install newer gdb that has typed dwarf stack
support, coming hopefully soon), you'd get incorrect debug info for
long long to int casts.

	Jakub

Re: [PATCH] Fix up typed DWARF stack support for POINTERS_EXTEND_UNSIGNED targets (PR debug/48853)

[H.J. Lu]

On Thu, May 12, 2011 at 7:56 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Wed, May 11, 2011 at 12:28:18PM -0700, H.J. Lu wrote:
>> This patch restores the old behavior for Pmode. OK for trunk if there
>> are no regressions?
>
> That is IMHO wrong, ignoring subregs is a very bad idea.
> While you can workaround generation of the DW_OP_GNU_convert that way
> (why do you want that?, just install newer gdb that has typed dwarf stack
> support, coming hopefully soon), you'd get incorrect debug info for
> long long to int casts.
>

I will wait for the new gdb.

Thanks.


-- 
H.J.

Re: [PATCH] Fix up typed DWARF stack support for POINTERS_EXTEND_UNSIGNED targets (PR debug/48853)

[Tom Tromey]

>>>>> "H.J." == H J Lu <hjl.tools@gmail.com> writes:

H.J.> On Thu, May 12, 2011 at 7:56 AM, Jakub Jelinek <jakub@redhat.com> wrote:
>> On Wed, May 11, 2011 at 12:28:18PM -0700, H.J. Lu wrote:
>>> This patch restores the old behavior for Pmode. OK for trunk if there
>>> are no regressions?
>> 
>> That is IMHO wrong, ignoring subregs is a very bad idea.
>> While you can workaround generation of the DW_OP_GNU_convert that way
>> (why do you want that?, just install newer gdb that has typed dwarf stack
>> support, coming hopefully soon), you'd get incorrect debug info for
>> long long to int casts.
>> 

H.J.> I will wait for the new gdb.

I'm going to commit the changes today.

Tom