[PATCH] Fix seg fault with --write PR gdb/20948

[Jozef Lawrynowicz <jozef dot l at mittosystems dot com>]

GDB segfaults when invoking it with the --write option, then quitting. First
reported in PR gdb/20948.

An assertion fails because elf_shstrtab is uninitialized, and 
elf_shstrtab is
only initialized if abfd_output_has_begun is FALSE.
bfd/format.c:bfd_check_format_matches as called from 
gdb/exec.c:exec_file_attach
always sets output_has_begun to TRUE if the bfd was opened for update, 
so the
attached patch sets output_has_begun back to FALSE in exec_file_attach 
when we
return from bfd_check_format_matches.

This leads to a further assertion failure in
bfd/elf.c:assign_file_positions_for_non_load_sections:

BFD_ASSERT (hdr->sh_offset == hdr->bfd_section->filepos);

filepos for non-load sections has been set already, but sh_offset is 0 as it
needs to be set by _bfd_elf_assign_file_position_for_section, which is 
called in
a further conditional block. So this first conditional has been extended to
evaluate to FALSE if sh_offset == 0 but filepos != 0.

The attached patche includes tests which verify that the --write behaviour
works as expected i.e. that modifications to the loaded executable persist
once the GDB session is ended.

For Unix and msp430-elf targets, completed testing for binutils, gas, 
ld, gdb,
sim (for msp430) without regressions.

If the patch is acceptable, I would appreciate if someone could commit 
it for
me as I don't have write access.

>From 5a9c30cac1edbdd8675025dc3faffa6e6544487d Mon Sep 17 00:00:00 2001
From: Jozef Lawrynowicz <jozef.l@mittosystems.com>
Date: Wed, 7 Mar 2018 16:57:15 +0000
Subject: [PATCH] Fix GDB segfault with --write

2018-03-08  Jozef Lawrynowicz <jozef.l@mittosystems.com>

Fix PR gdb/20948

bfd/
  * elf.c (assign_file_positions_for_non_load_sections): Allow hdr->sh_offset
    to be set by function if hdr->bfd_section->filepos != 0.

gdb/
  * exec.c (exec_file_attach): Set output_has_begun to FALSE for the bfd if
    write_files is TRUE.

gdb/testsuite/gdb.base/
  * write_mem.exp: New test.
  * write_mem.c: New test source file.
---
 bfd/elf.c                            |  9 ++++++++-
 gdb/exec.c                           |  4 ++++
 gdb/testsuite/gdb.base/write_mem.c   |  5 +++++
 gdb/testsuite/gdb.base/write_mem.exp | 31 +++++++++++++++++++++++++++++++
 4 files changed, 48 insertions(+), 1 deletion(-)
 create mode 100644 gdb/testsuite/gdb.base/write_mem.c
 create mode 100644 gdb/testsuite/gdb.base/write_mem.exp

diff --git a/bfd/elf.c b/bfd/elf.c
index 8ea5a81..b731e90 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -5736,7 +5736,14 @@ assign_file_positions_for_non_load_sections (bfd *abfd,
 
       hdr = *hdrpp;
       if (hdr->bfd_section != NULL
-	  && (hdr->bfd_section->filepos != 0
+	  && (((hdr->sh_flags & SHF_ALLOC) != 0
+	       && hdr->bfd_section->filepos != 0)
+	      /* If we haven't been called by the ELF linker, non load sections
+		 may have filepos>0 but sh_offset==0, so allow this function
+		 to set sh_offset.  */
+	      || ((hdr->sh_flags & SHF_ALLOC) == 0
+		  && hdr->bfd_section->filepos != 0
+		  && hdr->sh_offset != 0)
 	      || (hdr->sh_type == SHT_NOBITS
 		  && hdr->contents == NULL)))
 	BFD_ASSERT (hdr->sh_offset == hdr->bfd_section->filepos);
diff --git a/gdb/exec.c b/gdb/exec.c
index 6b910e8..c6662e0 100644
--- a/gdb/exec.c
+++ b/gdb/exec.c
@@ -351,6 +351,10 @@ exec_file_attach (const char *filename, int from_tty)
 		 gdb_bfd_errmsg (bfd_get_error (), matching));
 	}
 
+      /* bfd_check_format_matches assumes output_has_begun in all cases.  */
+      if (write_files)
+	exec_bfd->output_has_begun = FALSE;
+
       if (build_section_table (exec_bfd, &sections, &sections_end))
 	{
 	  /* Make sure to close exec_bfd, or else "run" might try to use
diff --git a/gdb/testsuite/gdb.base/write_mem.c b/gdb/testsuite/gdb.base/write_mem.c
new file mode 100644
index 0000000..b72eacd
--- /dev/null
+++ b/gdb/testsuite/gdb.base/write_mem.c
@@ -0,0 +1,5 @@
+int main (void)
+{
+  while (1);
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/write_mem.exp b/gdb/testsuite/gdb.base/write_mem.exp
new file mode 100644
index 0000000..558c61d
--- /dev/null
+++ b/gdb/testsuite/gdb.base/write_mem.exp
@@ -0,0 +1,31 @@
+global GDBFLAGS
+
+standard_testfile
+
+global srcdir
+global subdir
+
+if {[build_executable $testfile.exp $testfile \
+    $srcfile [list debug nowarnings] ] == -1} {
+	untested $testfile.exp
+	  return -1
+}
+
+clean_restart
+
+# Expect a failure before --write has been added to the command line
+test_print_reject "set {int}&main = 0x4242"
+
+set old_gdbflags $GDBFLAGS
+set GDBFLAGS "$old_gdbflags --write $binfile"
+clean_restart
+
+# Setting memory should now work correctly
+gdb_test_no_output "set {int}&main = 0x4242"
+
+# Check that memory write persists after quitting GDB
+gdb_exit
+gdb_start
+gdb_test "x /xh main" "<main>:.*4242"
+
+set GDBFLAGS $old_gdbflags
-- 
2.7.4