RFA: Fix both_direction

RFA: Fix both_direction

[Daniel Jacobowitz]

This patch fixes "gdb -write" support for ELF files.  There are two
problems:

We try to write out the string table even if we didn't create one. If we've
bypassed the normal output routines, we never initialize elf_shstrtab, which
is otherwise OK.  If it's not set, there's nothing we need to do (except not
crash).

With that fixed, gdb -write works - as long as you change something, thereby
encountering the "output_has_begun = TRUE" in bfd_set_section_contents.  If
you don't, then the linker's output routines are called.  This causes
sections to get rearranged, destroys special program headers (like
PT_GNU_STACK), and loses the static symbol table.  I believe that the
check_format hook is a better place to handle this.

OK?  Tested on i686-linux, using the binutils testsuite, the gdb testsuite,
and manual testing of gdb -write (which needs some testsuite coverage of its
own).

-- 
Daniel Jacobowitz
CodeSourcery, LLC

2005-05-11  Daniel Jacobowitz  <dan@codesourcery.com>

	* elf.c (_bfd_elf_write_object_contents): Check for non-NULL
	elf_shstrtab.
	* format.c (bfd_check_format_matches): Set output_has_begun
	for both_direction.
	* section.c (bfd_set_section_contents): Use bfd_write_p.  Remove
	special case for both_direction.

Index: bfd/elf.c
===================================================================
RCS file: /cvs/src/src/bfd/elf.c,v
retrieving revision 1.291
diff -u -p -r1.291 elf.c
--- bfd/elf.c	7 May 2005 13:22:45 -0000	1.291
+++ bfd/elf.c	11 May 2005 15:55:51 -0000
@@ -4912,8 +4912,9 @@ _bfd_elf_write_object_contents (bfd *abf
     }
 
   /* Write out the section header names.  */
-  if (bfd_seek (abfd, elf_tdata (abfd)->shstrtab_hdr.sh_offset, SEEK_SET) != 0
-      || ! _bfd_elf_strtab_emit (abfd, elf_shstrtab (abfd)))
+  if (elf_shstrtab (abfd) != NULL
+      && (bfd_seek (abfd, elf_tdata (abfd)->shstrtab_hdr.sh_offset, SEEK_SET) != 0
+          || ! _bfd_elf_strtab_emit (abfd, elf_shstrtab (abfd))))
     return FALSE;
 
   if (bed->elf_backend_final_write_processing)
Index: bfd/format.c
===================================================================
RCS file: /cvs/src/src/bfd/format.c,v
retrieving revision 1.20
diff -u -p -r1.20 format.c
--- bfd/format.c	4 May 2005 15:53:31 -0000	1.20
+++ bfd/format.c	11 May 2005 15:55:51 -0000
@@ -173,6 +173,14 @@ bfd_check_format_matches (bfd *abfd, bfd
 	  if (matching)
 	    free (matching_vector);
 
+	  /* If the file was opened for update, then `output_has_begun'
+	     some time ago when the file was created.  Do not recompute
+	     sections sizes or alignments in _bfd_set_section_contents.
+	     We can not set this flag until after checking the format,
+	     because it will interfere with creation of BFD sections.  */
+	  if (abfd->direction == both_direction)
+	    abfd->output_has_begun = TRUE;
+
 	  return TRUE;			/* File position has moved, BTW.  */
 	}
 
@@ -319,6 +327,14 @@ bfd_check_format_matches (bfd *abfd, bfd
       if (matching)
 	free (matching_vector);
 
+      /* If the file was opened for update, then `output_has_begun'
+	 some time ago when the file was created.  Do not recompute
+	 sections sizes or alignments in _bfd_set_section_contents.
+	 We can not set this flag until after checking the format,
+	 because it will interfere with creation of BFD sections.  */
+      if (abfd->direction == both_direction)
+	abfd->output_has_begun = TRUE;
+
       return TRUE;			/* File position has moved, BTW.  */
     }
 
