From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 11551 invoked by alias); 12 Jun 2012 12:45:35 -0000 Received: (qmail 11540 invoked by uid 22791); 12 Jun 2012 12:45:34 -0000 X-SWARE-Spam-Status: No, hits=-4.0 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL,SPF_NEUTRAL X-Spam-Check-By: sourceware.org Received: from ozlabs.org (HELO ozlabs.org) (203.10.76.45) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 12 Jun 2012 12:45:20 +0000 Received: from kryten (ppp121-45-170-72.lns20.syd6.internode.on.net [121.45.170.72]) (using TLSv1.2 with cipher DHE-RSA-AES128-SHA (128/128 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPSA id EE04FB6F9D; Tue, 12 Jun 2012 22:45:14 +1000 (EST) Date: Tue, 12 Jun 2012 12:45:00 -0000 From: Anton Blanchard To: Joel Brobecker Cc: gdb-patches@sourceware.org, emachado@linux.vnet.ibm.com Subject: Re: [PATCH 1/3] Fix ppc64 single step over atomic sequence testcase Message-ID: <20120612224515.15bc044d@kryten> In-Reply-To: <20120611193326.GN2687@adacore.com> References: <20120606135557.7da37cbe@kryten> <20120611193326.GN2687@adacore.com> 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: 2012-06/txt/msg00336.txt.bz2 Hi Joel, > Thanks for your contributions, > > First, a quick question: Are you making these contributions on behalf > of a corporation, or on your own? This is for copyright assignment > purposes - I just want to make sure that we have one already on file. Thanks for the review. I work for IBM and my paperwork should be on file. > > The current ppc64 single step over atomic sequence testcase is > > broken. Convert the test into assembly and use stepi to step > > through it. > > It would be useful for you to say what exactly is broken, and on > which platform. At least it seems to have been working for some > people (at IBM). Sorry the explanation was a bit terse. The testcase assumes it only requires two "next"s to get through the test function: set bp1 [gdb_get_line_number "lwarx"] gdb_breakpoint "$bp1" "Breakpoint $decimal at $hex" \ "Set the breakpoint at the start of the sequence" gdb_test continue "Continuing.*Breakpoint $decimal.*" \ "Continue until breakpoint" gdb_test next ".*__asm __volatile.*" \ "Step through the lwarx/stwcx sequence" gdb_test next ".*return 0.*" \ "Step through the ldarx/stdcx sequence" If I run this testcase manually on Fedora 16 (gcc 4.6.2), it actually takes 7 steps to get through it: Breakpoint 2, main () at ./gdb.arch/ppc64-atomic-inst.c:27 27 __asm __volatile ("1: lwarx %0,0,%2\n" \ (gdb) next 32 : "b" (word_addr), "m" (*word_addr) \ (gdb) next 27 __asm __volatile ("1: lwarx %0,0,%2\n" \ (gdb) next 39 : "=&b" (dword), "=m" (*dword_addr) \ (gdb) next 35 __asm __volatile ("1: ldarx %0,0,%2\n" \ (gdb) next 40 : "b" (dword_addr), "m" (*dword_addr) \ (gdb) next 35 __asm __volatile ("1: ldarx %0,0,%2\n" \ (gdb) next 43 return 0; (gdb) I'm not sure what is expected here, is "next" supposed to step all the way through inline assembly? Perhaps it is, but it seems fragile to depend on this high level behaviour. Anton > > > 2012-06-05 Anton Blanchard > > > > * gdb.arch/ppc64-atomic-inst.c: Remove > > * gdb.arch/ppc64-atomic-inst.s: New file > > * gdb.arch/ppc64-atomic-inst.exp: Adapt for asm based > > testcase > > > > Index: gdb/gdb/testsuite/gdb.arch/ppc64-atomic-inst.exp > > =================================================================== > > --- gdb.orig/gdb/testsuite/gdb.arch/ppc64-atomic-inst.exp > > +++ gdb/gdb/testsuite/gdb.arch/ppc64-atomic-inst.exp > > @@ -27,7 +27,7 @@ if {![istarget "powerpc*"] || ![is_lp64_ > > } > > > > set testfile "ppc64-atomic-inst" > > -set srcfile ${testfile}.c > > +set srcfile ${testfile}.s > > set binfile ${objdir}/${subdir}/${testfile} > > set compile_flags {debug quiet} > > > > @@ -50,11 +50,18 @@ set bp1 [gdb_get_line_number "lwarx"] > > gdb_breakpoint "$bp1" "Breakpoint $decimal at $hex" \ > > "Set the breakpoint at the start of the sequence" > > > > +set bp2 [gdb_get_line_number "ldarx"] > > +gdb_breakpoint "$bp2" "Breakpoint $decimal at $hex" \ > > + "Set the breakpoint at the start of the sequence" > > + > > gdb_test continue "Continuing.*Breakpoint $decimal.*" \ > > "Continue until breakpoint" > > > > -gdb_test next ".*__asm __volatile.*" \ > > +gdb_test nexti "bne.*1b" \ > > "Step through the lwarx/stwcx sequence" > > > > -gdb_test next ".*return 0.*" \ > > - "Step through the ldarx/stdcx sequence" > > +gdb_test continue "Continuing.*Breakpoint $decimal.*" \ > > + "Continue until breakpoint" > > + > > +gdb_test nexti "bne.*1b" \ > > + "Step through the lwarx/stwcx sequence" > > Index: gdb/gdb/testsuite/gdb.arch/ppc64-atomic-inst.c > > =================================================================== > > --- gdb.orig/gdb/testsuite/gdb.arch/ppc64-atomic-inst.c > > +++ /dev/null > > @@ -1,44 +0,0 @@ > > -/* This file is part of GDB, the GNU debugger. > > - > > - Copyright 2008-2012 Free Software Foundation, Inc. > > - > > - This program is free software; you can redistribute it and/or > > modify > > - it under the terms of the GNU General Public License as > > published by > > - the Free Software Foundation; either version 3 of the License, > > or > > - (at your option) any later version. > > - > > - This program is distributed in the hope that it will be useful, > > - but WITHOUT ANY WARRANTY; without even the implied warranty of > > - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > - GNU General Public License for more details. > > - > > - You should have received a copy of the GNU General Public > > License > > - along with this program. If not, see > > . */ - > > -#include > > - > > -int main() > > -{ > > - unsigned int word = 0; > > - unsigned int *word_addr = &word; > > - unsigned long dword = 0; > > - unsigned long *dword_addr = &dword; > > - > > - __asm __volatile ("1: lwarx %0,0,%2\n" \ > > - " addi %0,%0,1\n" \ > > - " stwcx. %0,0,%2\n" \ > > - " bne- 1b" > > \ > > - : "=&b" (word), "=m" (*word_addr) \ > > - : "b" (word_addr), "m" (*word_addr) \ > > - : "cr0", "memory"); > > \ - > > - __asm __volatile ("1: ldarx %0,0,%2\n" \ > > - " addi %0,%0,1\n" \ > > - " stdcx. %0,0,%2\n" \ > > - " bne- 1b" \ > > - : "=&b" (dword), "=m" (*dword_addr) \ > > - : "b" (dword_addr), "m" (*dword_addr) \ > > - : "cr0", "memory"); \ > > - > > - return 0; > > -} > > Index: gdb/gdb/testsuite/gdb.arch/ppc64-atomic-inst.s > > =================================================================== > > --- /dev/null > > +++ gdb/gdb/testsuite/gdb.arch/ppc64-atomic-inst.s > > @@ -0,0 +1,26 @@ > > + .include "gdb.asm/common.inc" > > + .include "gdb.asm/powerpc64.inc" > > + > > +.global main > > +gdbasm_declare main > > + li 0,0 > > + addi 4,1,-8 > > + > > + stw 0,0(4) > > +1: lwarx 5,0,4 > > + cmpwi 5,0 > > + bne 2f > > + addi 5,5,1 > > + stwcx. 5,0,4 > > + bne 1b > > + > > + std 0,0(4) > > +2: ldarx 5,0,4 > > + cmpdi 5,0 > > + bne 3f > > + addi 5,5,1 > > + stdcx. 5,0,4 > > + bne 1b > > + > > +3: li 3,0 > > + blr >