From patchwork Mon May 30 21:35:29 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Russell King (Oracle)" X-Patchwork-Id: 9142581 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id C066E60757 for ; Mon, 30 May 2016 21:39:44 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id B19AC27C23 for ; Mon, 30 May 2016 21:39:44 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id A53292819C; Mon, 30 May 2016 21:39:44 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.1 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED, T_DKIM_INVALID autolearn=unavailable version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 2D46927C23 for ; Mon, 30 May 2016 21:39:43 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1b7Usx-0007co-1i; Mon, 30 May 2016 21:38:07 +0000 Received: from pandora.armlinux.org.uk ([2001:4d48:ad52:3201:214:fdff:fe10:1be6]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1b7Uss-0007K6-Ts for linux-arm-kernel@lists.infradead.org; Mon, 30 May 2016 21:38:04 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=armlinux.org.uk; s=pandora-2014; h=Sender:In-Reply-To:Content-Type:MIME-Version:References:Message-ID:Subject:Cc:To:From:Date; bh=i1TC46uYvAvQaSI/P+cxjlKJp6janYec+jC8BvDcWV0=; b=HXewDtu0m9faCoMIMyMqOQfPX0xdvj/dQzxlXMOSEd57JKaFPRPxVnXkOVV2XrbH6CscmwI97YvD66gINnGpBMfldr1wMnt7Qg7sAOh6BAZg+B/58jpb5ES0hAGWOSTm92OH/7JRd35QRo47sY9D5eLUghhes/E+S+5KuOX2oVs=; Received: from n2100.armlinux.org.uk ([2002:4e20:1eda:1:214:fdff:fe10:4f86]:34312) by pandora.armlinux.org.uk with esmtpsa (TLSv1:DHE-RSA-AES256-SHA:256) (Exim 4.82_1-5b7a7c0-XX) (envelope-from ) id 1b7UqT-0007aR-R6; Mon, 30 May 2016 22:35:33 +0100 Received: from linux by n2100.armlinux.org.uk with local (Exim 4.76) (envelope-from ) id 1b7UqQ-0006Dx-IZ; Mon, 30 May 2016 22:35:30 +0100 Date: Mon, 30 May 2016 22:35:29 +0100 From: Russell King - ARM Linux To: Simon Marchi Subject: Re: Possible race between PTRACE_SETVFPREGS and PTRACE_CONT on ARM? Message-ID: <20160530213529.GS19428@n2100.arm.linux.org.uk> References: <574C7CDB.7050103@ericsson.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <574C7CDB.7050103@ericsson.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20160530_143803_356558_E694A1A9 X-CRM114-Status: GOOD ( 21.80 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP On Mon, May 30, 2016 at 01:48:11PM -0400, Simon Marchi wrote: > Hello knowledgeable ARM people! > > (Background: https://sourceware.org/ml/gdb/2016-05/msg00020.html ) > > Debugging a flaky GDB test case on ARM lead me to think there might > be race between PTRACE_SETVFPREGS and PTRACE_CONT on ARM > (PTRACE_SETVFPREGS is ARM-specific anyway). The test case (and the > reproducer below) changes the value of a VFP register (let's say d0) > using PTRACE_SETVFPREGS and resumes the thread with PTRACE_CONT. It > happens intermittently that the thread resumes execution with the > old value in d0 instead of the new one. So, I thought I'd look into this, and what I see here on my systems (whether it be Marvell Dove or iMX6) is that the program always exits with a return code of 1. Investigation on the Marvell Dove platform leads me to conclude that Ubuntu 14.04 gdb (7.7.1-0ubuntu5~14.04.2) is built without support for VFP - if I add an "info float" into the gdb script, I get: Breakpoint 1, break_here () at test.S:8 8 vmrs APSR_nzcv, fpscr No floating-point info available for this processor. which is incredibly annoying, because it means that your "p $d0 = 4.0" line has no effect on the VFP state - hence why its always exiting with 1. However, if we look closer, we see that gdb has decided to put the breakpoint _after_ the comparison instruction, as confirmed by the disassembly after the breakpoint is hit: 0x00008108 <+0>: vcmp.f64 d0, d1 => 0x0000810c <+4>: vmrs APSR_nzcv, fpscr 0x00008110 <+8>: moveq r0, #1 On iMX6, where I have Ubuntu 12.04 gdb (7.4-2012.04-0ubuntu2.1), "info float" works as one expects, but we still end up with the program exiting with a code of 1 - every time - because again, the breakpoint is misplaced. So, the gdb verisons I have here seem to be particularly poor - but with some modifications, I can test out on iMX6 by forcing gdb to do the right thing - by inserting a couple of "mov r0, r0" instructions after the "break_here" label. With that, on a single CPU, it seems to work correctly every time, but if I bring up a secondary CPU I start seeing the same problems you've reported - which seems to need the following patch to solve. Please can you check whether this resolves your problem? Thanks. arch/arm/kernel/ptrace.c | 2 +- 1 files changed, 1 insertion(+), 1 deletion(-) Tested-by: Simon Marchi diff --git a/arch/arm/kernel/ptrace.c b/arch/arm/kernel/ptrace.c index ef9119f7462e..4d9375814b53 100644 --- a/arch/arm/kernel/ptrace.c +++ b/arch/arm/kernel/ptrace.c @@ -733,8 +733,8 @@ static int vfp_set(struct task_struct *target, if (ret) return ret; - vfp_flush_hwstate(thread); thread->vfpstate.hard = new_vfp; + vfp_flush_hwstate(thread); return 0; }