From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 10898 invoked by alias); 28 Apr 2006 17:12:04 -0000 Received: (qmail 10880 invoked by uid 22791); 28 Apr 2006 17:12:03 -0000 X-Spam-Check-By: sourceware.org Received: from nile.gnat.com (HELO nile.gnat.com) (205.232.38.5) by sourceware.org (qpsmtpd/0.31) with ESMTP; Fri, 28 Apr 2006 17:11:58 +0000 Received: from localhost (localhost [127.0.0.1]) by filtered-nile.gnat.com (Postfix) with ESMTP id 642F648CCBA for ; Fri, 28 Apr 2006 13:11:56 -0400 (EDT) Received: from nile.gnat.com ([127.0.0.1]) by localhost (nile.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id 22648-01-8 for ; Fri, 28 Apr 2006 13:11:56 -0400 (EDT) Received: from takamaka.act-europe.fr (s142-179-108-108.bc.hsia.telus.net [142.179.108.108]) by nile.gnat.com (Postfix) with ESMTP id CD2C248CC0F for ; Fri, 28 Apr 2006 13:11:55 -0400 (EDT) Received: by takamaka.act-europe.fr (Postfix, from userid 507) id D989A47E7F; Fri, 28 Apr 2006 10:11:54 -0700 (PDT) Date: Fri, 28 Apr 2006 17:12:00 -0000 From: Joel Brobecker To: gdb-patches@sources.redhat.com Subject: [RFC/RFA/i386] pb reading insns if breakpoints still inserted Message-ID: <20060428171154.GP17613@adacore.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="IJpNTDwzlM2Ie8A6" Content-Disposition: inline User-Agent: Mutt/1.4i Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2006-04/txt/msg00367.txt.bz2 --IJpNTDwzlM2Ie8A6 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-length: 1420 Hello, following a discussion that started in: http://sources.redhat.com/ml/gdb-patches/2006-04/msg00345.html Here is a patch that fixes the problem. In summary: When we do next/step operations, we end up parsing the top frame function prologue and creating a frame_info with it. Unfortunately, at that point, the breakpoints are still inserted and that causes the prologue analyzer to misinterpret the function prologue and consequently breaks unwinding a bit. The fix that has been suggested is to fix the i386 prologue analyzer to handle inserted breakpoints. This is what the new read_insn() function does. I've always been very bad with names, so suggestions are more than welcome. I also think that read_insn() might be better located in a more general area, such as beside the various read_memory routines for instance. But before moving this function, I thought I'd ask... 2006-04-28 Joel Brobecker * i386-tdep.c (read_insn): New function. (i386_follow_jump): Use read_insn to read instructions. (i386_analyze_struct_return): Likewise. (i386_skip_probe): Likewise. (i386_match_insn): Likewise. (i386_analyze_frame_setup): Likewise. (i386_analyze_register_saves): Likewise. (i386_skip_prologue): Likewise. Tested on i686-pc-cygwin. No regression. Testcase to follow shortly. OK to apply? Thanks, -- Joel --IJpNTDwzlM2Ie8A6 Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="bt.diff" Content-length: 4728 Index: i386-tdep.c =================================================================== RCS file: /cvs/src/src/gdb/i386-tdep.c,v retrieving revision 1.224 diff -u -p -r1.224 i386-tdep.c --- i386-tdep.c 17 Mar 2006 00:14:24 -0000 1.224 +++ i386-tdep.c 28 Apr 2006 17:07:00 -0000 @@ -80,6 +80,34 @@ static char *i386_mmx_names[] = static const int i386_num_mmx_regs = ARRAY_SIZE (i386_mmx_names); +/* Read LEN bytes of inferior memory at PC, and store them in BUF. + This function works even if breakpoints are still inserted in + memory. + + Note that the breakpoint instruction on i386 takes only one byte, + so being careful of inserted breakpoints is only necessary if the + first byte of the instruction is being read. This function introduces + a small overhead that can be avoided by doing a direct memory read if + the bytes being read do not contain the first byte of an instruction. */ + +static void +read_insn (CORE_ADDR pc, gdb_byte *buf, int len) +{ + struct frame_info *current_frame = NULL; + + if (target_has_registers && target_has_stack && target_has_memory) + current_frame = get_current_frame (); + + if (current_frame != NULL) + safe_frame_unwind_memory (current_frame, pc, buf, len); + else + { + /* If we don't have a frame, then breakpoints cannot have been + inserted, and hence a direct memory read will work. */ + read_memory (pc, buf, len); + } +} + static int i386_mmx_regnum_p (struct gdbarch *gdbarch, int regnum) { @@ -354,7 +382,7 @@ i386_follow_jump (CORE_ADDR pc) long delta = 0; int data16 = 0; - op = read_memory_unsigned_integer (pc, 1); + read_insn (pc, &op, 1); if (op == 0x66) { data16 = 1; @@ -420,12 +448,12 @@ i386_analyze_struct_return (CORE_ADDR pc if (current_pc <= pc) return pc; - op = read_memory_unsigned_integer (pc, 1); + read_insn (pc, &op, 1); if (op != 0x58) /* popl %eax */ return pc; - read_memory (pc + 1, buf, 4); + read_insn (pc + 1, buf, 4); if (memcmp (buf, proto1, 3) != 0 && memcmp (buf, proto2, 4) != 0) return pc; @@ -464,7 +492,7 @@ i386_skip_probe (CORE_ADDR pc) gdb_byte buf[8]; gdb_byte op; - op = read_memory_unsigned_integer (pc, 1); + read_insn (pc, &op, 1); if (op == 0x68 || op == 0x6a) { @@ -535,7 +563,7 @@ i386_match_insn (CORE_ADDR pc, struct i3 struct i386_insn *insn; gdb_byte op; - op = read_memory_unsigned_integer (pc, 1); + read_insn (pc, &op, 1); for (insn = skip_insns; insn->len > 0; insn++) { @@ -548,7 +576,7 @@ i386_match_insn (CORE_ADDR pc, struct i3 gdb_assert (insn->len > 1); gdb_assert (insn->len <= I386_MAX_INSN_LEN); - read_memory (pc + 1, buf, insn->len - 1); + read_insn (pc + 1, buf, insn->len - 1); for (i = 1; i < insn->len; i++) { if ((buf[i - 1] & insn->mask[i]) != insn->insn[i]) @@ -634,7 +662,7 @@ i386_analyze_frame_setup (CORE_ADDR pc, if (limit <= pc) return limit; - op = read_memory_unsigned_integer (pc, 1); + read_insn (pc, &op, 1); if (op == 0x55) /* pushl %ebp */ { @@ -669,7 +697,7 @@ i386_analyze_frame_setup (CORE_ADDR pc, if (limit <= pc + skip) return limit; - op = read_memory_unsigned_integer (pc + skip, 1); + read_insn (pc + skip, &op, 1); /* Check for `movl %esp, %ebp' -- can be written in two ways. */ switch (op) @@ -703,7 +731,7 @@ i386_analyze_frame_setup (CORE_ADDR pc, NOTE: You can't subtract a 16-bit immediate from a 32-bit reg, so we don't have to worry about a data16 prefix. */ - op = read_memory_unsigned_integer (pc, 1); + read_insn (pc, &op, 1); if (op == 0x83) { /* `subl' with 8-bit immediate. */ @@ -759,7 +787,7 @@ i386_analyze_register_saves (CORE_ADDR p offset -= cache->locals; for (i = 0; i < 8 && pc < current_pc; i++) { - op = read_memory_unsigned_integer (pc, 1); + read_insn (pc, &op, 1); if (op < 0x50 || op > 0x57) break; @@ -848,7 +876,7 @@ i386_skip_prologue (CORE_ADDR start_pc) for (i = 0; i < 6; i++) { - op = read_memory_unsigned_integer (pc + i, 1); + read_insn (pc + i, &op, 1); if (pic_pat[i] != op) break; } @@ -856,7 +884,7 @@ i386_skip_prologue (CORE_ADDR start_pc) { int delta = 6; - op = read_memory_unsigned_integer (pc + delta, 1); + read_insn (pc + delta, &op, 1); if (op == 0x89) /* movl %ebx, x(%ebp) */ { @@ -869,7 +897,7 @@ i386_skip_prologue (CORE_ADDR start_pc) else /* Unexpected instruction. */ delta = 0; - op = read_memory_unsigned_integer (pc + delta, 1); + read_insn (pc + delta, &op, 1); } /* addl y,%ebx */ --IJpNTDwzlM2Ie8A6--