From patchwork Mon Dec 30 16:25:34 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Luis Machado X-Patchwork-Id: 37127 Received: (qmail 120541 invoked by alias); 30 Dec 2019 16:25:48 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 120533 invoked by uid 89); 30 Dec 2019 16:25:48 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.1 spammy=whenever, tells, Condition X-HELO: mail-pg1-f194.google.com Received: from mail-pg1-f194.google.com (HELO mail-pg1-f194.google.com) (209.85.215.194) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 30 Dec 2019 16:25:46 +0000 Received: by mail-pg1-f194.google.com with SMTP id b9so18223587pgk.12 for ; Mon, 30 Dec 2019 08:25:46 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:subject:date:message-id; bh=VCtMpGA9eCmjED2VBxoP9WDNQgkDkHfBLF2hTvm/6+A=; b=qs7AiE0rl8y9F+o3okyXvYihNbr0Hg7tCISWQfpAGMBvQmbKgpx5ja6Qo5McH8V0op A5eh5ZY5/mYHUZoYBfVgFrjlcw+c7sQ8uI0HT8Mn3HS0DV5L+GLfDgmGH+RiP7wSHftu 4TSaweXZruZKbxbFVPSUKpaTC+4R7EgbBqgu9lpcxT6Gad7aYvMjuj7K5IgENHCCRXnB hGTMOIynyZstQ4JWuoPBepElYPqS6907PNYuEMr7pBkBoa0ZjCcS2WhY4+si9//mpEQD BbxmYyclDUBxAegqfCCGvGffB0lcVsssRcHcda69hIYkjlEX3xwZhh3hWqx2kR0T43bo biEg== Return-Path: Received: from localhost.localdomain ([191.251.164.209]) by smtp.gmail.com with ESMTPSA id p24sm52397369pff.69.2019.12.30.08.25.43 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 30 Dec 2019 08:25:44 -0800 (PST) From: Luis Machado To: gdb-patches@sourceware.org Subject: [PATCH 1/2] [AArch64] Fix step-over-syscall.exp failure Date: Mon, 30 Dec 2019 13:25:34 -0300 Message-Id: <20191230162535.21211-1-luis.machado@linaro.org> X-IsSubscribed: yes In particular, this one: FAIL: gdb.base/step-over-syscall.exp: fork: displaced=on: check_pc_after_cross_syscall: single step over fork final pc When ptrace fork event reporting is enabled, GDB gets a PTRACE_EVENT_FORK event whenever the inferior executes the fork syscall. Then the logic is that GDB needs to step the inferior yet again in order to receive a predetermined SIGTRAP, but no execution takes place because the signal was already queued for delivery. That means the PC should stay the same. I noticed the aarch64 code is currently adjusting the PC in this situation, making the inferior skip an instruction without executing it. The existing code abuses the pc_adjust variable to contain both an offset and also a bool telling GDB when to adjust the PC (pc_adjust != 0). This patch fixes this case by adding a new bool that tells us when we're supposed to adjust the PC, and then proceeding to check if we did not execute the instruction (pc - to == 0), making proper adjustments for such case. Regression tested on aarch64-linux-gnu on the tryserver. gdb/ChangeLog: 2019-12-30 Luis Machado * aarch64-tdep.c (struct aarch64_displaced_step_closure ) : New member. : Adjust the documentation. (aarch64_displaced_step_b): Set should_adjust_pc. (aarch64_displaced_step_b_cond): Likewise. (aarch64_displaced_step_cb): Likewise. (aarch64_displaced_step_tb): Likewise. (aarch64_displaced_step_adr): Likewise. (aarch64_displaced_step_ldr_literal): Likewise. (aarch64_displaced_step_others): Likewise. (aarch64_displaced_step_fixup): Check if PC really moved before adjusting it. Change-Id: I828b7b7f2726f42ce107708f9692f07c63bf728c --- gdb/aarch64-tdep.c | 46 +++++++++++++++++++++++++++++++++++++++------- 1 file changed, 39 insertions(+), 7 deletions(-) diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c index 1d5fb2001d..a639b753cd 100644 --- a/gdb/aarch64-tdep.c +++ b/gdb/aarch64-tdep.c @@ -2737,7 +2737,12 @@ struct aarch64_displaced_step_closure : public displaced_step_closure is being displaced stepping. */ int cond = 0; - /* PC adjustment offset after displaced stepping. */ + /* True if we should adjust the PC after displaced stepping, false + otherwise. */ + bool should_adjust_pc = false; + + /* PC adjustment offset after displaced stepping, if should_adjust_pc + is true. */ int32_t pc_adjust = 0; }; @@ -2783,6 +2788,9 @@ aarch64_displaced_step_b (const int is_bl, const int32_t offset, emit_nop (dsd->insn_buf); dsd->insn_count++; dsd->dsc->pc_adjust = offset; + + if (offset != 0) + dsd->dsc->should_adjust_pc = true; } if (is_bl) @@ -2818,6 +2826,9 @@ aarch64_displaced_step_b_cond (const unsigned cond, const int32_t offset, dsd->dsc->cond = 1; dsd->dsc->pc_adjust = offset; dsd->insn_count = 1; + + if (offset != 0) + dsd->dsc->should_adjust_pc = true; } /* Dynamically allocate a new register. If we know the register @@ -2852,6 +2863,9 @@ aarch64_displaced_step_cb (const int32_t offset, const int is_cbnz, dsd->insn_count = 1; dsd->dsc->cond = 1; dsd->dsc->pc_adjust = offset; + + if (offset != 0) + dsd->dsc->should_adjust_pc = true; } /* Implementation of aarch64_insn_visitor method "tb". */ @@ -2877,6 +2891,9 @@ aarch64_displaced_step_tb (const int32_t offset, int is_tbnz, dsd->insn_count = 1; dsd->dsc->cond = 1; dsd->dsc->pc_adjust = offset; + + if (offset != 0) + dsd->dsc->should_adjust_pc = true; } /* Implementation of aarch64_insn_visitor method "adr". */ @@ -2902,6 +2919,7 @@ aarch64_displaced_step_adr (const int32_t offset, const unsigned rd, address); dsd->dsc->pc_adjust = 4; + dsd->dsc->should_adjust_pc = true; emit_nop (dsd->insn_buf); dsd->insn_count = 1; } @@ -2929,6 +2947,7 @@ aarch64_displaced_step_ldr_literal (const int32_t offset, const int is_sw, aarch64_register (rt, 1), zero); dsd->dsc->pc_adjust = 4; + dsd->dsc->should_adjust_pc = true; } /* Implementation of aarch64_insn_visitor method "others". */ @@ -2946,10 +2965,12 @@ aarch64_displaced_step_others (const uint32_t insn, if ((insn & 0xfffffc1f) == 0xd65f0000) { /* RET */ - dsd->dsc->pc_adjust = 0; } else - dsd->dsc->pc_adjust = 4; + { + dsd->dsc->pc_adjust = 4; + dsd->dsc->should_adjust_pc = true; + } } static const struct aarch64_insn_visitor visitor = @@ -3030,13 +3051,15 @@ aarch64_displaced_step_fixup (struct gdbarch *gdbarch, CORE_ADDR from, CORE_ADDR to, struct regcache *regs) { + ULONGEST pc; + aarch64_displaced_step_closure *dsc = (aarch64_displaced_step_closure *) dsc_; + /* Fetch the current PC, after the displaced execution took place. */ + regcache_cooked_read_unsigned (regs, AARCH64_PC_REGNUM, &pc); + if (dsc->cond) { - ULONGEST pc; - - regcache_cooked_read_unsigned (regs, AARCH64_PC_REGNUM, &pc); if (pc - to == 8) { /* Condition is true. */ @@ -3045,13 +3068,22 @@ aarch64_displaced_step_fixup (struct gdbarch *gdbarch, { /* Condition is false. */ dsc->pc_adjust = 4; + dsc->should_adjust_pc = true; } else gdb_assert_not_reached ("Unexpected PC value after displaced stepping"); } - if (dsc->pc_adjust != 0) + if (dsc->should_adjust_pc) { + + /* Sometimes we may get a SIGTRAP even before executing an instruction. + Such is the case when we are stepping over a fork/vfork/clone syscall + and the instruction after the syscall instruction. Make sure we don't + adjust the PC when we did not really move. */ + if ((pc - to) == 0) + dsc->pc_adjust = 0; + if (debug_displaced) { debug_printf ("displaced: fixup: set PC to %s:%d\n",