From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24579 invoked by alias); 23 Dec 2010 07:01:13 -0000 Received: (qmail 24562 invoked by uid 22791); 23 Dec 2010 07:01:11 -0000 X-SWARE-Spam-Status: No, hits=-1.8 required=5.0 tests=AWL,BAYES_00,TW_EG,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 23 Dec 2010 07:01:05 +0000 Received: (qmail 27845 invoked from network); 23 Dec 2010 07:01:02 -0000 Received: from unknown (HELO ?192.168.0.101?) (yao@127.0.0.2) by mail.codesourcery.com with ESMTPA; 23 Dec 2010 07:01:02 -0000 Message-ID: <4D12F3A8.3020102@codesourcery.com> Date: Thu, 23 Dec 2010 08:45:00 -0000 From: Yao Qi User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.13) Gecko/20101208 Thunderbird/3.1.7 MIME-Version: 1.0 To: Joel Brobecker CC: gdb-patches@sourceware.org Subject: Re: [rfa] Update PC without side effect in displaced stepping References: <4D0F0ABA.9010506@codesourcery.com> <201012200804.oBK84oPu005379@glazunov.sibelius.xs4all.nl> <4D0F5D36.2040909@codesourcery.com> <4D10D377.8080100@codesourcery.com> <20101223042236.GS2596@adacore.com> In-Reply-To: <20101223042236.GS2596@adacore.com> Content-Type: multipart/mixed; boundary="------------020707020004010303020603" 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: 2010-12/txt/msg00429.txt.bz2 This is a multi-part message in MIME format. --------------020707020004010303020603 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Content-length: 866 On 12/23/2010 12:22 PM, Joel Brobecker wrote: > I haven't seen the patch, so I cannot comment specifically, but I think > that you are using the wrong reasons to try to justify your initial > patch. It does not matter whether sparc or hppa support displaced > stepping or not. They might - it's not far-fetched for sparc, for > instance. Or other platforms where it matters might be contributed > in the future, and they could need displaced stepping too. By letting > your patch in, we would be making it harder for other platforms to > implement it. It would feel like sweeping the dust under the carpet... OK. I have to try the second approach, which is 1) exposing displaced stepping state to tdep, and 2) take displaced stepping state into account when determining the mode. Regression tested on armv7-unknown-linux-gnueabi. Is it OK? -- Yao (齐尧) --------------020707020004010303020603 Content-Type: text/x-patch; name="arm_displace_step_in_thumb_1223.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="arm_displace_step_in_thumb_1223.patch" Content-length: 4754 2010-12-23 Yao Qi * arm-tdep.c: (arm_pc_is_thumb): Adjust MEMADDR if it is within copy area of displaced stepping. * infrun.c (struct displaced_step_inferior_state): Move to ... Expose get_displaced_stepping_state. * inferior.h: ... here. Declare get_displaced_stepping_state. diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c index 636c1de..3227619 100644 --- a/gdb/arm-tdep.c +++ b/gdb/arm-tdep.c @@ -367,6 +367,22 @@ arm_pc_is_thumb (struct gdbarch *gdbarch, CORE_ADDR memaddr) struct obj_section *sec; struct minimal_symbol *sym; char type; + struct displaced_step_inferior_state *displaced + = get_displaced_stepping_state (ptid_get_pid (inferior_ptid)); + + /* If checking the mode of displaced instruction in copy area, the mode + should be determined by instruction on the original address. */ + + if (displaced && !ptid_equal (displaced->step_ptid, null_ptid) + && (displaced->step_copy == memaddr)) + { + if (debug_displaced) + fprintf_unfiltered (gdb_stdlog, + "displaced: check mode of %.8lx instead of %.8lx\n", + (unsigned long) displaced->step_original, + (unsigned long) memaddr); + memaddr = displaced->step_original; + } /* If bit 0 of the address is set, assume this is a Thumb address. */ if (IS_THUMB_ADDR (memaddr)) diff --git a/gdb/inferior.h b/gdb/inferior.h index f80ecb5..7e68d3b 100644 --- a/gdb/inferior.h +++ b/gdb/inferior.h @@ -362,10 +362,46 @@ extern struct regcache *stop_registers; /* True if we are debugging displaced stepping. */ extern int debug_displaced; +/* Per-inferior displaced stepping state. */ +struct displaced_step_inferior_state +{ + /* Pointer to next in linked list. */ + struct displaced_step_inferior_state *next; + + /* The process this displaced step state refers to. */ + int pid; + + /* A queue of pending displaced stepping requests. One entry per + thread that needs to do a displaced step. */ + struct displaced_step_request *step_request_queue; + + /* If this is not null_ptid, this is the thread carrying out a + displaced single-step in process PID. This thread's state will + require fixing up once it has completed its step. */ + ptid_t step_ptid; + + /* The architecture the thread had when we stepped it. */ + struct gdbarch *step_gdbarch; + + /* The closure provided gdbarch_displaced_step_copy_insn, to be used + for post-step cleanup. */ + struct displaced_step_closure *step_closure; + + /* The address of the original instruction, and the copy we + made. */ + CORE_ADDR step_original, step_copy; + + /* Saved contents of copy area. */ + gdb_byte *step_saved_copy; +}; + /* Dump LEN bytes at BUF in hex to FILE, followed by a newline. */ void displaced_step_dump_bytes (struct ui_file *file, const gdb_byte *buf, size_t len); + +struct displaced_step_inferior_state *get_displaced_stepping_state (int pid); + /* Possible values for gdbarch_call_dummy_location. */ #define ON_STACK 1 diff --git a/gdb/infrun.c b/gdb/infrun.c index 1bc00a4..d943dd3 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -998,46 +998,13 @@ struct displaced_step_request struct displaced_step_request *next; }; -/* Per-inferior displaced stepping state. */ -struct displaced_step_inferior_state -{ - /* Pointer to next in linked list. */ - struct displaced_step_inferior_state *next; - - /* The process this displaced step state refers to. */ - int pid; - - /* A queue of pending displaced stepping requests. One entry per - thread that needs to do a displaced step. */ - struct displaced_step_request *step_request_queue; - - /* If this is not null_ptid, this is the thread carrying out a - displaced single-step in process PID. This thread's state will - require fixing up once it has completed its step. */ - ptid_t step_ptid; - - /* The architecture the thread had when we stepped it. */ - struct gdbarch *step_gdbarch; - - /* The closure provided gdbarch_displaced_step_copy_insn, to be used - for post-step cleanup. */ - struct displaced_step_closure *step_closure; - - /* The address of the original instruction, and the copy we - made. */ - CORE_ADDR step_original, step_copy; - - /* Saved contents of copy area. */ - gdb_byte *step_saved_copy; -}; - /* The list of states of processes involved in displaced stepping presently. */ static struct displaced_step_inferior_state *displaced_step_inferior_states; /* Get the displaced stepping state of process PID. */ -static struct displaced_step_inferior_state * +struct displaced_step_inferior_state * get_displaced_stepping_state (int pid) { struct displaced_step_inferior_state *state; --------------020707020004010303020603--