From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 3857 invoked by alias); 2 May 2007 14:08:34 -0000 Received: (qmail 3849 invoked by uid 22791); 2 May 2007 14:08:33 -0000 X-Spam-Check-By: sourceware.org Received: from mtagate7.de.ibm.com (HELO mtagate7.de.ibm.com) (195.212.29.156) by sourceware.org (qpsmtpd/0.31) with ESMTP; Wed, 02 May 2007 14:08:30 +0000 Received: from d12nrmr1607.megacenter.de.ibm.com (d12nrmr1607.megacenter.de.ibm.com [9.149.167.49]) by mtagate7.de.ibm.com (8.13.8/8.13.8) with ESMTP id l42E8SIR176044 for ; Wed, 2 May 2007 14:08:28 GMT Received: from d12av02.megacenter.de.ibm.com (d12av02.megacenter.de.ibm.com [9.149.165.228]) by d12nrmr1607.megacenter.de.ibm.com (8.13.8/8.13.8/NCO v8.3) with ESMTP id l42E8RHQ2625576 for ; Wed, 2 May 2007 16:08:27 +0200 Received: from d12av02.megacenter.de.ibm.com (loopback [127.0.0.1]) by d12av02.megacenter.de.ibm.com (8.12.11.20060308/8.13.3) with ESMTP id l42E8RgH017035 for ; Wed, 2 May 2007 16:08:27 +0200 Received: from dyn-9-152-216-62.boeblingen.de.ibm.com (dyn-9-152-216-62.boeblingen.de.ibm.com [9.152.216.62]) by d12av02.megacenter.de.ibm.com (8.12.11.20060308/8.12.11) with ESMTP id l42E8RW0017030 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Wed, 2 May 2007 16:08:27 +0200 Received: from dyn-9-152-216-62.boeblingen.de.ibm.com (localhost.localdomain [127.0.0.1]) by dyn-9-152-216-62.boeblingen.de.ibm.com (8.12.8/8.12.8) with ESMTP id l42E8Rqo019291; Wed, 2 May 2007 16:08:27 +0200 Received: (from uweigand@localhost) by dyn-9-152-216-62.boeblingen.de.ibm.com (8.12.8/8.12.8/Submit) id l42E8Qc0019267; Wed, 2 May 2007 16:08:26 +0200 From: uweigand@de.ibm.com Message-Id: <200705021408.l42E8Qc0019267@dyn-9-152-216-62.boeblingen.de.ibm.com> Subject: Re: [RFC] "single step" atomic instruction sequences as a whole on PPC To: luisgpm@linux.vnet.ibm.com Date: Wed, 02 May 2007 14:08:00 -0000 Cc: uweigand@de.ibm.com (Ulrich Weigand), gdb-patches@sourceware.org In-Reply-To: <1177964763.15264.45.camel@localhost> from "Luis Machado" at Apr 30, 2007 05:26:03 PM X-Mailer: ELM [version 2.5 PL2] MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit 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: 2007-05/txt/msg00033.txt.bz2 Luis Machado wrote: > Patch updated to reflect the inclusion of the regcache parameter. Thanks! A number of additional comments: > +static int > +deal_with_atomic_sequence () Parameter is missing here. > +{ > + CORE_ADDR pc = read_pc (); > + CORE_ADDR breaks[2] = {-1, -1}; > + CORE_ADDR loc = pc; > + int insn = read_memory_integer (loc, PPC_INSN_SIZE); > + int last_break = 0; > + int i; > + > + /* Assume all atomic sequences start with an lwarx or ldarx instruction. */ Should have two spaces after '.' (multiple instances of this throughout the patch) > + if ((insn & LWARX_MASK) != LWARX_INSTRUCTION > + && (insn & LWARX_MASK) != LDARX_INSTRUCTION) > + return 0; > + > + /* Assume that no atomic sequence is longer than 6 instructions. */ > + for (i = 1; i < 5; ++i) > + { > + loc += PPC_INSN_SIZE; > + insn = read_memory_integer (loc, PPC_INSN_SIZE); > + > + /* Assume at most one conditional branch instruction between > + the lwarx and stwcx instructions.*/ Well, what happens if there *is* more than one? You should at least detect that case ... > + if ((insn & BC_MASK) == BC_INSTRUCTION) > + { > + last_break = 1; > + breaks[1] = IMMEDIATE_PART (insn); > + if ( ! ABSOLUTE_P(insn)) Space before '('; no spaces around '!'. > + breaks[1] += loc; > + continue; > + } Why don't you use the existing branch_dest routine instead of re-implementing (parts of) it here? > + if ((insn & STWCX_MASK) == STWCX_INSTRUCTION > + || (insn & STWCX_MASK) == STDCX_INSTRUCTION) > + break; > + } > + > + /* Assume that the atomic sequence ends with a stwcx instruction > + followed by a conditional branch instruction. */ > + if ((insn & STWCX_MASK) != STWCX_INSTRUCTION > + && (insn & STWCX_MASK) != STDCX_INSTRUCTION) > + warning (_("Tried to step over an atomic sequence of instructions at %s\n \ > + but could not find the end of the sequence."), > + core_addr_to_string(pc)); Shouldn't you return to the default single-step handling if you do not understand the sequence you find? > + > + loc += PPC_INSN_SIZE; > + insn = read_memory_integer (loc, PPC_INSN_SIZE); > + > + if ((insn & BC_MASK) != BC_INSTRUCTION) > + warning (_("Tried to step over an atomic sequence of instructions at %s\n \ > + but the instruction sequence ended in an unexpected way."), > + core_addr_to_string(pc)); I'm wondering why this test is necessary. So what if there is no conditional branch immediately after the stwcx? That shouldn't really matter ... > + > + breaks[0] = loc; > + > + /* This should never happen, but make sure we don't put > + two breakpoints on the same address. */ > + if (last_break && breaks[1] == breaks[0]) > + last_break = 0; > + > + for (i = 0; i <= last_break; ++i) > + insert_single_step_breakpoint (breaks[i]); > + > + printf_unfiltered (_("Stepping over an atomic sequence of instructions beginning at %s\n"), > + core_addr_to_string (pc)); Should we really output this message unconditionally? If you're automatically single-stepping (say, due to software watch points), this message could potentially show up very frequently ... Bye, Ulrich -- Dr. Ulrich Weigand Linux on zSeries Development Ulrich.Weigand@de.ibm.com