[PATCH] restore NOSTRIP for .debug_frame on mips-irix

[PATCH] restore NOSTRIP for .debug_frame on mips-irix

[Olivier Hainque]

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

Hello,

GCC used to set NOSTRIP bit on .debug_frame and doesn't any more.

This is causing troubles when using the libexc system library e.g.
for frame unwinding purposes, because it expects a single .debug_frame
per executable, the section has NOSTRIP set in system objects and the
native linker doesn't merge those having the bit set together with
those not having it.

Typically, for

   << /* t.c */
      #include <libexc.h>

      int main (void)
      {
	trace_back_stack_and_print ();
	return 0;
      }
   >>

We see

   $ gcc -o t t.c -lexc

   $ elfdump -hv t | grep -A 1 debug_frame
   [37]   SHT_MIPS_DWARF      0          0x9bc0     0x4b0       .debug_frame
	  0          0        0x4        0          0x00000000
   --
   [39]   SHT_MIPS_DWARF      0          0xa5a6     0xa4        .debug_frame
	  0          0        0x1        0          0x08000000 NOSTRIP

   $ ./t
   libexc(158601884): FATAL ERROR update_obj_info: dwarf_elf_init failed
   for ./t -- (107) DW_DLE_DEBUG_FRAME_DUPLICATE  Only one .debug_frame
   section is allowed

The attached patch addresses this by overriding the default elf
asm_named_section implementation on irix for ".debug_frame" only, to
special case it in a similar fashion as before.

Proper behavior with GNU as requires a change there as well to
propagate the flags down into the object, but this is a separate
issue.

Bootstrapped and regression tested for languages=c,c++,ada on
mips-sgi-irix6.5.

Thanks in advance,

Olivier

2007-10-08  Olivier Hainque  <hainque@adacore.com>

	* config/mips/mips.c (irix_asm_named_section): New function.
	Handle .debug_frame specially and fallback to the default elf
	support otherwise.
	* config/mips/iris.h (TARGET_ASM_NAMED_SECTION): Define.




