From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 92929 invoked by alias); 9 May 2016 11:05:39 -0000 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 Received: (qmail 91851 invoked by uid 89); 9 May 2016 11:05:38 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.6 required=5.0 tests=BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 spammy=HX-Received:10.66.33.1, 4136 X-HELO: mail-pa0-f47.google.com Received: from mail-pa0-f47.google.com (HELO mail-pa0-f47.google.com) (209.85.220.47) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Mon, 09 May 2016 11:05:28 +0000 Received: by mail-pa0-f47.google.com with SMTP id bt5so71174931pac.3 for ; Mon, 09 May 2016 04:05:27 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to :message-id:user-agent:mime-version:content-transfer-encoding; bh=jSuNcSOYqX0L0RHkE4/yFft1Dk3cl00Fx5LPYHFSFAI=; b=HLFcqXYpzw7qD9yhrBKS0d1v/xkBiimU+PI6SKWk2C78iO+NYWhUBIll1+OyvOffdK nMLkFf9EhvFxjM1kGQAAvqKTUWtvxN/bRVlmrRLzOyA5lVMOczDxQ+tOPIGfbKiiy7XZ pWUREF0eefYGDiK/7fuptLFKfgxQbfIP0hMEfTqK2rQmQn9qzPihRQsWXUBHrdg4h44n yH+ce/zbMepQVNVt10x6Nqk9BY7m39da2n3OTfete8g994k4HhW5aEccsgMEBgDADbFH RyIlkOs8KX+eHd7u8/KOtgr1fhm20Gd7F8Zs9WLRANcHCtN4Xd/OmY2BVq3KnS6kbdzY bSfw== X-Gm-Message-State: AOPr4FX3EJd4uPWidTpB5qkrXrjNHH4t12XI4a/bYuwc+9pz3QROASrnAkBrvbzvAH6u9g== X-Received: by 10.66.33.1 with SMTP id n1mr49311954pai.65.1462791925918; Mon, 09 May 2016 04:05:25 -0700 (PDT) Received: from E107787-LIN (gcc113.osuosl.org. [140.211.9.71]) by smtp.gmail.com with ESMTPSA id dh4sm39857576pad.37.2016.05.09.04.05.21 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Mon, 09 May 2016 04:05:25 -0700 (PDT) From: Yao Qi To: Pedro Alves Cc: Yao Qi , gdb-patches@sourceware.org Subject: Re: [PATCH 2/2] Remove gdbarch method displaced_step_hw_singlestep References: <1458742206-622-1-git-send-email-yao.qi@linaro.org> <1458742206-622-3-git-send-email-yao.qi@linaro.org> <56F2D1BF.5070005@redhat.com> Date: Mon, 09 May 2016 11:05:00 -0000 In-Reply-To: <56F2D1BF.5070005@redhat.com> (Pedro Alves's message of "Wed, 23 Mar 2016 17:26:23 +0000") Message-ID: <86r3dbebkg.fsf@gmail.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2016-05/txt/msg00144.txt.bz2 Pedro Alves writes: > I wasn't sure whether this is safe, but I convinced myself it is. > I'd have been nice if there had been a note in the log about the below: > > Currently, when stepping past an instruction in the scratch pad, these > archs won't ever reach the software_single_step method, always forcing a > hardware single-step, even if the software_single_step method would > insert some breakpoint. The question is: is it safe now for them to > analyse the instruction copied to the scratch pad, and potentially insert > a software single-step? > > I think it is safe, because we won't ever use displaced stepping > for the cases where the software_single_step method would return > something non-NULL. software_single_step returns non-NULL _only_ except displaced stepping on arm linux target. However, GDB won't use software single step for displaced stepping, because it either single-step the instructions the scratch pad or resume the program from the instructions in the scratch pad but these instructions end with a breakpoint instruction (see arm_displaced_init_closure). > for atomic regions, while OTOH, displaced_step_copy_insn always returns > NULL for atomic regions. E.g., notice how ppc_displaced_step_copy_insn > vs ppc_deal_with_atomic_sequence. I'd like to add some comments, see the patch below, --=20 Yao (=E9=BD=90=E5=B0=A7) =46rom c758e0080db93b26c88ab2d6dddb5451470c1a7f Mon Sep 17 00:00:00 2001 From: Yao Qi Date: Fri, 18 Mar 2016 16:25:38 +0000 Subject: [PATCH] Remove gdbarch method displaced_step_hw_singlestep displaced_step_hw_singlestep was added for some targets, which can do hardware single step, but need software single step for some special instructions. After we change gdbarch method software_single_step, displaced_step_hw_singlestep is no longer necessary, because we can get the same information from software_single_step. This patch is to remove displaced_step_hw_singlestep. gdb: 2016-05-09 Yao Qi * aarch64-linux-tdep.c (aarch64_linux_init_abi): Don't call set_gdbarch_displaced_step_hw_singlestep. * aarch64-tdep.c (aarch64_displaced_step_hw_singlestep): Remove. * arch-utils.c (default_displaced_step_hw_singlestep): Remove. * arch-utils.h (default_displaced_step_hw_singlestep): Remove. * gdbarch.sh (displaced_step_hw_singlestep): Remove. * gdbarch.c, gdbarch.h: Regenerated. * infrun.c (resume): Don't call gdbarch_displaced_step_hw_singlestep. Call gdbarch_software_single_step instead. * rs6000-tdep.c (ppc_displaced_step_hw_singlestep): Remove. (rs6000_gdbarch_init): Don't call set_gdbarch_displaced_step_hw_singlestep. * s390-linux-tdep.c (s390_displaced_step_hw_singlestep): Remove. (s390_gdbarch_init): Don't call set_gdbarch_displaced_step_hw_singlestep. diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c index 651a0f0..8344d06 100644 --- a/gdb/aarch64-linux-tdep.c +++ b/gdb/aarch64-linux-tdep.c @@ -1208,8 +1208,6 @@ aarch64_linux_init_abi (struct gdbarch_info info, str= uct gdbarch *gdbarch) set_gdbarch_displaced_step_free_closure (gdbarch, simple_displaced_step_free_closure); set_gdbarch_displaced_step_location (gdbarch, linux_displaced_step_locat= ion); - set_gdbarch_displaced_step_hw_singlestep (gdbarch, - aarch64_displaced_step_hw_singlestep); } =20 /* Provide a prototype to silence -Wmissing-prototypes. */ diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c index 52c9ea6..460eade 100644 --- a/gdb/aarch64-tdep.c +++ b/gdb/aarch64-tdep.c @@ -2637,15 +2637,6 @@ aarch64_displaced_step_fixup (struct gdbarch *gdbarc= h, } } =20 -/* Implement the "displaced_step_hw_singlestep" gdbarch method. */ - -int -aarch64_displaced_step_hw_singlestep (struct gdbarch *gdbarch, - struct displaced_step_closure *closure) -{ - return 1; -} - /* Initialize the current architecture based on INFO. If possible, re-use an architecture from ARCHES, which is a list of architectures already created during this debugging session. diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c index c3d7802..3049252 100644 --- a/gdb/arch-utils.c +++ b/gdb/arch-utils.c @@ -67,13 +67,6 @@ simple_displaced_step_free_closure (struct gdbarch *gdba= rch, xfree (closure); } =20 -int -default_displaced_step_hw_singlestep (struct gdbarch *gdbarch, - struct displaced_step_closure *closure) -{ - return !gdbarch_software_single_step_p (gdbarch); -} - CORE_ADDR displaced_step_at_entry_point (struct gdbarch *gdbarch) { diff --git a/gdb/arch-utils.h b/gdb/arch-utils.h index 9e1e70e..318e227 100644 --- a/gdb/arch-utils.h +++ b/gdb/arch-utils.h @@ -45,11 +45,6 @@ extern void simple_displaced_step_free_closure (struct gdbarch *gdbarch, struct displaced_step_closure *closu= re); =20 -/* Default implementation of gdbarch_displaced_hw_singlestep. */ -extern int - default_displaced_step_hw_singlestep (struct gdbarch *, - struct displaced_step_closure *); - /* Possible value for gdbarch_displaced_step_location: Place displaced instructions at the program's entry point, leaving space for inferior function call return breakpoints. */ diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c index e59a93d..d469dc9 100644 --- a/gdb/gdbarch.c +++ b/gdb/gdbarch.c @@ -277,7 +277,6 @@ struct gdbarch gdbarch_skip_permanent_breakpoint_ftype *skip_permanent_breakpoint; ULONGEST max_insn_length; gdbarch_displaced_step_copy_insn_ftype *displaced_step_copy_insn; - gdbarch_displaced_step_hw_singlestep_ftype *displaced_step_hw_singlestep; gdbarch_displaced_step_fixup_ftype *displaced_step_fixup; gdbarch_displaced_step_free_closure_ftype *displaced_step_free_closure; gdbarch_displaced_step_location_ftype *displaced_step_location; @@ -414,7 +413,6 @@ gdbarch_alloc (const struct gdbarch_info *info, gdbarch->adjust_dwarf2_line =3D default_adjust_dwarf2_line; gdbarch->register_reggroup_p =3D default_register_reggroup_p; gdbarch->skip_permanent_breakpoint =3D default_skip_permanent_breakpoint; - gdbarch->displaced_step_hw_singlestep =3D default_displaced_step_hw_sing= lestep; gdbarch->displaced_step_fixup =3D NULL; gdbarch->displaced_step_free_closure =3D NULL; gdbarch->displaced_step_location =3D NULL; @@ -624,7 +622,6 @@ verify_gdbarch (struct gdbarch *gdbarch) /* Skip verify of skip_permanent_breakpoint, invalid_p =3D=3D 0 */ /* Skip verify of max_insn_length, has predicate. */ /* Skip verify of displaced_step_copy_insn, has predicate. */ - /* Skip verify of displaced_step_hw_singlestep, invalid_p =3D=3D 0 */ /* Skip verify of displaced_step_fixup, has predicate. */ if ((! gdbarch->displaced_step_free_closure) !=3D (! gdbarch->displaced_= step_copy_insn)) fprintf_unfiltered (log, "\n\tdisplaced_step_free_closure"); @@ -873,9 +870,6 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *= file) "gdbarch_dump: displaced_step_free_closure =3D <%s>\= n", host_address_to_string (gdbarch->displaced_step_free= _closure)); fprintf_unfiltered (file, - "gdbarch_dump: displaced_step_hw_singlestep =3D <%s>= \n", - host_address_to_string (gdbarch->displaced_step_hw_s= inglestep)); - fprintf_unfiltered (file, "gdbarch_dump: displaced_step_location =3D <%s>\n", host_address_to_string (gdbarch->displaced_step_loca= tion)); fprintf_unfiltered (file, @@ -3749,23 +3743,6 @@ set_gdbarch_displaced_step_copy_insn (struct gdbarch= *gdbarch, } =20 int -gdbarch_displaced_step_hw_singlestep (struct gdbarch *gdbarch, struct disp= laced_step_closure *closure) -{ - gdb_assert (gdbarch !=3D NULL); - gdb_assert (gdbarch->displaced_step_hw_singlestep !=3D NULL); - if (gdbarch_debug >=3D 2) - fprintf_unfiltered (gdb_stdlog, "gdbarch_displaced_step_hw_singlestep = called\n"); - return gdbarch->displaced_step_hw_singlestep (gdbarch, closure); -} - -void -set_gdbarch_displaced_step_hw_singlestep (struct gdbarch *gdbarch, - gdbarch_displaced_step_hw_single= step_ftype displaced_step_hw_singlestep) -{ - gdbarch->displaced_step_hw_singlestep =3D displaced_step_hw_singlestep; -} - -int gdbarch_displaced_step_fixup_p (struct gdbarch *gdbarch) { gdb_assert (gdbarch !=3D NULL); diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h index e026c0e..da0edcc 100644 --- a/gdb/gdbarch.h +++ b/gdb/gdbarch.h @@ -957,20 +957,6 @@ typedef struct displaced_step_closure * (gdbarch_displ= aced_step_copy_insn_ftype) extern struct displaced_step_closure * gdbarch_displaced_step_copy_insn (s= truct gdbarch *gdbarch, CORE_ADDR from, CORE_ADDR to, struct regcache *regs= ); extern void set_gdbarch_displaced_step_copy_insn (struct gdbarch *gdbarch,= gdbarch_displaced_step_copy_insn_ftype *displaced_step_copy_insn); =20 -/* Return true if GDB should use hardware single-stepping to execute - the displaced instruction identified by CLOSURE. If false, - GDB will simply restart execution at the displaced instruction - location, and it is up to the target to ensure GDB will receive - control again (e.g. by placing a software breakpoint instruction - into the displaced instruction buffer). -=20=20 - The default implementation returns false on all targets that - provide a gdbarch_software_single_step routine, and true otherwise. */ - -typedef int (gdbarch_displaced_step_hw_singlestep_ftype) (struct gdbarch *= gdbarch, struct displaced_step_closure *closure); -extern int gdbarch_displaced_step_hw_singlestep (struct gdbarch *gdbarch, = struct displaced_step_closure *closure); -extern void set_gdbarch_displaced_step_hw_singlestep (struct gdbarch *gdba= rch, gdbarch_displaced_step_hw_singlestep_ftype *displaced_step_hw_singlest= ep); - /* Fix up the state resulting from successfully single-stepping a displaced instruction, to give the result we would have gotten from stepping the instruction in its original location. diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh index b144c04..8b8710f 100755 --- a/gdb/gdbarch.sh +++ b/gdb/gdbarch.sh @@ -780,17 +780,6 @@ V:ULONGEST:max_insn_length:::0:0 # that case. M:struct displaced_step_closure *:displaced_step_copy_insn:CORE_ADDR from,= CORE_ADDR to, struct regcache *regs:from, to, regs =20 -# Return true if GDB should use hardware single-stepping to execute -# the displaced instruction identified by CLOSURE. If false, -# GDB will simply restart execution at the displaced instruction -# location, and it is up to the target to ensure GDB will receive -# control again (e.g. by placing a software breakpoint instruction -# into the displaced instruction buffer). -# -# The default implementation returns false on all targets that -# provide a gdbarch_software_single_step routine, and true otherwise. -m:int:displaced_step_hw_singlestep:struct displaced_step_closure *closure:= closure::default_displaced_step_hw_singlestep::0 - # Fix up the state resulting from successfully single-stepping a # displaced instruction, to give the result we would have gotten from # stepping the instruction in its original location. diff --git a/gdb/infrun.c b/gdb/infrun.c index 3ffec86..464e62a 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -2626,8 +2626,33 @@ resume (enum gdb_signal sig) pc =3D regcache_read_pc (get_thread_regcache (inferior_ptid)); =20 displaced =3D get_displaced_stepping_state (ptid_get_pid (inferior_ptid= )); - step =3D gdbarch_displaced_step_hw_singlestep (gdbarch, - displaced->step_closure); + + /* Although GDB won't use software single step for displaced + stepping, we call software_single_step here to determine + whether to + - single step the instructions in the scratch pad, (like + x86, ppc and aarch64), + - or resume the program from the instructions in the scratch + pad. These instructions must end with a breakpoint + instruction. */ + if (gdbarch_software_single_step_p (gdbarch)) + { + VEC (CORE_ADDR) * next_pcs; + + next_pcs =3D gdbarch_software_single_step (gdbarch, + get_current_frame ()); + + if (next_pcs !=3D NULL) + { + step =3D 0; + VEC_free (CORE_ADDR, next_pcs); + } + else + step =3D 1; + } + else + step =3D 1; + } } =20 diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c index 26c8ed9..5f0ede2 100644 --- a/gdb/rs6000-tdep.c +++ b/gdb/rs6000-tdep.c @@ -1136,15 +1136,6 @@ ppc_displaced_step_fixup (struct gdbarch *gdbarch, from + offset); } =20 -/* Always use hardware single-stepping to execute the - displaced instruction. */ -static int -ppc_displaced_step_hw_singlestep (struct gdbarch *gdbarch, - struct displaced_step_closure *closure) -{ - return 1; -} - /* Checks for an atomic sequence of instructions beginning with a LWARX/LD= ARX instruction and ending with a STWCX/STDCX instruction. If such a seque= nce is found, attempt to step through it. A breakpoint is placed at the en= d of=20 @@ -6071,8 +6062,7 @@ rs6000_gdbarch_init (struct gdbarch_info info, struct= gdbarch_list *arches) /* Setup displaced stepping. */ set_gdbarch_displaced_step_copy_insn (gdbarch, ppc_displaced_step_copy_insn); - set_gdbarch_displaced_step_hw_singlestep (gdbarch, - ppc_displaced_step_hw_singlestep); + set_gdbarch_displaced_step_fixup (gdbarch, ppc_displaced_step_fixup); set_gdbarch_displaced_step_free_closure (gdbarch, simple_displaced_step_free_closure); diff --git a/gdb/s390-linux-tdep.c b/gdb/s390-linux-tdep.c index dd26ba6..77c7b33 100644 --- a/gdb/s390-linux-tdep.c +++ b/gdb/s390-linux-tdep.c @@ -759,14 +759,6 @@ s390_software_single_step (struct frame_info *frame) return next_pcs; } =20 -static int -s390_displaced_step_hw_singlestep (struct gdbarch *gdbarch, - struct displaced_step_closure *closure) -{ - return 1; -} - - /* Maps for register sets. */ =20 static const struct regcache_map_entry s390_gregmap[] =3D @@ -7990,7 +7982,6 @@ s390_gdbarch_init (struct gdbarch_info info, struct g= dbarch_list *arches) set_gdbarch_inner_than (gdbarch, core_addr_lessthan); set_gdbarch_breakpoint_from_pc (gdbarch, s390_breakpoint_from_pc); set_gdbarch_software_single_step (gdbarch, s390_software_single_step); - set_gdbarch_displaced_step_hw_singlestep (gdbarch, s390_displaced_step_h= w_singlestep); set_gdbarch_skip_prologue (gdbarch, s390_skip_prologue); set_gdbarch_stack_frame_destroyed_p (gdbarch, s390_stack_frame_destroyed= _p); =20