public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* MIPS, strip --only-keep-debug & an infinite loop
@ 2005-04-28 23:02 Mark Kettenis
  2005-04-29 12:30 ` Maciej W. Rozycki
  0 siblings, 1 reply; 19+ messages in thread
From: Mark Kettenis @ 2005-04-28 23:02 UTC (permalink / raw)
  To: binutils

While investigating one of the many FAILS in the GDB testsuite on
mips64-unknown-openbsd3.7, I disvovered that GDB was sitting in an
infinite loop while going over the .MIPS.options section of a loaded
file.  The file in question was created using `strip
--only-keep-debug' and was part of a test to check whether loading a
file with seperate debug info works.

I caught GDB in the following loop:

      l = contents;
      lend = contents + hdr->sh_size;
      while (l + sizeof (Elf_External_Options) <= lend)
	{
	  Elf_Internal_Options intopt;

	  [...]

	  l += intopt.size;
	}

with INTOPT.SIZE = 0.  This obviously isn't desirable and since we
cannot guarantee that the user will never pass us an input file that
causes this, we should do something about it.

The obvious solution seems to be to add something like:

         if (intopt.size == 0)
	   return FALSE;

Unfortunately, this makes GDB reject the input file, which makes
loading the seperate debug info impossible.

This is all caused by the fact that `strip --only-keep-debug' doesn't
really completely strip out the .MIPS.options section.  Instead, it
sets the on-disk size of the section to 0 and strips the data from the
file.  This makes BFD replace the contents with all zeroes when we try
to read them, which in turn causes INTOPT.SIZE = 0 in the loop
mentioned above.

The easiest way out of this is silently skip further processing of
.MIPS.options if INTOPT.size = 0 is encountered:

         if (intopt.size == 0)
	   break;

Is there a better way?

Mark

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: MIPS, strip --only-keep-debug & an infinite loop
  2005-04-28 23:02 MIPS, strip --only-keep-debug & an infinite loop Mark Kettenis
@ 2005-04-29 12:30 ` Maciej W. Rozycki
  2005-04-29 13:02   ` Thiemo Seufer
  2005-04-29 15:16   ` Mark Kettenis
  0 siblings, 2 replies; 19+ messages in thread
From: Maciej W. Rozycki @ 2005-04-29 12:30 UTC (permalink / raw)
  To: Mark Kettenis; +Cc: binutils

On Thu, 28 Apr 2005, Mark Kettenis wrote:

> This is all caused by the fact that `strip --only-keep-debug' doesn't
> really completely strip out the .MIPS.options section.  Instead, it
> sets the on-disk size of the section to 0 and strips the data from the
> file.  This makes BFD replace the contents with all zeroes when we try
> to read them, which in turn causes INTOPT.SIZE = 0 in the loop
> mentioned above.

 Hmm, it looks like a problem with "strip" in the first place -- 
".MIPS.options" isn't meant to be touched by that tool.

  Maciej

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: MIPS, strip --only-keep-debug & an infinite loop
  2005-04-29 12:30 ` Maciej W. Rozycki
@ 2005-04-29 13:02   ` Thiemo Seufer
  2005-04-29 13:06     ` Maciej W. Rozycki
  2005-04-29 15:16   ` Mark Kettenis
  1 sibling, 1 reply; 19+ messages in thread
From: Thiemo Seufer @ 2005-04-29 13:02 UTC (permalink / raw)
  To: Maciej W. Rozycki; +Cc: binutils

Maciej W. Rozycki wrote:
> On Thu, 28 Apr 2005, Mark Kettenis wrote:
> 
> > This is all caused by the fact that `strip --only-keep-debug' doesn't
> > really completely strip out the .MIPS.options section.  Instead, it
> > sets the on-disk size of the section to 0 and strips the data from the
> > file.  This makes BFD replace the contents with all zeroes when we try
> > to read them, which in turn causes INTOPT.SIZE = 0 in the loop
> > mentioned above.
> 
>  Hmm, it looks like a problem with "strip" in the first place -- 
> ".MIPS.options" isn't meant to be touched by that tool.

Well, for --only-keep-debug it should remove .MIPS.options.


Thiemo

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: MIPS, strip --only-keep-debug & an infinite loop
  2005-04-29 13:02   ` Thiemo Seufer
@ 2005-04-29 13:06     ` Maciej W. Rozycki
  2005-04-29 13:47       ` Thiemo Seufer
  0 siblings, 1 reply; 19+ messages in thread
From: Maciej W. Rozycki @ 2005-04-29 13:06 UTC (permalink / raw)
  To: Thiemo Seufer; +Cc: binutils

On Fri, 29 Apr 2005, Thiemo Seufer wrote:

> >  Hmm, it looks like a problem with "strip" in the first place -- 
> > ".MIPS.options" isn't meant to be touched by that tool.
> 
> Well, for --only-keep-debug it should remove .MIPS.options.

 The spec says the section is never to be stripped; it has that 
SHF_MIPS_NOSTRIP flag set for this purpose.  Given the section is meant to 
be used by run-time ELF loaders (both for static and dynamic binaries), 
that's quite a reasonable expectation.  Are there other (IRIX?) 
implementations that disagree with the spec in this respect?

  Maciej

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: MIPS, strip --only-keep-debug & an infinite loop
  2005-04-29 13:06     ` Maciej W. Rozycki