[-- Attachment #2: mips-debug_frame_flags.dif --]
[-- Type: text/plain, Size: 2720 bytes --]

Index: config/mips/iris.h
===================================================================
*** config/mips/iris.h	(revision 128810)
--- config/mips/iris.h	(working copy)
*************** along with GCC; see the file COPYING3.  
*** 60,65 ****
--- 60,71 ----
  #undef ASM_FINISH_DECLARE_OBJECT
  #define ASM_FINISH_DECLARE_OBJECT mips_finish_declare_object
  
+ /* System libraries like libexc expect mips specific flags to be set
+    on specific sections, such as NOSTRIP on .debug_frame.  The generic
+    elf circuitry doesn't know about them, so ...  */
+ #undef  TARGET_ASM_NAMED_SECTION
+ #define TARGET_ASM_NAMED_SECTION irix_asm_named_section
+ 
  /* The linker needs a space after "-o".  */
  #define SWITCHES_NEED_SPACES "o"
  
Index: config/mips/mips.c
===================================================================
*** config/mips/mips.c	(revision 128810)
--- config/mips/mips.c	(working copy)
*************** static void mips_output_mi_thunk (FILE *
*** 344,349 ****
--- 344,350 ----
  static section *mips_select_rtx_section (enum machine_mode, rtx,
  					 unsigned HOST_WIDE_INT);
  static section *mips_function_rodata_section (tree);
+ static void irix_asm_named_section (const char *, unsigned int, tree);
  static bool mips_in_small_data_p (const_tree);
  static bool mips_use_anchors_for_symbol_p (const_rtx);
  static int mips_fpr_return_fields (const_tree, tree *);
*************** mips_function_rodata_section (tree decl)
*** 9035,9040 ****
--- 9036,9074 ----
    return data_section;
  }
  
+ /* Implement TARGET_ASM_NAMED_SECTION.  Some sections need special bits
+    that the generic elf circuitry doesn't know about.  Handle them specially
+    here and fallback on the default code for the others.  */
+ 
+ #define SHF_MIPS_NOSTRIP 0x08000000
+ #define SHT_MIPS_DWARF   0x7000001e
+ 
+ static void
+ irix_asm_named_section (const char *name, unsigned int flags, tree decl)
+ {
+   unsigned int sh_type = 0, sh_flags = 0, sh_entsize = 0, sh_align = 0;
+   bool need_special_flags = false;
+ 
+   /* System libraries like libexc expect a single .debug_frame per
+      executable.  The section has NOSTRIP set in system objects and
+      the linker doesn't merge those having the bit set together with
+      those not having it, so make sure we set it as well.  */
+ 
+   if (strcmp (name, ".debug_frame") == 0)
+     {
+       need_special_flags = true;
+       sh_type = SHT_MIPS_DWARF;
+       sh_flags = SHF_MIPS_NOSTRIP;
+       sh_align = 4;
+     }
+ 
+   if (need_special_flags)
+     fprintf (asm_out_file, "\t.section %s,%#x,%#x,%u,%u\n",
+            name, sh_type, sh_flags, sh_entsize, sh_align);
+   else
+     default_elf_asm_named_section (name, flags, decl);
+ }
+ 

Re: [PATCH] restore NOSTRIP for .debug_frame on mips-irix

[Richard Sandiford]

Olivier Hainque <hainque@adacore.com> writes:
> Hello,
>
> GCC used to set NOSTRIP bit on .debug_frame and doesn't any more.
>
> This is causing troubles when using the libexc system library e.g.
> for frame unwinding purposes, because it expects a single .debug_frame
> per executable, the section has NOSTRIP set in system objects and the
> native linker doesn't merge those having the bit set together with
> those not having it.
>
> Typically, for
>
>    << /* t.c */
>       #include <libexc.h>
>
>       int main (void)
>       {
> 	trace_back_stack_and_print ();
> 	return 0;
>       }
>    >>
>
> We see
>
>    $ gcc -o t t.c -lexc
>
>    $ elfdump -hv t | grep -A 1 debug_frame
>    [37]   SHT_MIPS_DWARF      0          0x9bc0     0x4b0       .debug_frame
> 	  0          0        0x4        0          0x00000000
>    --
>    [39]   SHT_MIPS_DWARF      0          0xa5a6     0xa4        .debug_frame
> 	  0          0        0x1        0          0x08000000 NOSTRIP
>
>    $ ./t
>    libexc(158601884): FATAL ERROR update_obj_info: dwarf_elf_init failed
>    for ./t -- (107) DW_DLE_DEBUG_FRAME_DUPLICATE  Only one .debug_frame
>    section is allowed
>
> The attached patch addresses this by overriding the default elf
> asm_named_section implementation on irix for ".debug_frame" only, to
> special case it in a similar fashion as before.
>
> Proper behavior with GNU as requires a change there as well to
> propagate the flags down into the object, but this is a separate
> issue.

I'm not sure I agree.  If we need this bit set for correctness, I think
GAS should add it automatically when the GAS (rather than IRIX) .section
syntax is used.  GAS already knows that certain sections are special
in certain ways, and if proper behaviour requires a GAS change too,
I suppose users will have to upgrade to newer binutils either way.

Richard

Re: [PATCH] restore NOSTRIP for .debug_frame on mips-irix

[Olivier Hainque]

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

Hello Richard,

Richard Sandiford wrote:
> If we need this bit set for correctness, I think GAS should add it
> automatically when the GAS (rather than IRIX) .section syntax is
> used.

 The patch was simply modeled after what the compiler used to do.
 I agree that dealing with this in GAS looks cleaner.

> GAS already knows that certain sections are special in certain ways,

 Indeed.

> and if proper behaviour requires a GAS change too, I
> suppose users will have to upgrade to newer binutils either way.

 Actually, we had observed the behavior with 2.16, this led to a
 discussion with a patch suggestion at

    http://www.cygwin.com/ml/binutils/2005-02/msg00688.html

 The patch was approved but apparently didn't go in and I was
 assuming it was still needed.

 However, I just tried my patched compiler with untouched 2.17 and
 2.18 assemblers and both seem to propagate the extra NOSTRIP bit, so
 something else happened.

 Anyway, _bfd_mips_elf_fake_sections in bfd/elfxx-mips.c still really
 looks like the proper place to have this change as there is a lot of
 similar processing there already.

 The attached patch fixes the tiny case I posted, and I'd be happy to
 submit it to the binutils list after the required testing if you think
 is appropriate.

 Thanks for your feedback,

 Olivier


 

[-- Attachment #2: mips-debug_frame_nostrip.dif --]
[-- Type: text/plain, Size: 1008 bytes --]

*** bfd/elfxx-mips.c.ori	2007-10-10 09:28:42.715483416 -0400
--- bfd/elfxx-mips.c	2007-10-10 09:56:46.514369373 -0400
*************** _bfd_mips_elf_fake_sections (bfd *abfd, 
*** 5670,5676 ****
        hdr->sh_flags |= SHF_MIPS_NOSTRIP;
      }
    else if (CONST_STRNEQ (name, ".debug_"))
!     hdr->sh_type = SHT_MIPS_DWARF;
    else if (strcmp (name, ".MIPS.symlib") == 0)
      {
        hdr->sh_type = SHT_MIPS_SYMBOL_LIB;
--- 5670,5684 ----
        hdr->sh_flags |= SHF_MIPS_NOSTRIP;
      }
    else if (CONST_STRNEQ (name, ".debug_"))
!     {
!       hdr->sh_type = SHT_MIPS_DWARF;
! 
!       /* Irix facilities such as libexc expect a single .debug_frame
! 	 per executable, the system ones have NOSTRIP set and the linker
! 	 doesn't merge sections with different flags so ...  */
!       if (SGI_COMPAT (abfd) && CONST_STRNEQ (name, ".debug_frame"))
! 	hdr->sh_flags |= SHF_MIPS_NOSTRIP;
!     }
    else if (strcmp (name, ".MIPS.symlib") == 0)
      {
        hdr->sh_type = SHT_MIPS_SYMBOL_LIB;

Re: [PATCH] restore NOSTRIP for .debug_frame on mips-irix

[Richard Sandiford]

Olivier Hainque <hainque@adacore.com> writes:
> Hello Richard,
>
> Richard Sandiford wrote:
>> If we need this bit set for correctness, I think GAS should add it
>> automatically when the GAS (rather than IRIX) .section syntax is
>> used.
>
>  The patch was simply modeled after what the compiler used to do.
>  I agree that dealing with this in GAS looks cleaner.
>
>> GAS already knows that certain sections are special in certain ways,
>
>  Indeed.
>
>> and if proper behaviour requires a GAS change too, I
>> suppose users will have to upgrade to newer binutils either way.
>
>  Actually, we had observed the behavior with 2.16, this led to a
>  discussion with a patch suggestion at
>
>     http://www.cygwin.com/ml/binutils/2005-02/msg00688.html
>
>  The patch was approved but apparently didn't go in and I was
>  assuming it was still needed.
>
>  However, I just tried my patched compiler with untouched 2.17 and
>  2.18 assemblers and both seem to propagate the extra NOSTRIP bit, so
>  something else happened.

Ah, interesting.  I _very_ vaguely remember other patches related
to MIPS section flags, so perhaps one of those fixed it.  That's
not much help, sorry...

>  Anyway, _bfd_mips_elf_fake_sections in bfd/elfxx-mips.c still really
>  looks like the proper place to have this change as there is a lot of
>  similar processing there already.
>
>  The attached patch fixes the tiny case I posted, and I'd be happy to
>  submit it to the binutils list after the required testing if you think
>  is appropriate.

Thanks for the patch.  FWIW, it looks good to me, although I can't
approve it.

Richard

Re: [PATCH] restore NOSTRIP for .debug_frame on mips-irix

[Olivier Hainque]

Richard Sandiford wrote:
> I _very_ vaguely remember other patches related to MIPS section
> flags, so perhaps one of those fixed it.

 Sounds likely.

> That's not much help, sorry...

 No problem. We have a simple track to get the behavior we like
 eventually.
 
> Thanks for the patch.  FWIW, it looks good to me, although I can't
> approve it.

 Understood.

 I'll give it a whirl on our internal tree and testsuite, then see
 for a binutils submission.

 Thanks much for your feedback.

 Olivier