mirror of
https://sourceware.org/git/binutils-gdb.git
synced 2024-12-15 04:31:49 +08:00
Fix "gdb --write" with core files
If you load a core file into GDB with the --write option, or "set write on" (equivalent), and then poke memory expecting it to patch the core binary, you'll notice something odd -- the write seems to succeed, but in reality, it doesn't. The value you wrote doesn't persist. Like so: $ gdb -q --write -c testsuite/outputs/gdb.base/patch/gcore.test [New LWP 615986] Core was generated by `/home/pedro/gdb/build/gdb/testsuite/outputs/gdb.base/patch/patch'. Program terminated with signal SIGTRAP, Trace/breakpoint trap. #0 0x0000555555555131 in ?? () (gdb) p *(unsigned char *)0x0000555555555131 = 1 $1 = 1 '\001' (gdb) p *(unsigned char *)0x0000555555555131 $2 = 185 '\271' (gdb) Diffing hexdumps of before/after patching, reveals that a "0x1" was actually written somewhere in the file. The problem is that the "0x1" was written at the wrong offset in the file... That happens because _bfd_elf_set_section_contents does this to seek to the section's offset: pos = hdr->sh_offset + offset; if (bfd_seek (abfd, pos, SEEK_SET) != 0 || bfd_bwrite (location, count, abfd) != count) return false; ... and 'hdr->sh_offset' is zero, so we seek to just OFFSET, which is incorrect. The reason 'hdr->sh_offset' is zero is that kernel-generated core files normally don't even have a section header table (gdb-generated ones do, but that's more an accident than a feature), and indeed elf_core_file_p doesn't even try to read sections at all: /* Core files are simply standard ELF formatted files that partition the file using the execution view of the file (program header table) rather than the linking view. In fact, there is no section header table in a core file. The process status information (including the contents of the general register set) and the floating point register set are stored in a segment of type PT_NOTE. We handcraft a couple of extra bfd sections that allow standard bfd access to the general registers (.reg) and the floating point registers (.reg2). */ bfd_cleanup elf_core_file_p (bfd *abfd) Changing _bfd_elf_set_section_contents from: pos = hdr->sh_offset + offset; to: pos = section->filepos + offset; fixes it. If we do that however, the tail end of _bfd_elf_set_section_contents ends up as a copy of _bfd_generic_set_section_contents, so just call the latter, thus eliminating some duplicate code. New GDB testcase included, which exercises both patching an executable and patching a core file. Patching an executable already works without this fix, because in that case BFD reads in the sections table. Still, we had no testcase for that yet. In fact, we have no "set write on" testcases at all, this is the first one. Tested on x86-64 GNU/Linux, gdb, ld, binutils, and gas. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=18227 Change-Id: I0f49f58b48aabab2e269f2959b8fd8a7fe36fdce
This commit is contained in:
parent
31b15688c4
commit
169692ce6c
@ -9404,7 +9404,6 @@ _bfd_elf_set_section_contents (bfd *abfd,
|
||||
bfd_size_type count)
|
||||
{
|
||||
Elf_Internal_Shdr *hdr;
|
||||
file_ptr pos;
|
||||
|
||||
if (! abfd->output_has_begun
|
||||
&& ! _bfd_elf_compute_section_file_positions (abfd, NULL))
|
||||
@ -9457,12 +9456,8 @@ _bfd_elf_set_section_contents (bfd *abfd,
|
||||
return true;
|
||||
}
|
||||
|
||||
pos = hdr->sh_offset + offset;
|
||||
if (bfd_seek (abfd, pos, SEEK_SET) != 0
|
||||
|| bfd_bwrite (location, count, abfd) != count)
|
||||
return false;
|
||||
|
||||
return true;
|
||||
return _bfd_generic_set_section_contents (abfd, section,
|
||||
location, offset, count);
|
||||
}
|
||||
|
||||
bool
|
||||
|
26
gdb/testsuite/gdb.base/patch.c
Normal file
26
gdb/testsuite/gdb.base/patch.c
Normal file
@ -0,0 +1,26 @@
|
||||
/* Copyright 2022 Free Software Foundation, Inc.
|
||||
|
||||
This file is part of GDB.
|
||||
|
||||
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/>. */
|
||||
|
||||
/* Value != 0 so it doesn't ends up in .bss, which we wouldn't be able
|
||||
to patch. */
|
||||
int extern_global = 1;
|
||||
|
||||
int
|
||||
main()
|
||||
{
|
||||
return 0;
|
||||
}
|
107
gdb/testsuite/gdb.base/patch.exp
Normal file
107
gdb/testsuite/gdb.base/patch.exp
Normal file
@ -0,0 +1,107 @@
|
||||
# Copyright 2022 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/>.
|
||||
|
||||
# Test patching executables and core files, with "set write on".
|
||||
|
||||
standard_testfile
|
||||
|
||||
if {[build_executable "failed to prepare" $testfile $srcfile debug]} {
|
||||
return -1
|
||||
}
|
||||
|
||||
# Check that we can patch an executable.
|
||||
|
||||
with_test_prefix "exec" {
|
||||
clean_restart
|
||||
|
||||
gdb_test_no_output "set write on"
|
||||
|
||||
gdb_load $binfile
|
||||
|
||||
gdb_test "p extern_global" " = 1" "read original value"
|
||||
gdb_test "p extern_global = 2" " = 2" "modify value"
|
||||
gdb_test "p extern_global" " = 2" "value modified"
|
||||
|
||||
clean_restart $binfile
|
||||
|
||||
gdb_test "p extern_global" " = 2" "value modified persisted"
|
||||
}
|
||||
|
||||
# Check that we can patch a core file.
|
||||
|
||||
# Generate a core file.
|
||||
with_test_prefix "gcore" {
|
||||
clean_restart $binfile
|
||||
|
||||
if {![runto_main]} {
|
||||
return
|
||||
}
|
||||
|
||||
# Extract the value at PC, and add 1, letting it wrap if
|
||||
# necessary. This is the value we will poke into memory.
|
||||
set poke_value ""
|
||||
gdb_test_multiple "p/x (*(unsigned char *) \$pc) + 1" "compute poke value" {
|
||||
-re -wrap " = ($hex)" {
|
||||
set poke_value $expect_out(1,string)
|
||||
pass $gdb_test_name
|
||||
}
|
||||
}
|
||||
if {$poke_value == ""} {
|
||||
return
|
||||
}
|
||||
|
||||
set corefile [standard_output_file gcore.test]
|
||||
set core_supported [gdb_gcore_cmd "$corefile" "save a corefile"]
|
||||
if {!$core_supported} {
|
||||
return
|
||||
}
|
||||
}
|
||||
|
||||
# Load it into GDB with "set write on", and change the instruction at
|
||||
# PC.
|
||||
with_test_prefix "load core, write on" {
|
||||
# Don't load a binary, we want to make sure we're patching the
|
||||
# core, not the executable.
|
||||
clean_restart
|
||||
|
||||
gdb_test_no_output "set write on"
|
||||
|
||||
set core_loaded [gdb_core_cmd "$corefile" "re-load generated corefile"]
|
||||
if { $core_loaded == -1 } {
|
||||
return
|
||||
}
|
||||
|
||||
gdb_test "p/x *(unsigned char *) \$pc = $poke_value" \
|
||||
" = $poke_value" \
|
||||
"poke value"
|
||||
gdb_test "p/x *(unsigned char *) \$pc" \
|
||||
" = $poke_value" \
|
||||
"value modified"
|
||||
}
|
||||
|
||||
# Re-load it into a new GDB session, now with "set write off", and
|
||||
# confirm the value change persisted.
|
||||
with_test_prefix "re-load modified core" {
|
||||
# Don't load a binary, we want to make sure we've patched the
|
||||
# core, not the executable.
|
||||
clean_restart
|
||||
|
||||
set core_loaded [gdb_core_cmd "$corefile" "re-load generated corefile"]
|
||||
gdb_assert { $core_loaded != -1 } "core re-loaded"
|
||||
|
||||
gdb_test "p/x *(unsigned char *) \$pc" \
|
||||
" = $poke_value" \
|
||||
"value modified persisted"
|
||||
}
|
Loading…
Reference in New Issue
Block a user