Re: [PATCH] Fix PR gdb/20948: --write option to GDB causes segmentation fault

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

On 17/09/18 17:13, Tom Tromey wrote:
> Jozef> The PR is assigned to GDB, but the fix is to BFD, hence the
> Jozef> submission to binutils mailing list instead of GDB.
>
> Jozef> 	* gdb/testsuite/gdb.base/write_mem.exp: New test.
> Jozef> 	* gdb/testsuite/gdb.base/write_mem.c: Likewise.
>
> The ChangeLog entries have to be split into the appropriate directories.
>
> Jozef> diff --git a/gdb/testsuite/gdb.base/write_mem.c b/gdb/testsuite/gdb.base/write_mem.c
> Jozef> new file mode 100644
> Jozef> index 0000000..941f172
> Jozef> --- /dev/null
> Jozef> +++ b/gdb/testsuite/gdb.base/write_mem.c
> Jozef> @@ -0,0 +1,7 @@
> Jozef> +/* Test for PR gdb/20948.  */
>
> Both new gdb files need the GPL header.
> You can copy one from some other test file.
>
> Jozef> +if {[build_executable $testfile.exp $testfile \
> Jozef> +  $srcfile [list debug nowarnings] ] == -1} {
>
> The indentation of this line looks off to me.
> I would suggest checking other examples to see how it should look.
>
> This is ok with these things fixed.  Thank you for the patch.
>
> Tom

Thanks for the review, the updated patch is attached. I would appreciate 
it if someone would commit the patch for me, as I do not have write 
access. Jozef

>From 24fee9b2ce3881285f1b0a2e1bab5a2aa1e034e4 Mon Sep 17 00:00:00 2001
From: Jozef Lawrynowicz <jozef.l@mittosystems.com>
Date: Tue, 11 Sep 2018 22:56:36 +0100
Subject: [PATCH] Fix PR gdb/20948: --write option to GDB causes segmentation
 fault

When opening a BFD for update, as gdb --write does, modifications to
anything but the contents of sections is restricted.

Do not try to write back any ELF headers in this case.

bfd/ChangeLog:

2018-09-XX  Jozef Lawrynowicz  <jozef.l@mittosystems.com>

	PR gdb/20948
	* elf.c (_bfd_elf_write_object_contents): Return from function
	early if abfd->direction == both_direction.

gdb/testsuite/ChangeLog:

2018-09-XX  Jozef Lawrynowicz  <jozef.l@mittosystems.com>

	PR gdb/20948
	* gdb.base/write_mem.exp: New test.
	* gdb.base/write_mem.c: Likewise.
---
 bfd/elf.c                            | 12 +++++++++
 gdb/testsuite/gdb.base/write_mem.c   | 20 +++++++++++++++
 gdb/testsuite/gdb.base/write_mem.exp | 47 ++++++++++++++++++++++++++++++++++++
 3 files changed, 79 insertions(+)
 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 02d605c..5320ae2 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -6391,6 +6391,18 @@ _bfd_elf_write_object_contents (bfd *abfd)
   if (! abfd->output_has_begun
       && ! _bfd_elf_compute_section_file_positions (abfd, NULL))
     return FALSE;
+  /* Do not rewrite ELF data when the BFD has been opened for update.
+     abfd->output_has_begun was set to TRUE on opening, so creation of new
+     sections, and modification of existing section sizes was restricted.
+     This means the ELF header, program headers and section headers can't have
+     changed.
+     If the contents of any sections has been modified, then those changes have
+     already been written to the BFD.  */
+  else if (abfd->direction == both_direction)
+    {
+      BFD_ASSERT (abfd->output_has_begun);
+      return TRUE;
+    }
 
   i_shdrp = elf_elfsections (abfd);
 
diff --git a/gdb/testsuite/gdb.base/write_mem.c b/gdb/testsuite/gdb.base/write_mem.c
new file mode 100644
index 0000000..82d8c41
--- /dev/null
+++ b/gdb/testsuite/gdb.base/write_mem.c
@@ -0,0 +1,20 @@
+/* Copyright (C) 2018 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+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..db476e7
--- /dev/null
+++ b/gdb/testsuite/gdb.base/write_mem.exp
@@ -0,0 +1,47 @@
+# Copyright (C) 2018 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Contributed by Jozef Lawrynowicz (jozef.l@mittosystems.com)
+
+# Test for PR gdb/20948
+# Verify that invoking gdb with the --write argument works as expected
+
+global GDBFLAGS
+standard_testfile
+
+if {[build_executable $testfile.exp $testfile \
+	$srcfile [list debug nowarnings] ] == -1} {
+    untested $testfile.exp
+    return -1
+}
+
+set old_gdbflags $GDBFLAGS
+
+# Expect a failure before --write has been added to the command line
+set GDBFLAGS "$old_gdbflags $binfile"
+clean_restart
+test_print_reject "set {int}main = 0x4242" "Cannot access memory at address"
+
+# Setting memory should now work correctly after adding --write
+set GDBFLAGS "$old_gdbflags --write $binfile"
+clean_restart
+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