Index: bfd/section.c
===================================================================
RCS file: /cvs/src/src/bfd/section.c,v
retrieving revision 1.87
diff -u -p -r1.87 section.c
--- bfd/section.c	5 May 2005 14:34:04 -0000	1.87
+++ bfd/section.c	11 May 2005 15:55:52 -0000
@@ -1346,22 +1346,10 @@ bfd_set_section_contents (bfd *abfd,
       return FALSE;
     }
 
-  switch (abfd->direction)
+  if (!bfd_write_p (abfd))
     {
-    case read_direction:
-    case no_direction:
       bfd_set_error (bfd_error_invalid_operation);
       return FALSE;
-
-    case write_direction:
-      break;
-
-    case both_direction:
-      /* File is opened for update. `output_has_begun' some time ago when
-	   the file was created.  Do not recompute sections sizes or alignments
-	   in _bfd_set_section_content.  */
-      abfd->output_has_begun = TRUE;
-      break;
     }
 
   /* Record a copy of the data in memory if desired.  */

Re: RFA: Fix both_direction

[Daniel Jacobowitz]

On Wed, May 11, 2005 at 01:31:53PM -0400, Daniel Jacobowitz wrote:
> This patch fixes "gdb -write" support for ELF files.  There are two
> problems:
> 
> We try to write out the string table even if we didn't create one. If we've
> bypassed the normal output routines, we never initialize elf_shstrtab, which
> is otherwise OK.  If it's not set, there's nothing we need to do (except not
> crash).
> 
> With that fixed, gdb -write works - as long as you change something, thereby
> encountering the "output_has_begun = TRUE" in bfd_set_section_contents.  If
> you don't, then the linker's output routines are called.  This causes
> sections to get rearranged, destroys special program headers (like
> PT_GNU_STACK), and loses the static symbol table.  I believe that the
> check_format hook is a better place to handle this.
> 
> OK?  Tested on i686-linux, using the binutils testsuite, the gdb testsuite,
> and manual testing of gdb -write (which needs some testsuite coverage of its
> own).

> 2005-05-11  Daniel Jacobowitz  <dan@codesourcery.com>
> 
> 	* elf.c (_bfd_elf_write_object_contents): Check for non-NULL
> 	elf_shstrtab.
> 	* format.c (bfd_check_format_matches): Set output_has_begun
> 	for both_direction.
> 	* section.c (bfd_set_section_contents): Use bfd_write_p.  Remove
> 	special case for both_direction.

Ping?

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: RFA: Fix both_direction

[Alan Modra]

On Mon, May 16, 2005 at 10:28:28AM -0400, Daniel Jacobowitz wrote:
> On Wed, May 11, 2005 at 01:31:53PM -0400, Daniel Jacobowitz wrote:
> > This patch fixes "gdb -write" support for ELF files.  There are two
> > problems:
> > 
> > We try to write out the string table even if we didn't create one. If we've
> > bypassed the normal output routines, we never initialize elf_shstrtab, which
> > is otherwise OK.  If it's not set, there's nothing we need to do (except not
> > crash).
> > 
> > With that fixed, gdb -write works - as long as you change something, thereby
> > encountering the "output_has_begun = TRUE" in bfd_set_section_contents.  If
> > you don't, then the linker's output routines are called.  This causes
> > sections to get rearranged, destroys special program headers (like
> > PT_GNU_STACK), and loses the static symbol table.  I believe that the
> > check_format hook is a better place to handle this.
> > 
> > OK?  Tested on i686-linux, using the binutils testsuite, the gdb testsuite,
> > and manual testing of gdb -write (which needs some testsuite coverage of its
> > own).
> 
> > 2005-05-11  Daniel Jacobowitz  <dan@codesourcery.com>
> > 
> > 	* elf.c (_bfd_elf_write_object_contents): Check for non-NULL
> > 	elf_shstrtab.
> > 	* format.c (bfd_check_format_matches): Set output_has_begun
> > 	for both_direction.
> > 	* section.c (bfd_set_section_contents): Use bfd_write_p.  Remove
> > 	special case for both_direction.
> 
> Ping?

The idea looks OK to me.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre