MIPS: 64-bit DWARF (was: debug problem with prelinked libraries)

MIPS: 64-bit DWARF (was: debug problem with prelinked libraries)

[Thomas Schwinge]

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

Hello!

On 2010-07-01 15:43, I wrote:
> On 2010-05-07 13:23, Jan Kratochvil wrote:
>> On Wed, 05 May 2010 16:59:36 +0200, Andrew Stubbs wrote:
>>> > prelink includes code to manipulate the contents of the debug info.
>> [...]
>>> Maybe prelink has failed to relocate the debug info in this case?
>>
>> The attached patch should fix it.
>>
>> Although SHT_MIPS_DWARF value should be recognized only for e_machine as
>> EM_MIPS or EM_MIPS_RS3_LE or EM_MIPS_X?  Or are there some other rules?
>
>> --- prelink/src/dso.c-orig	2010-04-13 16:41:15.000000000 +0200
>> +++ prelink/src/dso.c	2010-05-07 15:16:25.000000000 +0200
>> @@ -1381,6 +1381,7 @@ adjust_dso (DSO *dso, GElf_Addr start, G
>>        switch (dso->shdr[i].sh_type)
>>  	{
>>  	case SHT_PROGBITS:
>> +	case SHT_MIPS_DWARF:
>>  	  name = strptr (dso, dso->ehdr.e_shstrndx, dso->shdr[i].sh_name);
>>  	  if (strcmp (name, ".stab") == 0
>>  	      && adjust_stabs (dso, i, start, adjust))
>
> Unfortunately, this patch causes the following regression for about every
> test of the prelink testsuite, in -mabi=64 configurations (only):
>
>     mips-wrs-linux-gnu-prelink -c ./prelink.conf -C ./prelink.cache --ld-library-path=. --dynamic-linker=./ld.so.1 -vm ./reloc1
>     Laying out 4 libraries in virtual address space 0000005800000000-0000009800000000
>     Assigned virtual address space slots for libraries:
>     ./ld.so.1                                                    0000005800000000-0000005800032328
>     ./libc.so.6                                                  0000005800040000-00000058001cbdb0
>     ./reloc1lib1.so                                              00000058001d0000-00000058001e0b10
>     ./reloc1lib2.so                                              00000058001f0000-00000058002009d0
>     Prelinking /scratch/thomas/issue8927/obj/test-4.3a-294-mips-wrs-linux-gnu/host-i686-pc-linux-gnu/mabi_64/prelink.d/ld-2.8.so
>     mips-wrs-linux-gnu-prelink: /scratch/thomas/issue8927/obj/test-4.3a-294-mips-wrs-linux-gnu/host-i686-pc-linux-gnu/mabi_64/prelink.d/ld-2.8.so: 64-bit DWARF not supported

This happens when processing .debug_lines, which is generated by GAS.

prelink does not support the 64-bit DWARF format.  From a quick glance,
GDB does, see dwarf2read.c:read_initial_length.

MIPS is, per gas/config/tc-mips, the only architecture to override
DWARF2_FORMAT (which defaults to 32-bit), and thus possibly can emit
64-bit DWARF code, which it actually does for -mabi=64, see
tc-mips.c:mips_dwarf2_format.

In <http://sourceware.org/ml/binutils/2002-11/msg00429.html> this 64-bit
DWARF format support has originally been added.

What's our way forward?  Can we switch MIPS back to only using the 32-bit
DWARF format (save the IRIX case)?  Do we need to extend prelink?

As I understand the GCC code, it always uses the 32-bit DWARF format,
even for MIPS 64-bit ABIs.


Regards,
 Thomas

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: MIPS: 64-bit DWARF

[Thomas Schwinge]

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

Hello!

This is about switching MIPS from 64-bit to 32-bit DWARF format (as other
architectures are doing), for the benefit of prelink.

On 2010-07-06 09:59, I wrote:
> On 2010-07-01 15:43, I wrote:
>> On 2010-05-07 13:23, Jan Kratochvil wrote:
>>> On Wed, 05 May 2010 16:59:36 +0200, Andrew Stubbs wrote:
>>>> > prelink includes code to manipulate the contents of the debug info.
>>> [...]
>>>> Maybe prelink has failed to relocate the debug info in this case?
>>>
>>> The attached patch should fix it.
>>>
>>> Although SHT_MIPS_DWARF value should be recognized only for e_machine as
>>> EM_MIPS or EM_MIPS_RS3_LE or EM_MIPS_X?  Or are there some other rules?
>>
>>> --- prelink/src/dso.c-orig	2010-04-13 16:41:15.000000000 +0200
>>> +++ prelink/src/dso.c	2010-05-07 15:16:25.000000000 +0200
>>> @@ -1381,6 +1381,7 @@ adjust_dso (DSO *dso, GElf_Addr start, G
>>>        switch (dso->shdr[i].sh_type)
>>>  	{
>>>  	case SHT_PROGBITS:
>>> +	case SHT_MIPS_DWARF:
>>>  	  name = strptr (dso, dso->ehdr.e_shstrndx, dso->shdr[i].sh_name);
>>>  	  if (strcmp (name, ".stab") == 0
>>>  	      && adjust_stabs (dso, i, start, adjust))
>>
>> Unfortunately, this patch causes the following regression for about every
>> test of the prelink testsuite, in -mabi=64 configurations (only):
>>
>>     mips-wrs-linux-gnu-prelink -c ./prelink.conf -C ./prelink.cache --ld-library-path=. --dynamic-linker=./ld.so.1 -vm ./reloc1
>>     Laying out 4 libraries in virtual address space 0000005800000000-0000009800000000
>>     Assigned virtual address space slots for libraries:
>>     ./ld.so.1                                                    0000005800000000-0000005800032328
>>     ./libc.so.6                                                  0000005800040000-00000058001cbdb0
>>     ./reloc1lib1.so                                              00000058001d0000-00000058001e0b10
>>     ./reloc1lib2.so                                              00000058001f0000-00000058002009d0
>>     Prelinking /scratch/thomas/issue8927/obj/test-4.3a-294-mips-wrs-linux-gnu/host-i686-pc-linux-gnu/mabi_64/prelink.d/ld-2.8.so
>>     mips-wrs-linux-gnu-prelink: /scratch/thomas/issue8927/obj/test-4.3a-294-mips-wrs-linux-gnu/host-i686-pc-linux-gnu/mabi_64/prelink.d/ld-2.8.so: 64-bit DWARF not supported
>
> This happens when processing .debug_lines, which is generated by GAS.
>
> prelink does not support the 64-bit DWARF format.  From a quick glance,
> GDB does, see dwarf2read.c:read_initial_length.
>
> MIPS is, per gas/config/tc-mips, the only architecture to override
> DWARF2_FORMAT (which defaults to 32-bit), and thus possibly can emit
> 64-bit DWARF code, which it actually does for -mabi=64, see
> tc-mips.c:mips_dwarf2_format.
>
> In <http://sourceware.org/ml/binutils/2002-11/msg00429.html> this 64-bit
> DWARF format support has originally been added.
>
> What's our way forward?  Can we switch MIPS back to only using the 32-bit
> DWARF format (save the IRIX case)?  Do we need to extend prelink?
>
> As I understand the GCC code, it always uses the 32-bit DWARF format,
> even for MIPS 64-bit ABIs.

Here is the patch that I tested for (a non-HEAD version of) binutils.
This makes prelink happy, and there are no regressions in the GDB
testsuite.  (Anything else to test?)  I left in the 64-bit stuff for
TE_IRIX, as I have no idea about it.

Index: gas/config/tc-mips.c
===================================================================
--- gas/config/tc-mips.c	(revision 291480)
+++ gas/config/tc-mips.c	(working copy)
@@ -15499,20 +15499,16 @@
 #endif
 }
 
+#ifdef TE_IRIX
 enum dwarf2_format
 mips_dwarf2_format (void)
 {
   if (HAVE_64BIT_SYMBOLS)
-    {
-#ifdef TE_IRIX
-      return dwarf2_format_64bit_irix;
-#else
-      return dwarf2_format_64bit;
-#endif
-    }
+    return dwarf2_format_64bit_irix;
   else
     return dwarf2_format_32bit;
 }
+#endif
 
 int
 mips_dwarf2_addr_size (void)
Index: gas/config/tc-mips.h
===================================================================
--- gas/config/tc-mips.h	(revision 291480)
+++ gas/config/tc-mips.h	(working copy)
@@ -165,9 +165,13 @@
 extern void mips_enable_auto_align (void);
 #define md_elf_section_change_hook()	mips_enable_auto_align()
 
+#ifdef TE_IRIX
 enum dwarf2_format;
 extern enum dwarf2_format mips_dwarf2_format (void);
-#define DWARF2_FORMAT() mips_dwarf2_format ()
+# define DWARF2_FORMAT() mips_dwarf2_format ()
+#else
+/* Use GAS' defaults.  */
+#endif
 
 extern int mips_dwarf2_addr_size (void);
 #define DWARF2_ADDR_SIZE(bfd) mips_dwarf2_addr_size ()

Is this approach OK, or is there a real reason to have 64-bit DWARF
format on MIPS?  If it's OK, then I'll port the patch to binutils HEAD
((unused) asection * added to DWARF2_FORMAT).


Regards,
 Thomas

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: MIPS: 64-bit DWARF

[David Daney]

On 07/14/2010 01:50 AM, Thomas Schwinge wrote:
[...]
>
> Here is the patch that I tested for (a non-HEAD version of) binutils.
> This makes prelink happy, and there are no regressions in the GDB
> testsuite.  (Anything else to test?)  I left in the 64-bit stuff for
> TE_IRIX, as I have no idea about it.
>

How was it tested?  Have you tested mips64-linux-gnu with -mabi=64?

Thanks,
David Daney


> Index: gas/config/tc-mips.c
> ===================================================================
> --- gas/config/tc-mips.c	(revision 291480)
> +++ gas/config/tc-mips.c	(working copy)
> @@ -15499,20 +15499,16 @@
>   #endif
>   }
>
> +#ifdef TE_IRIX
>   enum dwarf2_format
>   mips_dwarf2_format (void)
>   {
>     if (HAVE_64BIT_SYMBOLS)
> -    {
> -#ifdef TE_IRIX
> -      return dwarf2_format_64bit_irix;
> -#else
> -      return dwarf2_format_64bit;
> -#endif
> -    }
> +    return dwarf2_format_64bit_irix;
>     else
>       return dwarf2_format_32bit;
>   }
> +#endif
>
>   int
>   mips_dwarf2_addr_size (void)
> Index: gas/config/tc-mips.h
> ===================================================================
> --- gas/config/tc-mips.h	(revision 291480)
> +++ gas/config/tc-mips.h	(working copy)
> @@ -165,9 +165,13 @@
>   extern void mips_enable_auto_align (void);
>   #define md_elf_section_change_hook()	mips_enable_auto_align()
>
> +#ifdef TE_IRIX
>   enum dwarf2_format;
>   extern enum dwarf2_format mips_dwarf2_format (void);
> -#define DWARF2_FORMAT() mips_dwarf2_format ()
> +# define DWARF2_FORMAT() mips_dwarf2_format ()
> +#else
> +/* Use GAS' defaults.  */
> +#endif
>
>   extern int mips_dwarf2_addr_size (void);
>   #define DWARF2_ADDR_SIZE(bfd) mips_dwarf2_addr_size ()
>
> Is this approach OK, or is there a real reason to have 64-bit DWARF
> format on MIPS?  If it's OK, then I'll port the patch to binutils HEAD
> ((unused) asection * added to DWARF2_FORMAT).
>
>
> Regards,
>   Thomas

Re: MIPS: 64-bit DWARF

[Maciej W. Rozycki]

On Wed, 14 Jul 2010, David Daney wrote:

> > Here is the patch that I tested for (a non-HEAD version of) binutils.
> > This makes prelink happy, and there are no regressions in the GDB
> > testsuite.  (Anything else to test?)  I left in the 64-bit stuff for
> > TE_IRIX, as I have no idea about it.
> 
> How was it tested?  Have you tested mips64-linux-gnu with -mabi=64?

 GCC switched to emitting 32-bit DWARF records on the MIPS/Linux/n64 
platform a couple of years ago.  GDB as of 6.8 didn't cope at all with 
64-bit DWARF records that were generated for n64 binaries before the 
switch (for the MIPS platform, that is -- I'd expect it to work for some 
others, especially ones that have always been 64-bit like the Alpha) -- 
the usual symptom was a complete exhaustion of the stack space followed by 
a crash (tested natively only).  I am strongly convinced GAS should 
follow, for consistency if nothing else, and I am rather surprised both 
tools were not updated at once.

 TE_IRIX refers to the original 64-bit DWARF stuff invented by SGI that 
predates DWARF-3 and differs slightly from the latter.  This may perhaps 
be the reason of the GDB crash -- it may have this SGI variant hardcoded 
for the MIPS target or suchlike.  Having noticed the GCC switch I have not 
investigated it further.

 The change needs to be properly tested of course to see if it does not 
uncover an odd bug somewhere, but in principle I consider it the right and 
necessary move.

  Maciej

Re: MIPS: 64-bit DWARF

[Thomas Schwinge]

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

Hello!

On 2010-07-14 18:44, Maciej W. Rozycki wrote:
> On Wed, 14 Jul 2010, David Daney wrote:
>> > Here is the patch that I tested for (a non-HEAD version of) binutils.
>> > This makes prelink happy, and there are no regressions in the GDB
>> > testsuite.  (Anything else to test?)  I left in the 64-bit stuff for
>> > TE_IRIX, as I have no idea about it.
>> 
>> How was it tested?

I developed / applied the patch using a tree with non-HEAD sources, and
(re-)built a complete mips-linux-gnu toolchain (GCC has been built with
``--enable-mips-nonpic --with-arch32=4kc --with-arch64=5kf''), then ran
all binutils and GDB regression test suites for the following multilibs
/ switches, and compared to baseline sources:

    ".;"
    ".;@mabi=64"
    "mel;@mel"
    "hard-float;@mhard-float"
    "hard-float;@mhard-float@mabi=n32"
    "hard-float;@mhard-float@mabi=64"
    "hard-float/vr5500;@mhard-float@march=vr5500"
    "mel/hard-float;@mel@mhard-float"
    "mel/hard-float;@mel@mhard-float@mabi=n32"
    "mel/hard-float;@mel@mhard-float@mabi=64"
    "octeon;@march=octeon@mabi=n32"
    "octeon;@march=octeon@mabi=64"
    "octeon2;@march=octeon2@mabi=n32"
    "octeon2;@march=octeon2@mabi=64"
    "uclibc;@muclibc"
    "uclibc/mel;@muclibc@mel"

>> Have you tested mips64-linux-gnu with -mabi=64?

I've now running the same testing on HEAD sources for a
mips64el-linux-gnu toolchain for:

        ".;"
        ".;@mabi=32"
        ".;@mabi=64"

I'll post the results once that's finished.

>  The change needs to be properly tested of course to see if it does not 
> uncover an odd bug somewhere, but in principle I consider it the right and 
> necessary move.

Is there anything else I should be testing?


Regards,
 Thomas

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: MIPS: 64-bit DWARF

[Richard Sandiford]

Thomas Schwinge <thomas@codesourcery.com> writes:
> Here is the patch that I tested for (a non-HEAD version of) binutils.
> This makes prelink happy, and there are no regressions in the GDB
> testsuite.  (Anything else to test?)  I left in the 64-bit stuff for
> TE_IRIX, as I have no idea about it.

Patch is OK with the testing you described downstream, and with a
suitable changelog.  Just to be sure, would you mind checking the
"binutils" (including gas and ld) testresults for both mips64-linux-gnu
and mips64el-linux-gnu?  There's no need to test multilibs; just run
both without any test options.

Thanks,
Richard

Re: MIPS: 64-bit DWARF

[Thomas Schwinge]

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

On 2010-07-19 20:00, Richard Sandiford wrote:
> Thomas Schwinge <thomas@codesourcery.com> writes:
>> Here is the patch that I tested for (a non-HEAD version of) binutils.
>> This makes prelink happy, and there are no regressions in the GDB
>> testsuite.  (Anything else to test?)  I left in the 64-bit stuff for
>> TE_IRIX, as I have no idea about it.
>
> Patch is OK with the testing you described downstream, and with a
> suitable changelog.  Just to be sure, would you mind checking the
> "binutils" (including gas and ld) testresults for both mips64-linux-gnu
> and mips64el-linux-gnu?  There's no need to test multilibs; just run
> both without any test options.

Done (had this in my testing schedule already, but forgot to say so); no
regressions.

2010-07-22  Thomas Schwinge  <thomas@codesourcery.com>

	Switch MIPS to 32-bit DWARF format.
	* config/tc-mips.h (DWARF2_FORMAT): Only define for [TE_IRIX].
	* config/tc-mips.c (mips_dwarf2_format): Likewise.

Committed.


Regards,
 Thomas

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]