This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
[PATCH] record: factor out memory reads + debug output to a single function.
- From: Pedro Alves <palves at redhat dot com>
- To: gdb-patches at sourceware dot org
- Date: Fri, 20 Jul 2012 12:23:37 +0100
- Subject: [PATCH] record: factor out memory reads + debug output to a single function.
I was debugging something that caused a "Process record: error reading
memory..." warning, and found it hard to find _which_ place in the
code was printing that, because the same "if (target_read_memory
fails) print_that_same_debug_output" pattern appears in many places of
the recording support. This patch refactors things so we only have a
single place to set a breakpoint at, if necessary.
Most places are using
if (record_debug)
printf_unfiltered (_("Process record: error reading memory at "
a couple were unconditional printf_unfiltered calls,
one was:
if (record_debug)
fprintf_unfiltered (gdb_stdlog,
"Process record: error reading memory at "
and another was:
if (record_debug)
warning (_("Process record: error reading memory at "
I think that use of the different styles can actually be called a bug.
I'd argue that this message should not depend on record_debug, but if
it is, it should be "fprint_unfiltered (gdb_stdlog". In any case,
I've sticked with the most frequent variant in this patch.
Tested on x86_64 Fedora 17.
Applied.
2012-07-20 Pedro Alves <palves@redhat.com>
* i386-tdep.c (i386_record_modrm, i386_record_lea_modrm_addr)
(i386_process_record): Use record_read_memory.
* record.c (record_read_memory): New function.
(record_arch_list_add_mem, record_exec_insn): Use
record_read_memory.
* record.h (record_read_memory): Declare.
---
gdb/i386-tdep.c | 169 +++++++++++--------------------------------------------
gdb/record.c | 33 ++++++-----
gdb/record.h | 6 ++
3 files changed, 59 insertions(+), 149 deletions(-)
diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index 6a02906..9227245 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -3941,14 +3941,9 @@ i386_record_modrm (struct i386_record_s *irp)
{
struct gdbarch *gdbarch = irp->gdbarch;
- if (target_read_memory (irp->addr, &irp->modrm, 1))
- {
- if (record_debug)
- printf_unfiltered (_("Process record: error reading memory at "
- "addr %s len = 1.\n"),
- paddress (gdbarch, irp->addr));
- return -1;
- }
+ if (record_read_memory (gdbarch, irp->addr, &irp->modrm, 1))
+ return -1;
+
irp->addr++;
irp->mod = (irp->modrm >> 6) & 3;
irp->reg = (irp->modrm >> 3) & 7;
@@ -3982,14 +3977,8 @@ i386_record_lea_modrm_addr (struct i386_record_s *irp, uint64_t *addr)
if (base == 4)
{
havesib = 1;
- if (target_read_memory (irp->addr, &byte, 1))
- {
- if (record_debug)
- printf_unfiltered (_("Process record: error reading memory "
- "at addr %s len = 1.\n"),
- paddress (gdbarch, irp->addr));
- return -1;
- }
+ if (record_read_memory (gdbarch, irp->addr, &byte, 1))
+ return -1;
irp->addr++;
scale = (byte >> 6) & 3;
index = ((byte >> 3) & 7) | irp->rex_x;
@@ -4003,14 +3992,8 @@ i386_record_lea_modrm_addr (struct i386_record_s *irp, uint64_t *addr)
if ((base & 7) == 5)
{
base = 0xff;
- if (target_read_memory (irp->addr, buf, 4))
- {
- if (record_debug)
- printf_unfiltered (_("Process record: error reading "
- "memory at addr %s len = 4.\n"),
- paddress (gdbarch, irp->addr));
- return -1;
- }
+ if (record_read_memory (gdbarch, irp->addr, buf, 4))
+ return -1;
irp->addr += 4;
*addr = extract_signed_integer (buf, 4, byte_order);
if (irp->regmap[X86_RECORD_R8_REGNUM] && !havesib)
@@ -4018,26 +4001,14 @@ i386_record_lea_modrm_addr (struct i386_record_s *irp, uint64_t *addr)
}
break;
case 1:
- if (target_read_memory (irp->addr, buf, 1))
- {
- if (record_debug)
- printf_unfiltered (_("Process record: error reading memory "
- "at addr %s len = 1.\n"),
- paddress (gdbarch, irp->addr));
- return -1;
- }
+ if (record_read_memory (gdbarch, irp->addr, buf, 1))
+ return -1;
irp->addr++;
*addr = (int8_t) buf[0];
break;
case 2:
- if (target_read_memory (irp->addr, buf, 4))
- {
- if (record_debug)
- printf_unfiltered (_("Process record: error reading memory "
- "at addr %s len = 4.\n"),
- paddress (gdbarch, irp->addr));
- return -1;
- }
+ if (record_read_memory (gdbarch, irp->addr, buf, 4))
+ return -1;
*addr = extract_signed_integer (buf, 4, byte_order);
irp->addr += 4;
break;
@@ -4076,14 +4047,8 @@ i386_record_lea_modrm_addr (struct i386_record_s *irp, uint64_t *addr)
case 0:
if (irp->rm == 6)
{
- if (target_read_memory (irp->addr, buf, 2))
- {
- if (record_debug)
- printf_unfiltered (_("Process record: error reading "
- "memory at addr %s len = 2.\n"),
- paddress (gdbarch, irp->addr));
- return -1;
- }
+ if (record_read_memory (gdbarch, irp->addr, buf, 2))
+ return -1;
irp->addr += 2;
*addr = extract_signed_integer (buf, 2, byte_order);
irp->rm = 0;
@@ -4091,26 +4056,14 @@ i386_record_lea_modrm_addr (struct i386_record_s *irp, uint64_t *addr)
}
break;
case 1:
- if (target_read_memory (irp->addr, buf, 1))
- {
- if (record_debug)
- printf_unfiltered (_("Process record: error reading memory "
- "at addr %s len = 1.\n"),
- paddress (gdbarch, irp->addr));
- return -1;
- }
+ if (record_read_memory (gdbarch, irp->addr, buf, 1))
+ return -1;
irp->addr++;
*addr = (int8_t) buf[0];
break;
case 2:
- if (target_read_memory (irp->addr, buf, 2))
- {
- if (record_debug)
- printf_unfiltered (_("Process record: error reading memory "
- "at addr %s len = 2.\n"),
- paddress (gdbarch, irp->addr));
- return -1;
- }
+ if (record_read_memory (gdbarch, irp->addr, buf, 2))
+ return -1;
irp->addr += 2;
*addr = extract_signed_integer (buf, 2, byte_order);
break;
@@ -4360,14 +4313,8 @@ i386_process_record (struct gdbarch *gdbarch, struct regcache *regcache,
/* prefixes */
while (1)
{
- if (target_read_memory (ir.addr, &opcode8, 1))
- {
- if (record_debug)
- printf_unfiltered (_("Process record: error reading memory at "
- "addr %s len = 1.\n"),
- paddress (gdbarch, ir.addr));
- return -1;
- }
+ if (record_read_memory (gdbarch, ir.addr, &opcode8, 1))
+ return -1;
ir.addr++;
switch (opcode8) /* Instruction prefixes */
{
@@ -4458,14 +4405,8 @@ i386_process_record (struct gdbarch *gdbarch, struct regcache *regcache,
switch (opcode)
{
case 0x0f:
- if (target_read_memory (ir.addr, &opcode8, 1))
- {
- if (record_debug)
- printf_unfiltered (_("Process record: error reading memory at "
- "addr %s len = 1.\n"),
- paddress (gdbarch, ir.addr));
- return -1;
- }
+ if (record_read_memory (gdbarch, ir.addr, &opcode8, 1))
+ return -1;
ir.addr++;
opcode = (uint32_t) opcode8 | 0x0f00;
goto reswitch;
@@ -5130,40 +5071,22 @@ Do you want to stop the program?"),
ir.ot = ir.dflag + OT_WORD;
if (ir.aflag == 2)
{
- if (target_read_memory (ir.addr, buf, 8))
- {
- if (record_debug)
- printf_unfiltered (_("Process record: error reading "
- "memory at addr 0x%s len = 8.\n"),
- paddress (gdbarch, ir.addr));
- return -1;
- }
+ if (record_read_memory (gdbarch, ir.addr, buf, 8))
+ return -1;
ir.addr += 8;
addr = extract_unsigned_integer (buf, 8, byte_order);
}
else if (ir.aflag)
{
- if (target_read_memory (ir.addr, buf, 4))
- {
- if (record_debug)
- printf_unfiltered (_("Process record: error reading "
- "memory at addr 0x%s len = 4.\n"),
- paddress (gdbarch, ir.addr));
- return -1;
- }
+ if (record_read_memory (gdbarch, ir.addr, buf, 4))
+ return -1;
ir.addr += 4;
addr = extract_unsigned_integer (buf, 4, byte_order);
}
else
{
- if (target_read_memory (ir.addr, buf, 2))
- {
- if (record_debug)
- printf_unfiltered (_("Process record: error reading "
- "memory at addr 0x%s len = 2.\n"),
- paddress (gdbarch, ir.addr));
- return -1;
- }
+ if (record_read_memory (gdbarch, ir.addr, buf, 2))
+ return -1;
ir.addr += 2;
addr = extract_unsigned_integer (buf, 2, byte_order);
}
@@ -6135,14 +6058,8 @@ Do you want to stop the program?"),
break;
case 0x9b: /* fwait */
- if (target_read_memory (ir.addr, &opcode8, 1))
- {
- if (record_debug)
- printf_unfiltered (_("Process record: error reading memory at "
- "addr 0x%s len = 1.\n"),
- paddress (gdbarch, ir.addr));
- return -1;
- }
+ if (record_read_memory (gdbarch, ir.addr, &opcode8, 1))
+ return -1;
opcode = (uint32_t) opcode8;
ir.addr++;
goto reswitch;
@@ -6161,14 +6078,8 @@ Do you want to stop the program?"),
{
int ret;
uint8_t interrupt;
- if (target_read_memory (ir.addr, &interrupt, 1))
- {
- if (record_debug)
- printf_unfiltered (_("Process record: error reading memory "
- "at addr %s len = 1.\n"),
- paddress (gdbarch, ir.addr));
- return -1;
- }
+ if (record_read_memory (gdbarch, ir.addr, &interrupt, 1))
+ return -1;
ir.addr++;
if (interrupt != 0x80
|| tdep->i386_intx80_record == NULL)
@@ -6638,13 +6549,8 @@ Do you want to stop the program?"),
case 0x0f0f: /* 3DNow! data */
if (i386_record_modrm (&ir))
return -1;
- if (target_read_memory (ir.addr, &opcode8, 1))
- {
- printf_unfiltered (_("Process record: error reading memory at "
- "addr %s len = 1.\n"),
- paddress (gdbarch, ir.addr));
- return -1;
- }
+ if (record_read_memory (gdbarch, ir.addr, &opcode8, 1))
+ return -1;
ir.addr++;
switch (opcode8)
{
@@ -6911,13 +6817,8 @@ reswitch_prefix_add:
case 0xf20f38:
case 0x0f3a:
case 0x660f3a:
- if (target_read_memory (ir.addr, &opcode8, 1))
- {
- printf_unfiltered (_("Process record: error reading memory at "
- "addr %s len = 1.\n"),
- paddress (gdbarch, ir.addr));
- return -1;
- }
+ if (record_read_memory (gdbarch, ir.addr, &opcode8, 1))
+ return -1;
ir.addr++;
opcode = (uint32_t) opcode8 | opcode << 8;
goto reswitch_prefix_add;
diff --git a/gdb/record.c b/gdb/record.c
index e1a6121..ac55cdf 100644
--- a/gdb/record.c
+++ b/gdb/record.c
@@ -485,6 +485,20 @@ record_arch_list_add_reg (struct regcache *regcache, int regnum)
return 0;
}
+int
+record_read_memory (struct gdbarch *gdbarch,
+ CORE_ADDR memaddr, gdb_byte *myaddr,
+ ssize_t len)
+{
+ int ret = target_read_memory (memaddr, myaddr, len);
+
+ if (ret && record_debug)
+ printf_unfiltered (_("Process record: error reading memory "
+ "at addr %s len = %ld.\n"),
+ paddress (gdbarch, memaddr), (long) len);
+ return ret;
+}
+
/* Record the value of a region of memory whose address is ADDR and
length is LEN to record_arch_list. */
@@ -504,13 +518,8 @@ record_arch_list_add_mem (CORE_ADDR addr, int len)
rec = record_mem_alloc (addr, len);
- if (target_read_memory (addr, record_get_loc (rec), len))
+ if (record_read_memory (target_gdbarch, addr, record_get_loc (rec), len))
{
- if (record_debug)
- fprintf_unfiltered (gdb_stdlog,
- "Process record: error reading memory at "
- "addr = %s len = %d.\n",
- paddress (target_gdbarch, addr), len);
record_mem_release (rec);
return -1;
}
@@ -739,15 +748,9 @@ record_exec_insn (struct regcache *regcache, struct gdbarch *gdbarch,
paddress (gdbarch, entry->u.mem.addr),
entry->u.mem.len);
- if (target_read_memory (entry->u.mem.addr, mem, entry->u.mem.len))
- {
- entry->u.mem.mem_entry_not_accessible = 1;
- if (record_debug)
- warning (_("Process record: error reading memory at "
- "addr = %s len = %d."),
- paddress (gdbarch, entry->u.mem.addr),
- entry->u.mem.len);
- }
+ if (record_read_memory (gdbarch,
+ entry->u.mem.addr, mem, entry->u.mem.len))
+ entry->u.mem.mem_entry_not_accessible = 1;
else
{
if (target_write_memory (entry->u.mem.addr,
diff --git a/gdb/record.h b/gdb/record.h
index 84396e6..ee591ea 100644
--- a/gdb/record.h
+++ b/gdb/record.h
@@ -30,4 +30,10 @@ extern int record_arch_list_add_mem (CORE_ADDR addr, int len);
extern int record_arch_list_add_end (void);
extern struct cleanup *record_gdb_operation_disable_set (void);
+/* Wrapper for target_read_memory that prints a debug message if
+ reading memory fails. */
+extern int record_read_memory (struct gdbarch *gdbarch,
+ CORE_ADDR memaddr, gdb_byte *myaddr,
+ ssize_t len);
+
#endif /* _RECORD_H_ */