diff mbox

RFA: dwarf2out vs. LP64-on-ELF32

Message ID 87pq8p8095.fsf@talisman.home
State New
Headers show

Commit Message

Richard Sandiford June 24, 2012, 10:36 a.m. UTC
One of the more unfortunate things MIPS has inherited is an LP64 ABI
that uses 32-bit rather than 64-bit ELF.  I've no idea how many people
use it these days (if anyone), but it happens to the ABI of the MIPS
primary target, mipsisa64-elf.

Because we're using 32-bit ELF, DWARF2_ADDR_SIZE is 4 rather than 8.
This currently causes problems with dwarf2out when we have an eliminated
expression such arg_pointer_rtx or arg_pointer_rtx + CONST.  Normally we'd
treat them specially and apply the elimination, but this implicitly
requires Pmode to be no bigger than DWARF2_ADDR_SIZE:

    case REG:
      if (GET_MODE_CLASS (mode) != MODE_INT
	  || (GET_MODE_SIZE (mode) > DWARF2_ADDR_SIZE
#ifdef POINTERS_EXTEND_UNSIGNED
	      && (mode != Pmode || mem_mode == VOIDmode)
#endif
	      ))
	{
	  [...]
	  mem_loc_result = new_loc_descr (DW_OP_GNU_regval_type,
					  dbx_reg_number (rtl), 0);
	  [...]
	}
      if (REGNO (rtl) < FIRST_PSEUDO_REGISTER)
	mem_loc_result = based_loc_descr (rtl, 0, VAR_INIT_STATUS_INITIALIZED);
      [...]

    case PLUS:
    plus:
      if (is_based_loc (rtl)
	  && GET_MODE_SIZE (mode) <= DWARF2_ADDR_SIZE
	  && GET_MODE_CLASS (mode) == MODE_INT)
	mem_loc_result = based_loc_descr (XEXP (rtl, 0),
					  INTVAL (XEXP (rtl, 1)),
					  VAR_INIT_STATUS_INITIALIZED);

where based_loc_descr does the elimination.

So as things stand we take the first "if", and try to get the register
number of the fake arg_pointer_rtx.  This was latent for a while
(at least in the sense of GCC testing, although probably not if
you actually tried to debug using it) until H.J.'s recent patch
to add an assert to dbx_reg_regnumber.  Which just goes to show
how good an assert it is.

I realise it's a bit clunky, but is the attached patch OK?
Tested on mipsisa64-elf, where it restores the build, and on
x86_64-linux-gnu.

Richard


gcc/
	* dwarf2out.c (mem_loc_descriptor): Ignore DWARF2_ADDR_SIZE
	for frame- and argument-pointer reg+offset expressions.

Comments

H.J. Lu June 24, 2012, 2:17 p.m. UTC | #1
On Sun, Jun 24, 2012 at 3:36 AM, Richard Sandiford
<rdsandiford@googlemail.com> wrote:
> One of the more unfortunate things MIPS has inherited is an LP64 ABI
> that uses 32-bit rather than 64-bit ELF.  I've no idea how many people
> use it these days (if anyone), but it happens to the ABI of the MIPS
> primary target, mipsisa64-elf.
>
> Because we're using 32-bit ELF, DWARF2_ADDR_SIZE is 4 rather than 8.
> This currently causes problems with dwarf2out when we have an eliminated
> expression such arg_pointer_rtx or arg_pointer_rtx + CONST.  Normally we'd
> treat them specially and apply the elimination, but this implicitly
> requires Pmode to be no bigger than DWARF2_ADDR_SIZE:
>
>    case REG:
>      if (GET_MODE_CLASS (mode) != MODE_INT
>          || (GET_MODE_SIZE (mode) > DWARF2_ADDR_SIZE
> #ifdef POINTERS_EXTEND_UNSIGNED
>              && (mode != Pmode || mem_mode == VOIDmode)
> #endif
>              ))
>        {
>          [...]
>          mem_loc_result = new_loc_descr (DW_OP_GNU_regval_type,
>                                          dbx_reg_number (rtl), 0);
>          [...]
>        }
>      if (REGNO (rtl) < FIRST_PSEUDO_REGISTER)
>        mem_loc_result = based_loc_descr (rtl, 0, VAR_INIT_STATUS_INITIALIZED);
>      [...]
>
>    case PLUS:
>    plus:
>      if (is_based_loc (rtl)
>          && GET_MODE_SIZE (mode) <= DWARF2_ADDR_SIZE
>          && GET_MODE_CLASS (mode) == MODE_INT)
>        mem_loc_result = based_loc_descr (XEXP (rtl, 0),
>                                          INTVAL (XEXP (rtl, 1)),
>                                          VAR_INIT_STATUS_INITIALIZED);
>
> where based_loc_descr does the elimination.
>
> So as things stand we take the first "if", and try to get the register
> number of the fake arg_pointer_rtx.  This was latent for a while
> (at least in the sense of GCC testing, although probably not if
> you actually tried to debug using it) until H.J.'s recent patch
> to add an assert to dbx_reg_regnumber.  Which just goes to show
> how good an assert it is.

I added it for

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52857

> I realise it's a bit clunky, but is the attached patch OK?
> Tested on mipsisa64-elf, where it restores the build, and on
> x86_64-linux-gnu.
>
> Richard
>
>
> gcc/
>        * dwarf2out.c (mem_loc_descriptor): Ignore DWARF2_ADDR_SIZE
>        for frame- and argument-pointer reg+offset expressions.
>

