From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 31318 invoked by alias); 25 Jun 2008 13:22:45 -0000 Received: (qmail 31307 invoked by uid 22791); 25 Jun 2008 13:22:43 -0000 X-Spam-Check-By: sourceware.org Received: from igw3.br.ibm.com (HELO igw3.br.ibm.com) (32.104.18.26) by sourceware.org (qpsmtpd/0.31) with ESMTP; Wed, 25 Jun 2008 13:22:20 +0000 Received: from mailhub3.br.ibm.com (unknown [9.18.232.110]) by igw3.br.ibm.com (Postfix) with ESMTP id 9182B3901D9 for ; Wed, 25 Jun 2008 10:05:18 -0300 (BRST) Received: from d24av01.br.ibm.com (d24av01.br.ibm.com [9.18.232.46]) by mailhub3.br.ibm.com (8.13.8/8.13.8/NCO v8.7) with ESMTP id m5PDMBmM4481048 for ; Wed, 25 Jun 2008 10:22:12 -0300 Received: from d24av01.br.ibm.com (loopback [127.0.0.1]) by d24av01.br.ibm.com (8.12.11.20060308/8.13.3) with ESMTP id m5PDM46P031024 for ; Wed, 25 Jun 2008 10:22:06 -0300 Received: from [9.8.2.174] ([9.8.2.174]) by d24av01.br.ibm.com (8.12.11.20060308/8.12.11) with ESMTP id m5PDM0FT030885 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Wed, 25 Jun 2008 10:22:01 -0300 Subject: Re: [PATCH] PPC - Stepping off breakpoints in non-stop mode From: Luis Machado Reply-To: luisgpm@linux.vnet.ibm.com To: Joel Brobecker Cc: Pedro Alves , Daniel Jacobowitz , gdb-patches@sourceware.org In-Reply-To: <20080625123144.GA3700@adacore.com> References: <1209753019.7131.29.camel@gargoyle> <20080605200249.GH25085@caradoc.them.org> <1212768014.10042.60.camel@gargoyle> <200806241503.59231.pedro@codesourcery.com> <1214317934.10496.18.camel@gargoyle> <20080625123144.GA3700@adacore.com> Content-Type: text/plain Date: Wed, 25 Jun 2008 13:35:00 -0000 Message-Id: <1214400118.10496.36.camel@gargoyle> Mime-Version: 1.0 X-Mailer: Evolution 2.12.1 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2008-06/txt/msg00428.txt.bz2 > Overall, this looks good to me, I just have a few questions before we > commit. Thanks much to Pedro for his informal review, btw. Thanks for looking into this guys. > Can we ditch the PPC_MAX_INSN_LEN macro and only use PPC_INSN_SIZE? > Right now, you define the latter but never use it. Right now, you only > use PPC_MAX_INSN_LEN, which I like less than PPC_INSN_SIZE, because > AFAIK all instructions on powerpc are the same size. Yes, this makes sense. I've changed the code to use only PPC_INSN_SIZE. > Why did you move these macros to the .h file? Right now, they are only > used inside rs6000-tdep.c, so we could keep them there. > Same for these macros, we could certainly keep them inside rs6000-tdep.c. > I'm not opposed to having them in the .h file, if you prefer it that > way. But I generally prefer to keep definitions inside the .c file if > they are not used elsewhere. That way, I instantly know that this macro > is only used within that unit. Defines and macros looked better for me in header files since they're almost always constant values, but i didn't think of it the way you mentioned. It's a good point. They're back to the .c file. > I don't think you need to use a gdb_byte in this case. Wouldn't it > be simpler to use an int? Hopefully, that will allow you to avoid > this cast. Otherwise, it looks like this constant is only used once > in your code: > > if (link_register_bit) > > So another alternative is is to embed it inside the "if" statement > with a comment. Something like this: Fixed as well. How does it look now? Thanks, Luis 2008-06-25 Luis Machado * rs6000-tdep.c (ppc_displaced_step_fixup): New function. (deal_with_atomic_sequence): Update BC masks. (rs6000_gdbarch_init): Init displaced stepping infra-structure. Define BRANCH_MASK, B_INSN, BC_INSN, BXL_INSN, BP_MASK and BP_INSN. Index: gdb/rs6000-tdep.c =================================================================== --- gdb.orig/rs6000-tdep.c 2008-06-23 05:13:22.000000000 -0700 +++ gdb/rs6000-tdep.c 2008-06-25 06:20:02.000000000 -0700 @@ -841,6 +841,101 @@ return little_breakpoint; } +/* Instruction masks for displaced stepping. */ +#define BRANCH_MASK 0xfc000000 +#define BP_MASK 0xFC0007FE +#define B_INSN 0x48000000 +#define BC_INSN 0x40000000 +#define BXL_INSN 0x4c000000 +#define BP_INSN 0x7C000008 + +/* Fix up the state of registers and memory after having single-stepped + a displaced instruction. */ +void +ppc_displaced_step_fixup (struct gdbarch *gdbarch, + struct displaced_step_closure *closure, + CORE_ADDR from, CORE_ADDR to, + struct regcache *regs) +{ + /* Since we use simple_displaced_step_copy_insn, our closure is a + copy of the instruction. */ + ULONGEST insn = extract_unsigned_integer ((gdb_byte *) closure, + PPC_INSN_SIZE); + ULONGEST opcode = 0; + /* Offset for non-PC relative instructions. */ + LONGEST offset = PPC_INSN_SIZE; + + opcode = insn & BRANCH_MASK; + + if (debug_displaced) + fprintf_unfiltered (gdb_stdlog, + "displaced: (ppc) fixup (0x%s, 0x%s)\n", + paddr_nz (from), paddr_nz (to)); + + + /* Handle PC-relative branch instructions. */ + if (opcode == B_INSN || opcode == BC_INSN || opcode == BXL_INSN) + { + CORE_ADDR current_pc; + + /* Read the current PC value after the instruction has been executed + in a displaced location. Calculate the offset to be applied to the + original PC value before the displaced stepping. */ + regcache_cooked_read_unsigned (regs, gdbarch_pc_regnum (gdbarch), + ¤t_pc); + offset = current_pc - to; + + if (opcode != BXL_INSN) + { + if (!(insn & 0x2)) + { + /* AA bit indicating whether this is an absolute addressing or + PC-relative. */ + if (debug_displaced) + fprintf_unfiltered (gdb_stdlog, + "displaced: (ppc) branch instruction: 0x%s\n" + "displaced: (ppc) adjusted PC from 0x%s to 0x%s\n", + paddr_nz (insn), paddr_nz (current_pc), + paddr_nz (from + offset)); + + regcache_cooked_write_unsigned (regs, gdbarch_pc_regnum (gdbarch), + from + offset); + } + } + else + { + /* If we're here, it means we have a branch to LR or CTR. If the + branch was taken, the offset is probably greater than 4 (the next + instruction), so it's safe to assume that an offset of 4 means we + did not take the branch. */ + if (offset == PPC_INSN_SIZE) + regcache_cooked_write_unsigned (regs, gdbarch_pc_regnum (gdbarch), + from + PPC_INSN_SIZE); + } + + if (insn & 0x1) + { + /* LK bit Indicates whether we should set the link register to point + to the next instruction or not. */ + regcache_cooked_write_unsigned (regs, + gdbarch_tdep (gdbarch)->ppc_lr_regnum, + from + PPC_INSN_SIZE); + if (debug_displaced) + fprintf_unfiltered (gdb_stdlog, + "displaced: (ppc) adjusted LR to 0x%s\n", + paddr_nz (from + PPC_INSN_SIZE)); + + } + } + /* Check for breakpoints in the inferior. If we've found one, place the PC + right at the breakpoint instruction. */ + else if ((insn & BP_MASK) == BP_INSN) + regcache_cooked_write_unsigned (regs, gdbarch_pc_regnum (gdbarch), from); + else + /* Handle any other instructions that do not fit in the categories above. */ + regcache_cooked_write_unsigned (regs, gdbarch_pc_regnum (gdbarch), + from + offset); +} /* Instruction masks used during single-stepping of atomic sequences. */ #define LWARX_MASK 0xfc0007fe @@ -849,8 +944,6 @@ #define STWCX_MASK 0xfc0007ff #define STWCX_INSTRUCTION 0x7c00012d #define STDCX_INSTRUCTION 0x7c0001ad -#define BC_MASK 0xfc000000 -#define BC_INSTRUCTION 0x40000000 /* Checks for an atomic sequence of instructions beginning with a LWARX/LDARX instruction and ending with a STWCX/STDCX instruction. If such a sequence @@ -887,7 +980,7 @@ /* Assume that there is at most one conditional branch in the atomic sequence. If a conditional branch is found, put a breakpoint in its destination address. */ - if ((insn & BC_MASK) == BC_INSTRUCTION) + if ((insn & BRANCH_MASK) == BC_INSN) { int immediate = ((insn & ~3) << 16) >> 16; int absolute = ((insn >> 1) & 1); @@ -3214,6 +3307,17 @@ /* Put the _Decimal128 pseudo-registers after the SPE registers. */ tdep->ppc_dl0_regnum += 32; + /* Setup displaced stepping. */ + set_gdbarch_displaced_step_copy_insn (gdbarch, + simple_displaced_step_copy_insn); + set_gdbarch_displaced_step_fixup (gdbarch, ppc_displaced_step_fixup); + set_gdbarch_displaced_step_free_closure (gdbarch, + simple_displaced_step_free_closure); + set_gdbarch_displaced_step_location (gdbarch, + displaced_step_at_entry_point); + + set_gdbarch_max_insn_length (gdbarch, PPC_INSN_SIZE); + return gdbarch; }