diff mbox

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

Message ID BANLkTi==QDuvBzsqMkWpeafSrds=QZ1xBA@mail.gmail.com
State New
Headers show

Commit Message

H.J. Lu May 11, 2011, 7:28 p.m. UTC 
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.

Comments

Jakub Jelinek May 12, 2011, 2:56 p.m. UTC | #1 
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
H.J. Lu May 12, 2011, 3:36 p.m. UTC | #2 
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.
Tom Tromey May 12, 2011, 4:06 p.m. UTC | #3 
>>>>> "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
diff mbox

Patch

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;