It looks the same as

http://gcc.gnu.org/ml/gcc-patches/2012-04/msg01815.html

and my patch has 2 testcases.

Thanks.
Richard Sandiford June 24, 2012, 3:46 p.m. UTC | #2
"H.J. Lu" <hjl.tools@gmail.com> writes:
> On Sun, Jun 24, 2012 at 3:36 AM, Richard Sandiford
> <rdsandiford@googlemail.com> wrote:
>> I realise it's a bit clunky, but is the attached patch OK?
>> Tested on mipsisa64-elf, where it restores the build, and on
>> x86_64-linux-gnu.
>>
>> Richard
>>
>>
>> gcc/
>>        * dwarf2out.c (mem_loc_descriptor): Ignore DWARF2_ADDR_SIZE
>>        for frame- and argument-pointer reg+offset expressions.
>>
>
> It looks the same as
>
> http://gcc.gnu.org/ml/gcc-patches/2012-04/msg01815.html

Ah!  Yes indeed.

> and my patch has 2 testcases.

Point taken, but for the record, I didn't think testcases were needed
for mine, since it's a build breaker for a primary target...

Richard
H.J. Lu June 24, 2012, 3:52 p.m. UTC | #3
On Sun, Jun 24, 2012 at 8:46 AM, Richard Sandiford
<rdsandiford@googlemail.com> wrote:
> "H.J. Lu" <hjl.tools@gmail.com> writes:
>> On Sun, Jun 24, 2012 at 3:36 AM, Richard Sandiford
>> <rdsandiford@googlemail.com> wrote:
>>> I realise it's a bit clunky, but is the attached patch OK?
>>> Tested on mipsisa64-elf, where it restores the build, and on
>>> x86_64-linux-gnu.
>>>
>>> Richard
>>>
>>>
>>> gcc/
>>>        * dwarf2out.c (mem_loc_descriptor): Ignore DWARF2_ADDR_SIZE
>>>        for frame- and argument-pointer reg+offset expressions.
>>>
>>
>> It looks the same as
>>
>> http://gcc.gnu.org/ml/gcc-patches/2012-04/msg01815.html
>
> Ah!  Yes indeed.
>
>> and my patch has 2 testcases.
>
> Point taken, but for the record, I didn't think testcases were needed
> for mine, since it's a build breaker for a primary target...
>

I see.

FWIW, GDB crashed when I tried to debug some x32 binaries.  I
think the assert and this fix should be applied to trunk as well as
the affected branches.
Andrew Pinski June 24, 2012, 6:04 p.m. UTC | #4
On Sun, Jun 24, 2012 at 3:36 AM, Richard Sandiford
<rdsandiford@googlemail.com> wrote:
> One of the more unfortunate things MIPS has inherited is an LP64 ABI
> that uses 32-bit rather than 64-bit ELF.  I've no idea how many people
> use it these days (if anyone), but it happens to the ABI of the MIPS
> primary target, mipsisa64-elf.

We (Cavium) use the EABI-64 ABI for our simple-exec (bare-metal)
target so there is an user base out there now.

Thanks,
Andrew
Richard Sandiford June 25, 2012, 8:41 p.m. UTC | #5
Andrew Pinski <pinskia@gmail.com> writes:
> On Sun, Jun 24, 2012 at 3:36 AM, Richard Sandiford
> <rdsandiford@googlemail.com> wrote:
>> One of the more unfortunate things MIPS has inherited is an LP64 ABI
>> that uses 32-bit rather than 64-bit ELF.  I've no idea how many people
>> use it these days (if anyone), but it happens to the ABI of the MIPS
>> primary target, mipsisa64-elf.
>
> We (Cavium) use the EABI-64 ABI for our simple-exec (bare-metal)
> target so there is an user base out there now.

Thanks, that's good to know.  I'm never entirely sure how things
are being used, so this sort of information definitely helps.

I also realised that it's no different from -msym32 -mabi=64,
which is the standard way of building many 64-bit linux kernels,
so my negative comments above were just plain wrong.

Richard
diff mbox

Patch

Index: gcc/dwarf2out.c
===================================================================
--- gcc/dwarf2out.c	2012-06-23 13:32:55.723114868 +0100
+++ gcc/dwarf2out.c	2012-06-23 16:55:39.694558787 +0100
@@ -11183,6 +11183,8 @@  mem_loc_descriptor (rtx rtl, enum machin
     case REG:
       if (GET_MODE_CLASS (mode) != MODE_INT
 	  || (GET_MODE_SIZE (mode) > DWARF2_ADDR_SIZE
+	      && rtl != frame_pointer_rtx
+	      && rtl != arg_pointer_rtx
 #ifdef POINTERS_EXTEND_UNSIGNED
 	      && (mode != Pmode || mem_mode == VOIDmode)
 #endif
@@ -11455,7 +11457,9 @@  mem_loc_descriptor (rtx rtl, enum machin
     case PLUS:
     plus:
       if (is_based_loc (rtl)
-	  && GET_MODE_SIZE (mode) <= DWARF2_ADDR_SIZE
+	  && (GET_MODE_SIZE (mode) <= DWARF2_ADDR_SIZE
+	      || XEXP (rtl, 0) == frame_pointer_rtx
+	      || XEXP (rtl, 0) == arg_pointer_rtx)
 	  && GET_MODE_CLASS (mode) == MODE_INT)
 	mem_loc_result = based_loc_descr (XEXP (rtl, 0),
 					  INTVAL (XEXP (rtl, 1)),