aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTom de Vries <tdevries@suse.de>2024-10-30 13:30:51 +0100
committerTom de Vries <tdevries@suse.de>2024-10-30 13:30:51 +0100
commit35d53ce6429a5e822aff29803956eb008775ef15 (patch)
tree7e6dd2bdc05a645e412d0066c1f9e1eacee1cf68
parent5330d85af1dbf48156a35f3908571ba57aae5304 (diff)
downloadbinutils-35d53ce6429a5e822aff29803956eb008775ef15.zip
binutils-35d53ce6429a5e822aff29803956eb008775ef15.tar.gz
binutils-35d53ce6429a5e822aff29803956eb008775ef15.tar.bz2
[gdb/tdep] Use std::array in amd64-windows-tdep.c
I noticed commit 84786372e1c ("Fix size of register buffer") fixing a stack-buffer-overflow found by AddressSanitizer in amd64_windows_store_arg_in_reg: ... - gdb_byte buf[8]; + gdb_byte buf[16]; ... and wondered if we could have found this without AddressSanitizer. I realized that the problem is that this: ... gdb_byte buf[N]; ... regcache->cooked_write (regno, buf); ... is using the deprecated variant of cooked_write instead of the one using gdb::array_view: ... /* Transfer of pseudo-registers. */ void cooked_write (int regnum, gdb::array_view<const gdb_byte> src); /* Deprecated overload of the above. */ void cooked_write (int regnum, const gdb_byte *src); ... and consequently cooked_write does not know the size of buf. Fix this by using std::array, and likewise in other places in gdb/amd64-windows-tdep.c. In the process I fixed another out of bounds access here: ... gdb_byte imm16[2]; ... cache->prev_sp = cur_sp + extract_unsigned_integer (imm16, 4, byte_order); ... where we're reading 4 bytes from the 2-byte buffer imm16. Tested by rebuilding on x86_64-linux. Tested-By: Hannes Domani <ssbssa@yahoo.de>
-rw-r--r--gdb/amd64-windows-tdep.c51
1 files changed, 27 insertions, 24 deletions
diff --git a/gdb/amd64-windows-tdep.c b/gdb/amd64-windows-tdep.c
index 57dcc4f..df59a44 100644
--- a/gdb/amd64-windows-tdep.c
+++ b/gdb/amd64-windows-tdep.c
@@ -208,12 +208,15 @@ amd64_windows_store_arg_in_reg (struct regcache *regcache,
struct type *type = arg->type ();
const gdb_byte *valbuf = arg->contents ().data ();
/* We only set 8 bytes, buf if it's a XMM register, 16 bytes are read. */
- gdb_byte buf[16];
+ std::array<gdb_byte, 16> buf;
gdb_assert (type->length () <= 8);
- memset (buf, 0, sizeof buf);
- memcpy (buf, valbuf, std::min (type->length (), (ULONGEST) 8));
- regcache->cooked_write (regno, buf);
+ memset (buf.data (), 0, buf.size ());
+ memcpy (buf.data (), valbuf, std::min (type->length (), (ULONGEST) 8));
+ size_t reg_size = regcache_register_size (regcache, regno);
+ gdb_assert (reg_size <= buf.size ());
+ gdb::array_view<gdb_byte> view (buf);
+ regcache->cooked_write (regno, view.slice (0, reg_size));
}
/* Push the arguments for an inferior function call, and return
@@ -317,7 +320,7 @@ amd64_windows_push_dummy_call
function_call_return_method return_method, CORE_ADDR struct_addr)
{
enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
- gdb_byte buf[8];
+ std::array<gdb_byte, 8> buf;
/* Pass arguments. */
sp = amd64_windows_push_arguments (regcache, nargs, args, sp,
@@ -330,7 +333,7 @@ amd64_windows_push_dummy_call
register. */
const int arg_regnum = amd64_windows_dummy_call_integer_regs[0];
- store_unsigned_integer (buf, 8, byte_order, struct_addr);
+ store_unsigned_integer (buf, byte_order, struct_addr);
regcache->cooked_write (arg_regnum, buf);
}
@@ -340,11 +343,11 @@ amd64_windows_push_dummy_call
/* Store return address. */
sp -= 8;
- store_unsigned_integer (buf, 8, byte_order, bp_addr);
- write_memory (sp, buf, 8);
+ store_unsigned_integer (buf, byte_order, bp_addr);
+ write_memory (sp, buf.data (), buf.size ());
/* Update the stack pointer... */
- store_unsigned_integer (buf, 8, byte_order, sp);
+ store_unsigned_integer (buf, byte_order, sp);
regcache->cooked_write (AMD64_RSP_REGNUM, buf);
/* ...and fake a frame pointer. */
@@ -433,13 +436,13 @@ amd64_skip_main_prologue (struct gdbarch *gdbarch, CORE_ADDR pc)
target_read_memory (pc, &op, 1);
if (op == 0xe8)
{
- gdb_byte buf[4];
+ std::array<gdb_byte, 4> buf;
- if (target_read_memory (pc + 1, buf, sizeof buf) == 0)
+ if (target_read_memory (pc + 1, buf.data (), buf.size ()) == 0)
{
CORE_ADDR call_dest;
- call_dest = pc + 5 + extract_signed_integer (buf, 4, byte_order);
+ call_dest = pc + 5 + extract_signed_integer (buf, byte_order);
bound_minimal_symbol s = lookup_minimal_symbol_by_pc (call_dest);
if (s.minsym != NULL
&& s.minsym->linkage_name () != NULL
@@ -617,12 +620,12 @@ amd64_windows_frame_decode_epilogue (const frame_info_ptr &this_frame,
case 0xec:
{
/* jmp rel32 */
- gdb_byte rel32[4];
+ std::array<gdb_byte, 4> rel32;
CORE_ADDR npc;
- if (target_read_memory (pc + 1, rel32, 4) != 0)
+ if (target_read_memory (pc + 1, rel32.data (), rel32.size ()) != 0)
return -1;
- npc = pc + 5 + extract_signed_integer (rel32, 4, byte_order);
+ npc = pc + 5 + extract_signed_integer (rel32, byte_order);
/* If the jump is within the function, then this is not a marker,
otherwise this is a tail-call. */
@@ -632,13 +635,13 @@ amd64_windows_frame_decode_epilogue (const frame_info_ptr &this_frame,
case 0xc2:
{
/* ret n */
- gdb_byte imm16[2];
+ std::array<gdb_byte, 2> imm16;
- if (target_read_memory (pc + 1, imm16, 2) != 0)
+ if (target_read_memory (pc + 1, imm16.data (), imm16.size ()) != 0)
return -1;
cache->prev_rip_addr = cur_sp;
cache->prev_sp = cur_sp
- + extract_unsigned_integer (imm16, 4, byte_order);
+ + extract_unsigned_integer (imm16, byte_order);
return 1;
}
@@ -797,11 +800,11 @@ amd64_windows_frame_decode_insns (const frame_info_ptr &this_frame,
/* According to msdn:
If an FP reg is used, then any unwind code taking an offset must
only be used after the FP reg is established in the prolog. */
- gdb_byte buf[8];
+ std::array<gdb_byte, 8> buf;
int frreg = amd64_windows_w2gdb_regnum[frame_reg];
- get_frame_register (this_frame, frreg, buf);
- save_addr = extract_unsigned_integer (buf, 8, byte_order);
+ get_frame_register (this_frame, frreg, buf.data ());
+ save_addr = extract_unsigned_integer (buf, byte_order);
frame_debug_printf (" frame_reg=%s, val=%s",
gdbarch_register_name (gdbarch, frreg),
@@ -1084,7 +1087,7 @@ amd64_windows_frame_cache (const frame_info_ptr &this_frame, void **this_cache)
struct gdbarch *gdbarch = get_frame_arch (this_frame);
enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
struct amd64_windows_frame_cache *cache;
- gdb_byte buf[8];
+ std::array<gdb_byte, 8> buf;
CORE_ADDR pc;
CORE_ADDR unwind_info = 0;
@@ -1096,8 +1099,8 @@ amd64_windows_frame_cache (const frame_info_ptr &this_frame, void **this_cache)
/* Get current PC and SP. */
pc = get_frame_pc (this_frame);
- get_frame_register (this_frame, AMD64_RSP_REGNUM, buf);
- cache->sp = extract_unsigned_integer (buf, 8, byte_order);
+ get_frame_register (this_frame, AMD64_RSP_REGNUM, buf.data ());
+ cache->sp = extract_unsigned_integer (buf, byte_order);
cache->pc = pc;
/* If we can't find the unwind info, keep trying as though this is a