@ 2005-04-29 13:47       ` Thiemo Seufer
  2005-04-29 13:51         ` Daniel Jacobowitz
  0 siblings, 1 reply; 19+ messages in thread
From: Thiemo Seufer @ 2005-04-29 13:47 UTC (permalink / raw)
  To: Maciej W. Rozycki; +Cc: binutils

Maciej W. Rozycki wrote:
> On Fri, 29 Apr 2005, Thiemo Seufer wrote:
> 
> > >  Hmm, it looks like a problem with "strip" in the first place -- 
> > > ".MIPS.options" isn't meant to be touched by that tool.
> > 
> > Well, for --only-keep-debug it should remove .MIPS.options.
> 
>  The spec says the section is never to be stripped; it has that 
> SHF_MIPS_NOSTRIP flag set for this purpose.  Given the section is meant to 
> be used by run-time ELF loaders (both for static and dynamic binaries), 
> that's quite a reasonable expectation.  Are there other (IRIX?) 
> implementations that disagree with the spec in this respect?

No, for the ELF loader that's correct. I misunderstood the semantics
of --only-keep-debug. (Why isn't it just called --keep-debug?)


Thiemo

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: MIPS, strip --only-keep-debug & an infinite loop
  2005-04-29 13:47       ` Thiemo Seufer
@ 2005-04-29 13:51         ` Daniel Jacobowitz
  2005-04-29 14:06           ` Maciej W. Rozycki
  0 siblings, 1 reply; 19+ messages in thread
From: Daniel Jacobowitz @ 2005-04-29 13:51 UTC (permalink / raw)
  To: binutils

On Fri, Apr 29, 2005 at 03:01:55PM +0200, Thiemo Seufer wrote:
> Maciej W. Rozycki wrote:
> > On Fri, 29 Apr 2005, Thiemo Seufer wrote:
> > 
> > > >  Hmm, it looks like a problem with "strip" in the first place -- 
> > > > ".MIPS.options" isn't meant to be touched by that tool.

objcopy --only-keep-debug does the same thing to _every_ non-debug
section.  Take a look at the resulting files.

> > > Well, for --only-keep-debug it should remove .MIPS.options.
> > 
> >  The spec says the section is never to be stripped; it has that 
> > SHF_MIPS_NOSTRIP flag set for this purpose.  Given the section is meant to 
> > be used by run-time ELF loaders (both for static and dynamic binaries), 
> > that's quite a reasonable expectation.  Are there other (IRIX?) 
> > implementations that disagree with the spec in this respect?
> 
> No, for the ELF loader that's correct. I misunderstood the semantics
> of --only-keep-debug. (Why isn't it just called --keep-debug?)

I would not expect --keep-debug to remove non-debug sections, unlike
--only-keep-debug :-)

-- 
Daniel Jacobowitz
CodeSourcery, LLC

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: MIPS, strip --only-keep-debug & an infinite loop
  2005-04-29 13:51         ` Daniel Jacobowitz
@ 2005-04-29 14:06           ` Maciej W. Rozycki
  2005-04-29 14:07             ` Daniel Jacobowitz
  0 siblings, 1 reply; 19+ messages in thread
From: Maciej W. Rozycki @ 2005-04-29 14:06 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: binutils

On Fri, 29 Apr 2005, Daniel Jacobowitz wrote:

> objcopy --only-keep-debug does the same thing to _every_ non-debug
> section.  Take a look at the resulting files.

 Hard to do actually, as `objdump -h' hangs similarly... ;-)

 But I've tried with a different target and the result is weird -- all 
non-debugging sections are merged to overlap starting from the same file 
offset, keeping their sizes, VMA, etc. intact...  Program headers, if 
present, get adjusted accordingly.

> I would not expect --keep-debug to remove non-debug sections, unlike
> --only-keep-debug :-)

 Well, I'm confused about what to expect, especially as documentation 
further states:

`--only-keep-debug'
     Strip a file, removing any sections that would be stripped by
     `--strip-debug' and leaving the debugging sections.

Given what you write, I suppose s/would/would not/ should be applied to 
above.  Am I right?  Then the affected sections should get completely 
removed and not mangled like currently, avoiding the problem altogether.  
Removing ".MIPS.options" alongside ".text", ".data", etc. is of course 
something that's beyond the MIPS64 ELF spec and would be quite a 
reasonable action to perform.

  Maciej

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: MIPS, strip --only-keep-debug & an infinite loop
  2005-04-29 14:06           ` Maciej W. Rozycki
@ 2005-04-29 14:07             ` Daniel Jacobowitz
  2005-04-29 16:13               ` Maciej W. Rozycki
  0 siblings, 1 reply; 19+ messages in thread
From: Daniel Jacobowitz @ 2005-04-29 14:07 UTC (permalink / raw)
  To: Maciej W. Rozycki; +Cc: binutils

On Fri, Apr 29, 2005 at 02:47:51PM +0100, Maciej W. Rozycki wrote:
> On Fri, 29 Apr 2005, Daniel Jacobowitz wrote:
> 
> > objcopy --only-keep-debug does the same thing to _every_ non-debug
> > section.  Take a look at the resulting files.
> 
>  Hard to do actually, as `objdump -h' hangs similarly... ;-)

Readelf will be of more use.

>  But I've tried with a different target and the result is weird -- all 
> non-debugging sections are merged to overlap starting from the same file 
> offset, keeping their sizes, VMA, etc. intact...  Program headers, if 
> present, get adjusted accordingly.

You missed the vital bit.  They all become NOBITS sections.  The
original file still has their original contents; they are only in this
file because .symtab will have contents, and it references section
numbers.

> > I would not expect --keep-debug to remove non-debug sections, unlike
> > --only-keep-debug :-)
> 
>  Well, I'm confused about what to expect, especially as documentation 
> further states:
> 
> `--only-keep-debug'
>      Strip a file, removing any sections that would be stripped by
>      `--strip-debug' and leaving the debugging sections.
> 
> Given what you write, I suppose s/would/would not/ should be applied to 
> above.  Am I right?

Yes.

>  Then the affected sections should get completely 
> removed and not mangled like currently, avoiding the problem altogether.  
> Removing ".MIPS.options" alongside ".text", ".data", etc. is of course 
> something that's beyond the MIPS64 ELF spec and would be quite a 
> reasonable action to perform.

Not at all, because then we could not define debugging symbols in
.text.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: MIPS, strip --only-keep-debug & an infinite loop
  2005-04-29 12:30 ` Maciej W. Rozycki
  2005-04-29 13:02   ` Thiemo Seufer
@ 2005-04-29 15:16   ` Mark Kettenis
  1 sibling, 0 replies; 19+ messages in thread
From: Mark Kettenis @ 2005-04-29 15:16 UTC (permalink / raw)
  To: macro; +Cc: binutils

   Date: Fri, 29 Apr 2005 12:57:08 +0100 (BST)
   From: "Maciej W. Rozycki" <macro@linux-mips.org>

   On Thu, 28 Apr 2005, Mark Kettenis wrote:

   > This is all caused by the fact that `strip --only-keep-debug' doesn't
   > really completely strip out the .MIPS.options section.  Instead, it
   > sets the on-disk size of the section to 0 and strips the data from the
   > file.  This makes BFD replace the contents with all zeroes when we try
   > to read them, which in turn causes INTOPT.SIZE = 0 in the loop
   > mentioned above.

    Hmm, it looks like a problem with "strip" in the first place -- 
   ".MIPS.options" isn't meant to be touched by that tool.

Are you saying that .MIPS.options should be present in both the
stripped executable and the file containing the debug info?  That
doesn't seem to be possible since currently BFD presumes that a
section either contains debug information or doesn't.

Mark

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: MIPS, strip --only-keep-debug & an infinite loop
  2005-04-29 14:07             ` Daniel Jacobowitz
@ 2005-04-29 16:13               ` Maciej W. Rozycki
  2005-05-01  1:50                 ` Daniel Jacobowitz
  2005-05-17 23:20                 ` Mark Kettenis
  0 siblings, 2 replies; 19+ messages in thread
From: Maciej W. Rozycki @ 2005-04-29 16:13 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: binutils

On Fri, 29 Apr 2005, Daniel Jacobowitz wrote:

> > > objcopy --only-keep-debug does the same thing to _every_ non-debug
> > > section.  Take a look at the resulting files.
> > 
> >  Hard to do actually, as `objdump -h' hangs similarly... ;-)
> 
> Readelf will be of more use.

 Indeed.

> >  But I've tried with a different target and the result is weird -- all 
> > non-debugging sections are merged to overlap starting from the same file 
> > offset, keeping their sizes, VMA, etc. intact...  Program headers, if 
> > present, get adjusted accordingly.
> 
> You missed the vital bit.  They all become NOBITS sections.  The
> original file still has their original contents; they are only in this
> file because .symtab will have contents, and it references section
> numbers.

 That makes sense.  And with readelf I can see the problem now -- 
".MIPS.options" is left marked as SHT_MIPS_OPTIONS as opposed to 
SHT_NOBITS.  Since that section no longer contains anything relevant, it 
should be considered a bug.

> >  Then the affected sections should get completely 
> > removed and not mangled like currently, avoiding the problem altogether.  
> > Removing ".MIPS.options" alongside ".text", ".data", etc. is of course 
> > something that's beyond the MIPS64 ELF spec and would be quite a 
> > reasonable action to perform.
> 
> Not at all, because then we could not define debugging symbols in
> .text.

 So again it's bad wording.  How about this patch?

2005-04-29  Maciej W. Rozycki  <macro@linux-mips.org>

	* doc/binutils.texi (strip, objcopy): Clarify the description of 
	the "--strip-debug" option.  Fix a typo.

  Maciej

binutils-2.16.90-20050429-only-keep-debug.patch
Index: binutils/doc/binutils.texi
===================================================================
RCS file: /cvs/src/src/binutils/doc/binutils.texi,v
retrieving revision 1.73
diff -u -p -r1.73 binutils.texi
--- binutils/doc/binutils.texi	25 Apr 2005 09:23:24 -0000	1.73
+++ binutils/doc/binutils.texi	29 Apr 2005 14:54:00 -0000
@@ -1428,8 +1428,9 @@ Creates a .gnu_debuglink section which c
 and adds it to the output file.
 
 @item --only-keep-debug
-Strip a file, removing any sections that would be stripped by
-@option{--strip-debug} and leaving the debugging sections.
+Strip a file, removing contents of any sections that would not be
+stripped by @option{--strip-debug} and leaving the debugging sections
+intact.
 
 The intention is that this option will be used in conjunction with
 @option{--add-gnu-debuglink} to create a two part executable.  One a
@@ -1460,7 +1461,7 @@ optional.  You could instead do this:
 @item Run @code{objcopy --add-gnu-debuglink=foo.full foo}
 @end enumerate
 
-ie the file pointed to by the @option{--add-gnu-debuglink} can be the
+i.e. the file pointed to by the @option{--add-gnu-debuglink} can be the
 full executable.  It does not have to be a file created by the
 @option{--only-keep-debug} switch.
 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: MIPS, strip --only-keep-debug & an infinite loop
  2005-04-29 16:13               ` Maciej W. Rozycki
@ 2005-05-01  1:50                 ` Daniel Jacobowitz
  2005-05-01 16:16                   ` Maciej W. Rozycki
  2005-05-17 23:20                 ` Mark Kettenis
  1 sibling, 1 reply; 19+ messages in thread
From: Daniel Jacobowitz @ 2005-05-01  1:50 UTC (permalink / raw)
  To: Maciej W. Rozycki; +Cc: binutils

On Fri, Apr 29, 2005 at 04:16:27PM +0100, Maciej W. Rozycki wrote:
> > >  But I've tried with a different target and the result is weird -- all 
> > > non-debugging sections are merged to overlap starting from the same file 
> > > offset, keeping their sizes, VMA, etc. intact...  Program headers, if 
> > > present, get adjusted accordingly.
> > 
> > You missed the vital bit.  They all become NOBITS sections.  The
> > original file still has their original contents; they are only in this
> > file because .symtab will have contents, and it references section
> > numbers.
> 
>  That makes sense.  And with readelf I can see the problem now -- 
> ".MIPS.options" is left marked as SHT_MIPS_OPTIONS as opposed to 
> SHT_NOBITS.  Since that section no longer contains anything relevant, it 
> should be considered a bug.

Hmm... I'm not completely sure, but your explanation is definitely
plausible.  But BFD does a lot of things based on section name.  Will
it actually help if the section is SHT_NOBITS, if its name is still
.MIPS.options?

> > >  Then the affected sections should get completely 
> > > removed and not mangled like currently, avoiding the problem altogether.  
> > > Removing ".MIPS.options" alongside ".text", ".data", etc. is of course 
> > > something that's beyond the MIPS64 ELF spec and would be quite a 
> > > reasonable action to perform.
> > 
> > Not at all, because then we could not define debugging symbols in
> > .text.
> 
>  So again it's bad wording.  How about this patch?
> 
> 2005-04-29  Maciej W. Rozycki  <macro@linux-mips.org>
> 
> 	* doc/binutils.texi (strip, objcopy): Clarify the description of 
> 	the "--strip-debug" option.  Fix a typo.

This patch is OK.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: MIPS, strip --only-keep-debug & an infinite loop
  2005-05-01  1:50                 ` Daniel Jacobowitz
@ 2005-05-01 16:16                   ` Maciej W. Rozycki
  2005-05-01 16:17                     ` Daniel Jacobowitz
  0 siblings, 1 reply; 19+ messages in thread
From: Maciej W. Rozycki @ 2005-05-01 16:16 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: binutils

On Sat, 30 Apr 2005, Daniel Jacobowitz wrote:

> >  That makes sense.  And with readelf I can see the problem now -- 
> > ".MIPS.options" is left marked as SHT_MIPS_OPTIONS as opposed to 
> > SHT_NOBITS.  Since that section no longer contains anything relevant, it 
> > should be considered a bug.
> 
> Hmm... I'm not completely sure, but your explanation is definitely
> plausible.  But BFD does a lot of things based on section name.  Will
> it actually help if the section is SHT_NOBITS, if its name is still
> .MIPS.options?

 For MIPS/ELF the only place that seems to depend on names rather than the 
type is _bfd_mips_elf_fake_sections().  I can check how such a change 
would affect it.  As a last resort, given it's already considered a hack, 
another one could perhaps be added on top of it making these special 
sections lose their special type if SHT_NOBITS is already set and having 
no contents is unexpected and makes such a section useless.

  Maciej

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: MIPS, strip --only-keep-debug & an infinite loop
  2005-05-01 16:16                   ` Maciej W. Rozycki
@ 2005-05-01 16:17                     ` Daniel Jacobowitz
  0 siblings, 0 replies; 19+ messages in thread
From: Daniel Jacobowitz @ 2005-05-01 16:17 UTC (permalink / raw)
  To: binutils

On Sun, May 01, 2005 at 05:16:09PM +0100, Maciej W. Rozycki wrote:
> On Sat, 30 Apr 2005, Daniel Jacobowitz wrote:
> 
> > >  That makes sense.  And with readelf I can see the problem now -- 
> > > ".MIPS.options" is left marked as SHT_MIPS_OPTIONS as opposed to 
> > > SHT_NOBITS.  Since that section no longer contains anything relevant, it 
> > > should be considered a bug.
> > 
> > Hmm... I'm not completely sure, but your explanation is definitely
> > plausible.  But BFD does a lot of things based on section name.  Will
> > it actually help if the section is SHT_NOBITS, if its name is still
> > .MIPS.options?
> 
>  For MIPS/ELF the only place that seems to depend on names rather than the 
> type is _bfd_mips_elf_fake_sections().  I can check how such a change 
> would affect it.  As a last resort, given it's already considered a hack, 
> another one could perhaps be added on top of it making these special 
> sections lose their special type if SHT_NOBITS is already set and having 
> no contents is unexpected and makes such a section useless.

Either answer sounds reasonable to me :-)

-- 
Daniel Jacobowitz
CodeSourcery, LLC

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: MIPS, strip --only-keep-debug & an infinite loop
  2005-04-29 16:13               ` Maciej W. Rozycki
  2005-05-01  1:50                 ` Daniel Jacobowitz
@ 2005-05-17 23:20                 ` Mark Kettenis
  2005-05-18 13:36                   ` Maciej W. Rozycki
  1 sibling, 1 reply; 19+ messages in thread
From: Mark Kettenis @ 2005-05-17 23:20 UTC (permalink / raw)
  To: macro; +Cc: drow, binutils

   Date: Fri, 29 Apr 2005 16:16:27 +0100 (BST)
   From: "Maciej W. Rozycki" <macro@linux-mips.org>

   On Fri, 29 Apr 2005, Daniel Jacobowitz wrote:

   > >  But I've tried with a different target and the result is weird -- all 
   > > non-debugging sections are merged to overlap starting from the same file 
   > > offset, keeping their sizes, VMA, etc. intact...  Program headers, if 
   > > present, get adjusted accordingly.
   > 
   > You missed the vital bit.  They all become NOBITS sections.  The
   > original file still has their original contents; they are only in this
   > file because .symtab will have contents, and it references section
   > numbers.

    That makes sense.  And with readelf I can see the problem now -- 
   ".MIPS.options" is left marked as SHT_MIPS_OPTIONS as opposed to 
   SHT_NOBITS.  Since that section no longer contains anything relevant, it 
   should be considered a bug.

Hmm, this seems to have fallen between the cracks.  Anyway, here is a
patch that seems to work for me.  It seems to fix objcopy/strip
--only-keep-debug and avoids the infinite loop.  However, I do think
the SEC_HAS_CONTENTS check should probably be applied to other faked
sections too.

Mark


Index: ChangeLog
from  Mark Kettenis  <kettenis@gnu.org>

	* elfxx-mips.c (_bfd_mips_elf_section_processing): Return FALSE if
	option size is zero.
	(_bfd_mips_elf_section_from_shdr): Likewise.
	(_bfd_mips_elf_fake_sections): Only fake SHT_MIPS_OPTIONS if the
	section has contents.

Index: elfxx-mips.c
===================================================================
RCS file: /cvs/src/src/bfd/elfxx-mips.c,v
retrieving revision 1.138
diff -u -p -r1.138 elfxx-mips.c
--- elfxx-mips.c 7 May 2005 13:22:55 -0000 1.138
+++ elfxx-mips.c 17 May 2005 22:53:29 -0000
@@ -5017,6 +5017,8 @@ _bfd_mips_elf_section_processing (bfd *a
 	      if (bfd_bwrite (buf, 4, abfd) != 4)
 		return FALSE;
 	    }
+	  if (intopt.size == 0)
+	    return FALSE;
 	  l += intopt.size;
 	}
     }
@@ -5223,6 +5225,8 @@ _bfd_mips_elf_section_from_shdr (bfd *ab
 		 &intreg);
 	      elf_gp (abfd) = intreg.ri_gp_value;
 	    }
+	  if (intopt.size == 0)
+	    return FALSE;
 	  l += intopt.size;
 	}
       free (contents);
@@ -5313,7 +5317,8 @@ _bfd_mips_elf_fake_sections (bfd *abfd, 
       hdr->sh_flags |= SHF_MIPS_NOSTRIP;
       /* The sh_info field is set in final_write_processing.  */
     }
-  else if (MIPS_ELF_OPTIONS_SECTION_NAME_P (name))
+  else if (MIPS_ELF_OPTIONS_SECTION_NAME_P (name)
+	   && (sec->flags & SEC_HAS_CONTENTS))
     {
       hdr->sh_type = SHT_MIPS_OPTIONS;
       hdr->sh_entsize = 1;

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: MIPS, strip --only-keep-debug & an infinite loop
  2005-05-17 23:20                 ` Mark Kettenis
@ 2005-05-18 13:36                   ` Maciej W. Rozycki
  2005-05-18 17:32                     ` Mark Kettenis
  2005-05-23 20:51                     ` Maciej W. Rozycki
  0 siblings, 2 replies; 19+ messages in thread
From: Maciej W. Rozycki @ 2005-05-18 13:36 UTC (permalink / raw)
  To: Mark Kettenis; +Cc: drow, binutils

On Wed, 18 May 2005, Mark Kettenis wrote:

>     That makes sense.  And with readelf I can see the problem now -- 
>    ".MIPS.options" is left marked as SHT_MIPS_OPTIONS as opposed to 
>    SHT_NOBITS.  Since that section no longer contains anything relevant, it 
>    should be considered a bug.
> 
> Hmm, this seems to have fallen between the cracks.  Anyway, here is a
> patch that seems to work for me.  It seems to fix objcopy/strip
> --only-keep-debug and avoids the infinite loop.  However, I do think
> the SEC_HAS_CONTENTS check should probably be applied to other faked
> sections too.

 It hasn't really fallen -- it's just my lack of time, sigh...  Thanks for 
looking into it -- I'll see if I can test it before 2.16.1.

  Maciej

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: MIPS, strip --only-keep-debug & an infinite loop
  2005-05-18 13:36                   ` Maciej W. Rozycki
@ 2005-05-18 17:32                     ` Mark Kettenis
  2005-05-23 20:51                     ` Maciej W. Rozycki
  1 sibling, 0 replies; 19+ messages in thread
From: Mark Kettenis @ 2005-05-18 17:32 UTC (permalink / raw)
  To: macro; +Cc: mark.kettenis, drow, binutils

   Date: Wed, 18 May 2005 11:58:52 +0100 (BST)
   From: "Maciej W. Rozycki" <macro@linux-mips.org>

   On Wed, 18 May 2005, Mark Kettenis wrote:

   >     That makes sense.  And with readelf I can see the problem now -- 
   >    ".MIPS.options" is left marked as SHT_MIPS_OPTIONS as opposed to 
   >    SHT_NOBITS.  Since that section no longer contains anything relevant, it 
   >    should be considered a bug.
   > 
   > Hmm, this seems to have fallen between the cracks.  Anyway, here is a
   > patch that seems to work for me.  It seems to fix objcopy/strip
   > --only-keep-debug and avoids the infinite loop.  However, I do think
   > the SEC_HAS_CONTENTS check should probably be applied to other faked
   > sections too.

    It hasn't really fallen -- it's just my lack of time, sigh...  Thanks for 
   looking into it -- I'll see if I can test it before 2.16.1.

Ok, thanks.  But this doesn't feel like something you need to rush in.
Perhaps the safest thing is to only add the "return FALSE" bit to the
release branch.  That at least avoids the infinite loop.

Mark

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: MIPS, strip --only-keep-debug & an infinite loop
  2005-05-18 13:36                   ` Maciej W. Rozycki
  2005-05-18 17:32                     ` Mark Kettenis
@ 2005-05-23 20:51                     ` Maciej W. Rozycki
  2005-05-25 23:35                       ` Mark Kettenis
  2005-05-26 16:27                       ` Eric Christopher
  1 sibling, 2 replies; 19+ messages in thread
From: Maciej W. Rozycki @ 2005-05-23 20:51 UTC (permalink / raw)
  To: Mark Kettenis; +Cc: drow, binutils

On Wed, 18 May 2005, Maciej W. Rozycki wrote:

> > Hmm, this seems to have fallen between the cracks.  Anyway, here is a
> > patch that seems to work for me.  It seems to fix objcopy/strip
> > --only-keep-debug and avoids the infinite loop.  However, I do think
> > the SEC_HAS_CONTENTS check should probably be applied to other faked
> > sections too.
> 
>  It hasn't really fallen -- it's just my lack of time, sigh...  Thanks for 
> looking into it -- I'll see if I can test it before 2.16.1.

 Well, I've had a look at the patch and I found out just returning a 
failure is inadequate for these functions.  For 
_bfd_mips_elf_section_processing() it's simply ignored.  For 
_bfd_mips_elf_section_from_shdr() it leads to such a damaged object being 
completely rejected, which prevents doing `objdump' or fixing it with 
another run of `strip --only-keep-debug' and is probably an overkill.  In 
neither case any useful diagnostics is available.  Therefore I've 
rewritten these changes to provide some diagnostics and attempt to 
continue anyway.

 For _bfd_mips_elf_fake_sections() I agree appropriate handling of all 
special sections is desirable.

 This has been checked not to cause any regressions in the test suite run 
for mipsel-linux-gnu natively.

2005-05-23  Mark Kettenis  <kettenis@gnu.org>
	    Maciej W. Rozycki  <macro@linux-mips.org>

	* elfxx-mips.c (_bfd_mips_elf_section_processing): Warn and
	stop processing of options if one of invalid size is
	encountered.
        (_bfd_mips_elf_section_from_shdr): Likewise.
        (_bfd_mips_elf_fake_sections): Reset the type of empty special
	sections.

 OK to apply?  Since it fixes a fatal problem, I'm asking for permission 
for 2.16.1, too.

  Maciej

binutils-2.16-mips-fake-sections.patch
diff -up --recursive --new-file binutils-2.16.macro/bfd/elfxx-mips.c binutils-2.16/bfd/elfxx-mips.c
--- binutils-2.16.macro/bfd/elfxx-mips.c	2005-04-19 18:37:04.000000000 +0000
+++ binutils-2.16/bfd/elfxx-mips.c	2005-05-22 06:08:02.000000000 +0000
@@ -4990,6 +4990,13 @@ _bfd_mips_elf_section_processing (bfd *a
 
 	  bfd_mips_elf_swap_options_in (abfd, (Elf_External_Options *) l,
 					&intopt);
+	  if (intopt.size < sizeof (Elf_External_Options))
+	    {
+	      (*_bfd_error_handler)
+		(_("%B: Warning: bad `%s' option size %u smaller than its header"),
+		abfd, MIPS_ELF_OPTIONS_SECTION_NAME (abfd), intopt.size);
+	      break;
+	    }
 	  if (ABI_64_P (abfd) && intopt.kind == ODK_REGINFO)
 	    {
 	      bfd_byte buf[8];
@@ -5202,6 +5209,13 @@ _bfd_mips_elf_section_from_shdr (bfd *ab
 
 	  bfd_mips_elf_swap_options_in (abfd, (Elf_External_Options *) l,
 					&intopt);
+	  if (intopt.size < sizeof (Elf_External_Options))
+	    {
+	      (*_bfd_error_handler)
+		(_("%B: Warning: bad `%s' option size %u smaller than its header"),
+		abfd, MIPS_ELF_OPTIONS_SECTION_NAME (abfd), intopt.size);
+	      break;
+	    }
 	  if (ABI_64_P (abfd) && intopt.kind == ODK_REGINFO)
 	    {
 	      Elf64_Internal_RegInfo intreg;
@@ -5240,8 +5254,10 @@ bfd_boolean
 _bfd_mips_elf_fake_sections (bfd *abfd, Elf_Internal_Shdr *hdr, asection *sec)
 {
   register const char *name;
+  unsigned int sh_type;
 
   name = bfd_get_section_name (abfd, sec);
+  sh_type = hdr->sh_type;
 
   if (strcmp (name, ".liblist") == 0)
     {
@@ -5343,6 +5359,12 @@ _bfd_mips_elf_fake_sections (bfd *abfd, 
       hdr->sh_entsize = 8;
     }
 
+  /* In the unlikely event a special section is empty it has to lose its
+     special meaning.  This may happen e.g. when using `strip' with the
+     "--only-keep-debug" option.  */
+  if (sec->size > 0 && !(sec->flags & SEC_HAS_CONTENTS))
+    hdr->sh_type = sh_type;
+
   /* The generic elf_fake_sections will set up REL_HDR using the default
    kind of relocations.  We used to set up a second header for the
    non-default kind of relocations here, but only NewABI would use

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: MIPS, strip --only-keep-debug & an infinite loop
  2005-05-23 20:51                     ` Maciej W. Rozycki
@ 2005-05-25 23:35                       ` Mark Kettenis
  2005-05-26 16:27                       ` Eric Christopher
  1 sibling, 0 replies; 19+ messages in thread
From: Mark Kettenis @ 2005-05-25 23:35 UTC (permalink / raw)
  To: Maciej W. Rozycki; +Cc: Mark Kettenis, drow, binutils

> On Wed, 18 May 2005, Maciej W. Rozycki wrote:
>  Well, I've had a look at the patch and I found out just returning a
> failure is inadequate for these functions.  For
> _bfd_mips_elf_section_processing() it's simply ignored.  For
> _bfd_mips_elf_section_from_shdr() it leads to such a damaged object being
> completely rejected, which prevents doing `objdump' or fixing it with
> another run of `strip --only-keep-debug' and is probably an overkill.  In
> neither case any useful diagnostics is available.  Therefore I've
> rewritten these changes to provide some diagnostics and attempt to
> continue anyway.
>
>  For _bfd_mips_elf_fake_sections() I agree appropriate handling of all
> special sections is desirable.
>
>  This has been checked not to cause any regressions in the test suite run
> for mipsel-linux-gnu natively.
>
> 2005-05-23  Mark Kettenis  <kettenis@gnu.org>
> 	    Maciej W. Rozycki  <macro@linux-mips.org>
>
> 	* elfxx-mips.c (_bfd_mips_elf_section_processing): Warn and
> 	stop processing of options if one of invalid size is
> 	encountered.
>         (_bfd_mips_elf_section_from_shdr): Likewise.
>         (_bfd_mips_elf_fake_sections): Reset the type of empty special
> 	sections.
>
>  OK to apply?  Since it fixes a fatal problem, I'm asking for permission
> for 2.16.1, too.

This looks fine to me.  Unfortunately I won't have the opportunity to test
this for a few weeks.  But I'm pretty confident it'll work.

Mark

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: MIPS, strip --only-keep-debug & an infinite loop
  2005-05-23 20:51                     ` Maciej W. Rozycki
  2005-05-25 23:35                       ` Mark Kettenis
@ 2005-05-26 16:27                       ` Eric Christopher
  1 sibling, 0 replies; 19+ messages in thread
From: Eric Christopher @ 2005-05-26 16:27 UTC (permalink / raw)
  To: Maciej W. Rozycki; +Cc: Mark Kettenis, drow, binutils


>  This has been checked not to cause any regressions in the test suite run 
> for mipsel-linux-gnu natively.
> 
> 2005-05-23  Mark Kettenis  <kettenis@gnu.org>
> 	    Maciej W. Rozycki  <macro@linux-mips.org>
> 
> 	* elfxx-mips.c (_bfd_mips_elf_section_processing): Warn and
> 	stop processing of options if one of invalid size is
> 	encountered.
>         (_bfd_mips_elf_section_from_shdr): Likewise.
>         (_bfd_mips_elf_fake_sections): Reset the type of empty special
> 	sections.
> 
>  OK to apply?  Since it fixes a fatal problem, I'm asking for permission 
> for 2.16.1, too.

OK for both.

thanks.

-eric

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2005-05-26 15:31 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2005-04-28 23:02 MIPS, strip --only-keep-debug & an infinite loop Mark Kettenis
2005-04-29 12:30 ` Maciej W. Rozycki
2005-04-29 13:02   ` Thiemo Seufer
2005-04-29 13:06     ` Maciej W. Rozycki
2005-04-29 13:47       ` Thiemo Seufer
2005-04-29 13:51         ` Daniel Jacobowitz
2005-04-29 14:06           ` Maciej W. Rozycki
2005-04-29 14:07             ` Daniel Jacobowitz
2005-04-29 16:13               ` Maciej W. Rozycki
2005-05-01  1:50                 ` Daniel Jacobowitz
2005-05-01 16:16                   ` Maciej W. Rozycki
2005-05-01 16:17                     ` Daniel Jacobowitz
2005-05-17 23:20                 ` Mark Kettenis
2005-05-18 13:36                   ` Maciej W. Rozycki
2005-05-18 17:32                     ` Mark Kettenis
2005-05-23 20:51                     ` Maciej W. Rozycki
2005-05-25 23:35                       ` Mark Kettenis
2005-05-26 16:27                       ` Eric Christopher
2005-04-29 15:16   ` Mark Kettenis

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).