From: Tom de Vries Date: Wed, 30 Oct 2024 12:30:51 +0000 (+0100) Subject: [gdb/tdep] Use std::array in amd64-windows-tdep.c X-Git-Tag: gdb-16-branchpoint~559 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=35d53ce6429a5e822aff29803956eb008775ef15;p=thirdparty%2Fbinutils-gdb.git [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 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 --- diff --git a/gdb/amd64-windows-tdep.c b/gdb/amd64-windows-tdep.c index 57dcc4f56bd..df59a44b824 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 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 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 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 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 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 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 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 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