[PATCH 1/3] Fix ppc64 single step over atomic sequence testcase

[PATCH 1/3] Fix ppc64 single step over atomic sequence testcase

[Anton Blanchard]

The current ppc64 single step over atomic sequence testcase is broken.
Convert the test into assembly and use stepi to step through it.

Anton
--

2012-06-05  Anton Blanchard  <anton@samba.org>

	* 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 <http://www.gnu.org/licenses/>.  */
-
-#include <stdio.h>
-
-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

[PATCH 2/3] Support up to 3 conditional branches in an atomic sequence

[Anton Blanchard]

gdb currently supports 1 conditional branch inside a ppc larx/stcx
critical region. Unfortunately there is existing code that contains more
than 1, for example in the ppc64 Linux kernel:

c00000000003ac18 <.__hash_page_4K>:
...
c00000000003ac4c:       7f e0 30 a8     ldarx   r31,0,r6
c00000000003ac50:       7c 80 f8 79     andc.   r0,r4,r31
c00000000003ac54:       40 82 02 94     bne-    c00000000003aee8 <htab_wrong_access>
c00000000003ac58:       73 e0 08 00     andi.   r0,r31,2048
c00000000003ac5c:       40 82 01 b0     bne-    c00000000003ae0c <htab_bail_ok>
c00000000003ac60:       54 9e f6 30     rlwinm  r30,r4,30,24,24
c00000000003ac64:       7f de fb 78     or      r30,r30,r31
c00000000003ac68:       63 de 09 00     ori     r30,r30,2304
c00000000003ac6c:       67 de 10 00     oris    r30,r30,4096
c00000000003ac70:       7f c0 31 ad     stdcx.  r30,0,r6

If we try to single step through this we get stuck forever because
the reservation is never set when we step over the stdcx.

The following patch bumps the number to 3 conditional branches + 1
terminating branch. With this patch applied I can single step through
the offending function in the ppc64 Linux kernel.

Anton
--

2012-06-05  Anton Blanchard  <anton@samba.org>

	* gdb/breakpoint.h: Define NR_SINGLE_STEP_BREAKPOINTS
	* rs6000-tdep.c (ppc_deal_with_atomic_sequence): Allow for more
	than two breakpoints.
	* gdb/breakpoint.c (insert_single_step_breakpoint): Likewise
	(insert_single_step_breakpoint): Likewise
	(single_step_breakpoints_inserted): Likewise
	(cancel_single_step_breakpoints): Likewise
	(detach_single_step_breakpoints): Likewise
	(single_step_breakpoint_inserted_here_p): Likewise

Index: gdb/gdb/rs6000-tdep.c
===================================================================
--- gdb.orig/gdb/rs6000-tdep.c	2012-06-06 12:59:47.857432494 +1000
+++ gdb/gdb/rs6000-tdep.c	2012-06-06 12:59:57.409592812 +1000
@@ -1087,7 +1087,7 @@ ppc_deal_with_atomic_sequence (struct fr
   struct address_space *aspace = get_frame_address_space (frame);
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   CORE_ADDR pc = get_frame_pc (frame);
-  CORE_ADDR breaks[2] = {-1, -1};
+  CORE_ADDR breaks[NR_SINGLE_STEP_BREAKPOINTS];
   CORE_ADDR loc = pc;
   CORE_ADDR closing_insn; /* Instruction that closes the atomic sequence.  */
   int insn = read_memory_integer (loc, PPC_INSN_SIZE, byte_order);
@@ -1096,7 +1096,6 @@ ppc_deal_with_atomic_sequence (struct fr
   int last_breakpoint = 0; /* Defaults to 0 (no breakpoints placed).  */  
   const int atomic_sequence_length = 16; /* Instruction sequence length.  */
   int opcode; /* Branch instruction's OPcode.  */
-  int bc_insn_count = 0; /* Conditional branch instruction count.  */
 
   /* Assume all atomic sequences start with a lwarx/ldarx instruction.  */
   if ((insn & LWARX_MASK) != LWARX_INSTRUCTION
@@ -1110,24 +1109,20 @@ ppc_deal_with_atomic_sequence (struct fr
       loc += PPC_INSN_SIZE;
       insn = read_memory_integer (loc, PPC_INSN_SIZE, byte_order);
 
-      /* Assume that there is at most one conditional branch in the atomic
-         sequence.  If a conditional branch is found, put a breakpoint in 
-         its destination address.  */
       if ((insn & BRANCH_MASK) == BC_INSN)
         {
           int immediate = ((insn & 0xfffc) ^ 0x8000) - 0x8000;
           int absolute = insn & 2;
 
-          if (bc_insn_count >= 1)
-            return 0; /* More than one conditional branch found, fallback 
+          if (last_breakpoint >= (NR_SINGLE_STEP_BREAKPOINTS-1))
+            return 0; /* too many conditional branches found, fallback
                          to the standard single-step code.  */
  
 	  if (absolute)
-	    breaks[1] = immediate;
+	    breaks[last_breakpoint] = immediate;
 	  else
-	    breaks[1] = loc + immediate;
+	    breaks[last_breakpoint] = loc + immediate;
 
-	  bc_insn_count++;
 	  last_breakpoint++;
         }
 
@@ -1146,18 +1141,29 @@ ppc_deal_with_atomic_sequence (struct fr
   insn = read_memory_integer (loc, PPC_INSN_SIZE, byte_order);
 
   /* Insert a breakpoint right after the end of the atomic sequence.  */
-  breaks[0] = loc;
+  breaks[last_breakpoint] = loc;
 
-  /* Check for duplicated breakpoints.  Check also for a breakpoint
-     placed (branch instruction's destination) anywhere in sequence.  */
-  if (last_breakpoint
-      && (breaks[1] == breaks[0]
-	  || (breaks[1] >= pc && breaks[1] <= closing_insn)))
-    last_breakpoint = 0;
-
-  /* Effectively inserts the breakpoints.  */
   for (index = 0; index <= last_breakpoint; index++)
-    insert_single_step_breakpoint (gdbarch, aspace, breaks[index]);
+    {
+      int index2;
+      int insert_bp = 1;
+
+      /* Check for a breakpoint placed (branch instruction's destination)
+	 anywhere in sequence.  */
+      if (breaks[index] >= pc && breaks[index] <= closing_insn)
+	continue;
+
+      /* Check for duplicated breakpoints.  */
+      for (index2 = 0; index2 < index; index2++)
+        {
+	  if (breaks[index] == breaks[index2])
+	    insert_bp = 0;
+	}
+
+      /* insert the breakpoint.  */
+      if (insert_bp)
+        insert_single_step_breakpoint (gdbarch, aspace, breaks[index]);
+    }
 
   return 1;
 }
Index: gdb/gdb/breakpoint.c
===================================================================
--- gdb.orig/gdb/breakpoint.c	2012-06-06 12:59:47.841432226 +1000
+++ gdb/gdb/breakpoint.c	2012-06-06 12:59:57.417592947 +1000
@@ -14394,11 +14394,10 @@ deprecated_remove_raw_breakpoint (struct
   return ret;
 }
 
-/* One (or perhaps two) breakpoints used for software single
-   stepping.  */
+/* Breakpoints used for software single stepping.  */
 
-static void *single_step_breakpoints[2];
-static struct gdbarch *single_step_gdbarch[2];
+static void *single_step_breakpoints[NR_SINGLE_STEP_BREAKPOINTS];
+static struct gdbarch *single_step_gdbarch[NR_SINGLE_STEP_BREAKPOINTS];
 
 /* Create and insert a breakpoint for software single step.  */
 
@@ -14407,19 +14406,17 @@ insert_single_step_breakpoint (struct gd
 			       struct address_space *aspace, 
 			       CORE_ADDR next_pc)
 {
+  int i;
   void **bpt_p;
 
-  if (single_step_breakpoints[0] == NULL)
-    {
-      bpt_p = &single_step_breakpoints[0];
-      single_step_gdbarch[0] = gdbarch;
-    }
-  else
-    {
-      gdb_assert (single_step_breakpoints[1] == NULL);
-      bpt_p = &single_step_breakpoints[1];
-      single_step_gdbarch[1] = gdbarch;
-    }
+  for (i = 0; i < NR_SINGLE_STEP_BREAKPOINTS; i++)
+    if (single_step_breakpoints[i] == NULL)
+        break;
+
+  gdb_assert (i < NR_SINGLE_STEP_BREAKPOINTS);
+
+  bpt_p = &single_step_breakpoints[i];
+  single_step_gdbarch[i] = gdbarch;
 
   /* NOTE drow/2006-04-11: A future improvement to this function would
      be to only create the breakpoints once, and actually put them on
@@ -14440,8 +14437,13 @@ insert_single_step_breakpoint (struct gd
 int
 single_step_breakpoints_inserted (void)
 {
-  return (single_step_breakpoints[0] != NULL
-          || single_step_breakpoints[1] != NULL);
+  int i;
+
+  for (i = 0; i < NR_SINGLE_STEP_BREAKPOINTS; i++)
+    if (single_step_breakpoints[i] != NULL)
+      return 1;
+
+  return 0;
 }
 
 /* Remove and delete any breakpoints used for software single step.  */
@@ -14449,22 +14451,21 @@ single_step_breakpoints_inserted (void)
 void
 remove_single_step_breakpoints (void)
 {
+  int i;
+
   gdb_assert (single_step_breakpoints[0] != NULL);
 
   /* See insert_single_step_breakpoint for more about this deprecated
      call.  */
-  deprecated_remove_raw_breakpoint (single_step_gdbarch[0],
-				    single_step_breakpoints[0]);
-  single_step_gdbarch[0] = NULL;
-  single_step_breakpoints[0] = NULL;
 
-  if (single_step_breakpoints[1] != NULL)
-    {
-      deprecated_remove_raw_breakpoint (single_step_gdbarch[1],
-					single_step_breakpoints[1]);
-      single_step_gdbarch[1] = NULL;
-      single_step_breakpoints[1] = NULL;
-    }
+  for (i = 0; i < NR_SINGLE_STEP_BREAKPOINTS; i++)
+    if (single_step_breakpoints[i] != NULL)
+      {
+	deprecated_remove_raw_breakpoint (single_step_gdbarch[i],
+					  single_step_breakpoints[i]);
+	single_step_gdbarch[i] = NULL;
+	single_step_breakpoints[i] = NULL;
+      }
 }
 
 /* Delete software single step breakpoints without removing them from
@@ -14477,7 +14478,7 @@ cancel_single_step_breakpoints (void)
 {
   int i;
 
-  for (i = 0; i < 2; i++)
+  for (i = 0; i < NR_SINGLE_STEP_BREAKPOINTS; i++)
     if (single_step_breakpoints[i])
       {
 	xfree (single_step_breakpoints[i]);
@@ -14494,7 +14495,7 @@ detach_single_step_breakpoints (void)
 {
   int i;
 
-  for (i = 0; i < 2; i++)
+  for (i = 0; i < NR_SINGLE_STEP_BREAKPOINTS; i++)
     if (single_step_breakpoints[i])
       target_remove_breakpoint (single_step_gdbarch[i],
 				single_step_breakpoints[i]);
@@ -14509,7 +14510,7 @@ single_step_breakpoint_inserted_here_p (
 {
   int i;
 
-  for (i = 0; i < 2; i++)
+  for (i = 0; i < NR_SINGLE_STEP_BREAKPOINTS; i++)
     {
       struct bp_target_info *bp_tgt = single_step_breakpoints[i];
       if (bp_tgt
Index: gdb/gdb/breakpoint.h
===================================================================
--- gdb.orig/gdb/breakpoint.h	2012-06-06 12:59:47.829432024 +1000
+++ gdb/gdb/breakpoint.h	2012-06-06 12:59:57.417592947 +1000
@@ -1394,8 +1394,9 @@ extern int is_catchpoint (struct breakpo
    deletes all breakpoints.  */
 extern void delete_command (char *arg, int from_tty);
 
-/* Manage a software single step breakpoint (or two).  Insert may be
-   called twice before remove is called.  */
+/* Manage software single step breakpoints.  */
+#define NR_SINGLE_STEP_BREAKPOINTS 4
+
 extern void insert_single_step_breakpoint (struct gdbarch *,
 					   struct address_space *, 
 					   CORE_ADDR);

[PATCH 3/3] Add multiple branches to single step through atomic sequence testcase

[Anton Blanchard]

Test 3 conditional branches in an atomic sequence, 2 to the same
destination.

Anton
--

2012-06-05  Anton Blanchard  <anton@samba.org>

	* gdb.arch/ppc64-atomic-inst.s: Add second and third branch
	inside atomic sequence.

Index: gdb/gdb/testsuite/gdb.arch/ppc64-atomic-inst.s
===================================================================
--- gdb.orig/gdb/testsuite/gdb.arch/ppc64-atomic-inst.s	2012-06-06 12:59:56.697580862 +1000
+++ gdb/gdb/testsuite/gdb.arch/ppc64-atomic-inst.s	2012-06-06 13:46:08.258410718 +1000
@@ -10,17 +10,29 @@ gdbasm_declare main
 1:	lwarx	5,0,4
 	cmpwi	5,0
 	bne	2f
+	cmpwi	5,1
+	beq	3f
+	cmpwi	5,2
+	beq	3f	/* branch to same destination */
 	addi	5,5,1
 	stwcx.	5,0,4
 	bne	1b
 
-	std	0,0(4)
-2:	ldarx	5,0,4
+2:	nop
+
+3:	std	0,0(4)
+1:	ldarx	5,0,4
 	cmpdi	5,0
-	bne	3f
+	bne	2f
+	cmpdi	5,1
+	beq	3f
+	cmpwi	5,2
+	beq	3f	/* branch to same destination */
 	addi	5,5,1
 	stdcx.	5,0,4
 	bne	1b
 
+2:	nop
+
 3:	li	3,0
 	blr

Re: [PATCH 1/3] Fix ppc64 single step over atomic sequence testcase

[Joel Brobecker]

Hello Anton,

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.

> 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).

> 2012-06-05  Anton Blanchard  <anton@samba.org>
> 
> 	* 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 <http://www.gnu.org/licenses/>.  */
> -
> -#include <stdio.h>
> -
> -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

-- 
Joel

Re: [PATCH 1/3] Fix ppc64 single step over atomic sequence testcase

[Anton Blanchard]

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  <anton@samba.org>
> > 
> > 	* 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
> > <http://www.gnu.org/licenses/>.  */ -
> > -#include <stdio.h>
> > -
> > -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
> 

Re: [PATCH 1/3] Fix ppc64 single step over atomic sequence testcase

[Luis Gustavo]

Hi Anton,

On 06/12/2012 09:45 AM, Anton Blanchard wrote:
>> 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.

I authored that testcase a while ago. For the most common case we didn't 
have multiple branch instructions inside such a sequence IIRC, but if we 
have that now, the testcase should definitely be updated.

GDB is supposed to skip eacho sequence (__asm__ block) in response to a 
"next", but it may have had its behavior changed from when i originally 
wrote this. It seems to take multiple next's to get to a different 
__asm__ block now, so it seems to be broken.

It may be a difference in GCC's debuginfo output or GDB just got smarter.

Regards,
Luis

Re: [PATCH 1/3] Fix ppc64 single step over atomic sequence testcase

[Joel Brobecker]

> GDB is supposed to skip eacho sequence (__asm__ block) in response
> to a "next", but it may have had its behavior changed from when i
> originally wrote this. It seems to take multiple next's to get to a
> different __asm__ block now, so it seems to be broken.
> 
> It may be a difference in GCC's debuginfo output or GDB just got smarter.

I might be wrong, but I don't remember us changing anything in this
area. "next/step" behavior is really determined by the line info
generated by the compiler.  If the compiler was changed from not
generated a line entry for the __asm__ blocks, and now does, then
it'll change GDB's behavior.  If next/step behavior testing is
important to the testcase, then it would seem to make sense to
convert the testcase to using an assembly file instead.

-- 
Joel

Re: [PATCH 1/3] Fix ppc64 single step over atomic sequence testcase

[Luis Gustavo]

On 06/12/2012 10:16 AM, Joel Brobecker wrote:
>> GDB is supposed to skip eacho sequence (__asm__ block) in response
>> to a "next", but it may have had its behavior changed from when i
>> originally wrote this. It seems to take multiple next's to get to a
>> different __asm__ block now, so it seems to be broken.
>>
>> It may be a difference in GCC's debuginfo output or GDB just got smarter.
>
> I might be wrong, but I don't remember us changing anything in this
> area. "next/step" behavior is really determined by the line info
> generated by the compiler.  If the compiler was changed from not
> generated a line entry for the __asm__ blocks, and now does, then
> it'll change GDB's behavior.  If next/step behavior testing is
> important to the testcase, then it would seem to make sense to
> convert the testcase to using an assembly file instead.
>

I agree that using an assembly file is both more apropriate and more 
straightforward when it comes to testing these sequences.

Luis

Re: [PATCH 1/3] Fix ppc64 single step over atomic sequence testcase

[Edjunior Barbosa Machado]

Hi,

On 06/12/2012 10:28 AM, Luis Gustavo wrote:> On 06/12/2012 10:16 AM, Joel Brobecker wrote:

>>> GDB is supposed to skip eacho sequence (__asm__ block) in response
>>> to a "next", but it may have had its behavior changed from when i
>>> originally wrote this. It seems to take multiple next's to get to a
>>> different __asm__ block now, so it seems to be broken.
>>>
>>> It may be a difference in GCC's debuginfo output or GDB just got
>>> smarter.
>>
>> I might be wrong, but I don't remember us changing anything in this
>> area. "next/step" behavior is really determined by the line info
>> generated by the compiler.  If the compiler was changed from not
>> generated a line entry for the __asm__ blocks, and now does, then
>> it'll change GDB's behavior.  If next/step behavior testing is
>> important to the testcase, then it would seem to make sense to
>> convert the testcase to using an assembly file instead.
>>


It seems that's pretty much what's happening here. When checking the testcase built using gcc 4.3.4, got the following line number statements:

...
Special opcode 34: advance Address by 8 to 0x100005a8 and Line by 1 to 25
Special opcode 35: advance Address by 8 to 0x100005b0 and Line by 2 to 27
Special opcode 139: advance Address by 36 to 0x100005d4 and Line by 8 to 35
Special opcode 139: advance Address by 36 to 0x100005f8 and Line by 8 to 43
Special opcode 20: advance Address by 4 to 0x100005fc and Line by 1 to 44

and the tests passes with only 2 'next's, while gcc 4.7.0 (from Fedora 17) provides the following:

...
Special opcode 48: advance Address by 12 to 0x10000670 and Line by 1 to 25
Special opcode 67: advance Address by 16 to 0x10000680 and Line by 6 to 31
Special opcode 15: advance Address by 4 to 0x10000684 and Line by -4 to 27
Special opcode 24: advance Address by 4 to 0x10000688 and Line by 5 to 32
Special opcode 14: advance Address by 4 to 0x1000068c and Line by -5 to 27
Advance Line by 12 to 39
Special opcode 89: advance Address by 24 to 0x100006a4 and Line by 0 to 39
Special opcode 15: advance Address by 4 to 0x100006a8 and Line by -4 to 35
Special opcode 24: advance Address by 4 to 0x100006ac and Line by 5 to 40
Special opcode 14: advance Address by 4 to 0x100006b0 and Line by -5 to 35
Special opcode 111: advance Address by 28 to 0x100006cc and Line by 8 to 43
Special opcode 20: advance Address by 4 to 0x100006d0 and Line by 1 to 44

requiring more 'next's to go though the code.

> 
> I agree that using an assembly file is both more apropriate and more
> straightforward when it comes to testing these sequences.
> 
> Luis
> 
> 
> 


I had the chance to test the patch and rewriting this in assembly does fixes the tests regardless of gcc version. However, the only "issue" I noticed was that it should be run inside the src directory, otherwise it will fail due to the asm .include's:

 spawn -ignore SIGHUP gcc ../../../gdb.git/gdb/testsuite/gdb.arch/ppc64-atomic-inst.s -g -lm -o /home/emachado/gdb/build/gdb/testsuite/gdb.arch/ppc64-atomic-inst^M
../../../gdb.git/gdb/testsuite/gdb.arch/ppc64-atomic-inst.s: Assembler messages:^M
../../../gdb.git/gdb/testsuite/gdb.arch/ppc64-atomic-inst.s:1: Error: can't open gdb.asm/common.inc for reading: No such file or directory^M
../../../gdb.git/gdb/testsuite/gdb.arch/ppc64-atomic-inst.s:2: Error: can't open gdb.asm/powerpc64.inc for reading: No such file or directory^M
../../../gdb.git/gdb/testsuite/gdb.arch/ppc64-atomic-inst.s:5: Error: Unrecognized opcode: `gdbasm_declare'^M


Thanks and regards,
-- 
Edjunior

Re: [PATCH 1/3] Fix ppc64 single step over atomic sequence testcase

[Anton Blanchard]

Hi Edjunior,

> I had the chance to test the patch and rewriting this in assembly
> does fixes the tests regardless of gcc version. However, the only
> "issue" I noticed was that it should be run inside the src directory,
> otherwise it will fail due to the asm .include's:
> 
>  spawn -ignore SIGHUP
> gcc ../../../gdb.git/gdb/testsuite/gdb.arch/ppc64-atomic-inst.s -g
> -lm
> -o /home/emachado/gdb/build/gdb/testsuite/gdb.arch/ppc64-atomic-inst^M ../../../gdb.git/gdb/testsuite/gdb.arch/ppc64-atomic-inst.s:
> Assembler
> messages:^M ../../../gdb.git/gdb/testsuite/gdb.arch/ppc64-atomic-inst.s:1:
> Error: can't open gdb.asm/common.inc for reading: No such file or
> directory^M ../../../gdb.git/gdb/testsuite/gdb.arch/ppc64-atomic-inst.s:2:
> Error: can't open gdb.asm/powerpc64.inc for reading: No such file or
> directory^M ../../../gdb.git/gdb/testsuite/gdb.arch/ppc64-atomic-inst.s:5:
> Error: Unrecognized opcode: `gdbasm_declare'^M

Thanks for testing. It looks like no one else is using the asm macros
outside of gdb.asm. Any suggestions? I'm only using one macro so I
could open code it I guess.

Anton

Re: [PATCH 1/3] Fix ppc64 single step over atomic sequence testcase

[Edjunior Barbosa Machado]

Hi Anton,

On 06/12/2012 09:45 PM, Anton Blanchard wrote:

> Thanks for testing. It looks like no one else is using the asm macros
> outside of gdb.asm. Any suggestions? I'm only using one macro so I
> could open code it I guess.
> 


Well, I don't have any other idea besides use the expansion of
gdbasm_declare macro. From a quick test I've made here, it works ok.

Thanks and regards,
-- 
Edjunior

Re: [PATCH 1/3] Fix ppc64 single step over atomic sequence testcase

[Joel Brobecker]

> > Thanks for testing. It looks like no one else is using the asm macros
> > outside of gdb.asm. Any suggestions? I'm only using one macro so I
> > could open code it I guess.

Can you just add -I${srcdir}/${subdir} to the compilation flags,
and then .include "common.inc" instead of gdb.arch/common.inc?

-- 
Joel

Re: [PATCH 2/3] Support up to 3 conditional branches in an atomic sequence

[Joel Brobecker]

Anton,

Thanks for the patch. I am not a real specialist of the software
single-step implementation, even though I worked on it a really
long time ago. But your changes mostly make sense to me (and seem
to be relatively mechanical in nature).

> 2012-06-05  Anton Blanchard  <anton@samba.org>
> 
> 	* gdb/breakpoint.h: Define NR_SINGLE_STEP_BREAKPOINTS
> 	* rs6000-tdep.c (ppc_deal_with_atomic_sequence): Allow for more
> 	than two breakpoints.
> 	* gdb/breakpoint.c (insert_single_step_breakpoint): Likewise
> 	(insert_single_step_breakpoint): Likewise
> 	(single_step_breakpoints_inserted): Likewise
> 	(cancel_single_step_breakpoints): Likewise
> 	(detach_single_step_breakpoints): Likewise
> 	(single_step_breakpoint_inserted_here_p): Likewise

Mostly OK. I would like you to change the name of the macro to
MAX_SINGLE_STEP_BREAKPOINTS, though. Can you, please?

Other comments and questions inline.

> -  int bc_insn_count = 0; /* Conditional branch instruction count.  */

Thanks for deleting that variable that seems redundant with
last_breakpoint...

> +          if (last_breakpoint >= (NR_SINGLE_STEP_BREAKPOINTS-1))
> +            return 0; /* too many conditional branches found, fallback

Can you remove the extra parens which are useless in this case?
And binary operators should have a space before and after. Thus:

        if (last_breakpoint > MAX_SINGLE_STEP_BREAKPOINTS - 1)

> Index: gdb/gdb/breakpoint.h
[...]
> -/* Manage a software single step breakpoint (or two).  Insert may be
> -   called twice before remove is called.  */
> +/* Manage software single step breakpoints.  */
> +#define NR_SINGLE_STEP_BREAKPOINTS 4
> +

Just curious: Why did you remove the second sentence from the comment?
Is it no longer true? Maybe it is the "twice" that should be changed
into "multiple times"?

Thanks,
-- 
Joel

Re: [PATCH 3/3] Add multiple branches to single step through atomic sequence testcase

[Joel Brobecker]

> Test 3 conditional branches in an atomic sequence, 2 to the same
> destination.
[...]
> 2012-06-05  Anton Blanchard  <anton@samba.org>
> 
> 	* gdb.arch/ppc64-atomic-inst.s: Add second and third branch
> 	inside atomic sequence.

I won't pretend I understand PowerPC code all that well, I only know
the basics. My only concern with this change is whether you might have
reduced the number of conditions being tested. I would have expected
this  patch to *add* more code, and associated changes in the .exp
file.  But apparently, the code update is sufficient. How does the
change work? I see two labels used twice, for instance?

Thanks.

> 
> Index: gdb/gdb/testsuite/gdb.arch/ppc64-atomic-inst.s
> ===================================================================
> --- gdb.orig/gdb/testsuite/gdb.arch/ppc64-atomic-inst.s	2012-06-06 12:59:56.697580862 +1000
> +++ gdb/gdb/testsuite/gdb.arch/ppc64-atomic-inst.s	2012-06-06 13:46:08.258410718 +1000
> @@ -10,17 +10,29 @@ gdbasm_declare main
>  1:	lwarx	5,0,4
>  	cmpwi	5,0
>  	bne	2f
> +	cmpwi	5,1
> +	beq	3f
> +	cmpwi	5,2
> +	beq	3f	/* branch to same destination */
>  	addi	5,5,1
>  	stwcx.	5,0,4
>  	bne	1b
>  
> -	std	0,0(4)
> -2:	ldarx	5,0,4
> +2:	nop
> +
> +3:	std	0,0(4)
> +1:	ldarx	5,0,4
>  	cmpdi	5,0
> -	bne	3f
> +	bne	2f
> +	cmpdi	5,1
> +	beq	3f
> +	cmpwi	5,2
> +	beq	3f	/* branch to same destination */
>  	addi	5,5,1
>  	stdcx.	5,0,4
>  	bne	1b
>  
> +2:	nop
> +
>  3:	li	3,0
>  	blr

-- 
Joel

Re: [PATCH 1/3] Fix ppc64 single step over atomic sequence testcase

[Anton Blanchard]

Hi Joel,

> Can you just add -I${srcdir}/${subdir} to the compilation flags,
> and then .include "common.inc" instead of gdb.arch/common.inc?

Sorry for taking so long to respond. I tried that but srcdir wasn't an
absolute path so it failed. Using the existing asm macro was only
saving us a couple of lines so how about we just open code it?

Anton
--

The current ppc64 single step over atomic sequence testcase is broken.
Convert the test into assembly and use stepi to step through it.

2012-06-05  Anton Blanchard  <anton@samba.org>

	* 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: b/gdb/testsuite/gdb.arch/ppc64-atomic-inst.exp
===================================================================
--- a/gdb/testsuite/gdb.arch/ppc64-atomic-inst.exp
+++ b/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: b/gdb/testsuite/gdb.arch/ppc64-atomic-inst.c
===================================================================
--- a/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 <http://www.gnu.org/licenses/>.  */
-
-#include <stdio.h>
-
-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: b/gdb/testsuite/gdb.arch/ppc64-atomic-inst.s
===================================================================
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/ppc64-atomic-inst.s
@@ -0,0 +1,29 @@
+	.global main
+	.section	".opd","aw"
+main:
+	.quad .main, .TOC.@tocbase, 0
+	.section	".text"
+	.type	main, @function
+.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

Re: [PATCH 2/3] Support up to 3 conditional branches in an atomic sequence

[Anton Blanchard]

Hi Joel,

Thanks for the review.

> Mostly OK. I would like you to change the name of the macro to
> MAX_SINGLE_STEP_BREAKPOINTS, though. Can you, please?

Makes sense, done.

> > +          if (last_breakpoint >= (NR_SINGLE_STEP_BREAKPOINTS-1))
> > +            return 0; /* too many conditional branches found,
> > fallback
> 
> Can you remove the extra parens which are useless in this case?
> And binary operators should have a space before and after.

Done.

> > Index: gdb/gdb/breakpoint.h
> [...]
> > -/* Manage a software single step breakpoint (or two).  Insert may
> > be
> > -   called twice before remove is called.  */
> > +/* Manage software single step breakpoints.  */
> > +#define NR_SINGLE_STEP_BREAKPOINTS 4
> > +
> 
> Just curious: Why did you remove the second sentence from the comment?
> Is it no longer true? Maybe it is the "twice" that should be changed
> into "multiple times"?

Agreed. Here is a new version.

Anton
--

gdb currently supports 1 conditional branch inside a ppc larx/stcx
critical region. Unfortunately there is existing code that contains more
than 1, for example in the ppc64 Linux kernel:

c00000000003ac18 <.__hash_page_4K>:
...
c00000000003ac4c:       7f e0 30 a8     ldarx   r31,0,r6
c00000000003ac50:       7c 80 f8 79     andc.   r0,r4,r31
c00000000003ac54:       40 82 02 94     bne-    c00000000003aee8 <htab_wrong_access>
c00000000003ac58:       73 e0 08 00     andi.   r0,r31,2048
c00000000003ac5c:       40 82 01 b0     bne-    c00000000003ae0c <htab_bail_ok>
c00000000003ac60:       54 9e f6 30     rlwinm  r30,r4,30,24,24
c00000000003ac64:       7f de fb 78     or      r30,r30,r31
c00000000003ac68:       63 de 09 00     ori     r30,r30,2304
c00000000003ac6c:       67 de 10 00     oris    r30,r30,4096
c00000000003ac70:       7f c0 31 ad     stdcx.  r30,0,r6

If we try to single step through this we get stuck forever because
the reservation is never set when we step over the stdcx.

The following patch bumps the number to 3 conditional branches + 1
terminating branch. With this patch applied I can single step through
the offending function in the ppc64 Linux kernel.

2012-06-05  Anton Blanchard  <anton@samba.org>

	* gdb/breakpoint.h: Define MAX_SINGLE_STEP_BREAKPOINTS
	* rs6000-tdep.c (ppc_deal_with_atomic_sequence): Allow for more
	than two breakpoints.
	* gdb/breakpoint.c (insert_single_step_breakpoint): Likewise
	(insert_single_step_breakpoint): Likewise
	(single_step_breakpoints_inserted): Likewise
	(cancel_single_step_breakpoints): Likewise
	(detach_single_step_breakpoints): Likewise
	(single_step_breakpoint_inserted_here_p): Likewise

Index: b/gdb/rs6000-tdep.c
===================================================================
--- a/gdb/rs6000-tdep.c
+++ b/gdb/rs6000-tdep.c
@@ -1087,7 +1087,7 @@ ppc_deal_with_atomic_sequence (struct fr
   struct address_space *aspace = get_frame_address_space (frame);
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   CORE_ADDR pc = get_frame_pc (frame);
-  CORE_ADDR breaks[2] = {-1, -1};
+  CORE_ADDR breaks[MAX_SINGLE_STEP_BREAKPOINTS];
   CORE_ADDR loc = pc;
   CORE_ADDR closing_insn; /* Instruction that closes the atomic sequence.  */
   int insn = read_memory_integer (loc, PPC_INSN_SIZE, byte_order);
@@ -1096,7 +1096,6 @@ ppc_deal_with_atomic_sequence (struct fr
   int last_breakpoint = 0; /* Defaults to 0 (no breakpoints placed).  */  
   const int atomic_sequence_length = 16; /* Instruction sequence length.  */
   int opcode; /* Branch instruction's OPcode.  */
-  int bc_insn_count = 0; /* Conditional branch instruction count.  */
 
   /* Assume all atomic sequences start with a lwarx/ldarx instruction.  */
   if ((insn & LWARX_MASK) != LWARX_INSTRUCTION
@@ -1110,24 +1109,20 @@ ppc_deal_with_atomic_sequence (struct fr
       loc += PPC_INSN_SIZE;
       insn = read_memory_integer (loc, PPC_INSN_SIZE, byte_order);
 
-      /* Assume that there is at most one conditional branch in the atomic
-         sequence.  If a conditional branch is found, put a breakpoint in 
-         its destination address.  */
       if ((insn & BRANCH_MASK) == BC_INSN)
         {
           int immediate = ((insn & 0xfffc) ^ 0x8000) - 0x8000;
           int absolute = insn & 2;
 
-          if (bc_insn_count >= 1)
-            return 0; /* More than one conditional branch found, fallback 
+          if (last_breakpoint >= MAX_SINGLE_STEP_BREAKPOINTS - 1)
+            return 0; /* too many conditional branches found, fallback
                          to the standard single-step code.  */
  
 	  if (absolute)
-	    breaks[1] = immediate;
+	    breaks[last_breakpoint] = immediate;
 	  else
-	    breaks[1] = loc + immediate;
+	    breaks[last_breakpoint] = loc + immediate;
 
-	  bc_insn_count++;
 	  last_breakpoint++;
         }
 
@@ -1146,18 +1141,29 @@ ppc_deal_with_atomic_sequence (struct fr
   insn = read_memory_integer (loc, PPC_INSN_SIZE, byte_order);
 
   /* Insert a breakpoint right after the end of the atomic sequence.  */
-  breaks[0] = loc;
+  breaks[last_breakpoint] = loc;
 
-  /* Check for duplicated breakpoints.  Check also for a breakpoint
-     placed (branch instruction's destination) anywhere in sequence.  */
-  if (last_breakpoint
-      && (breaks[1] == breaks[0]
-	  || (breaks[1] >= pc && breaks[1] <= closing_insn)))
-    last_breakpoint = 0;
-
-  /* Effectively inserts the breakpoints.  */
   for (index = 0; index <= last_breakpoint; index++)
-    insert_single_step_breakpoint (gdbarch, aspace, breaks[index]);
+    {
+      int index2;
+      int insert_bp = 1;
+
+      /* Check for a breakpoint placed (branch instruction's destination)
+	 anywhere in sequence.  */
+      if (breaks[index] >= pc && breaks[index] <= closing_insn)
+	continue;
+
+      /* Check for duplicated breakpoints.  */
+      for (index2 = 0; index2 < index; index2++)
+        {
+	  if (breaks[index] == breaks[index2])
+	    insert_bp = 0;
+	}
+
+      /* insert the breakpoint.  */
+      if (insert_bp)
+        insert_single_step_breakpoint (gdbarch, aspace, breaks[index]);
+    }
 
   return 1;
 }
Index: b/gdb/breakpoint.c
===================================================================
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -14741,11 +14741,10 @@ deprecated_remove_raw_breakpoint (struct
   return ret;
 }
 
-/* One (or perhaps two) breakpoints used for software single
-   stepping.  */
+/* Breakpoints used for software single stepping.  */
 
-static void *single_step_breakpoints[2];
-static struct gdbarch *single_step_gdbarch[2];
+static void *single_step_breakpoints[MAX_SINGLE_STEP_BREAKPOINTS];
+static struct gdbarch *single_step_gdbarch[MAX_SINGLE_STEP_BREAKPOINTS];
 
 /* Create and insert a breakpoint for software single step.  */
 
@@ -14754,19 +14753,17 @@ insert_single_step_breakpoint (struct gd
 			       struct address_space *aspace, 
 			       CORE_ADDR next_pc)
 {
+  int i;
   void **bpt_p;
 
-  if (single_step_breakpoints[0] == NULL)
-    {
-      bpt_p = &single_step_breakpoints[0];
-      single_step_gdbarch[0] = gdbarch;
-    }
-  else
-    {
-      gdb_assert (single_step_breakpoints[1] == NULL);
-      bpt_p = &single_step_breakpoints[1];
-      single_step_gdbarch[1] = gdbarch;
-    }
+  for (i = 0; i < MAX_SINGLE_STEP_BREAKPOINTS; i++)
+    if (single_step_breakpoints[i] == NULL)
+        break;
+
+  gdb_assert (i < MAX_SINGLE_STEP_BREAKPOINTS);
+
+  bpt_p = &single_step_breakpoints[i];
+  single_step_gdbarch[i] = gdbarch;
 
   /* NOTE drow/2006-04-11: A future improvement to this function would
      be to only create the breakpoints once, and actually put them on
@@ -14787,8 +14784,13 @@ insert_single_step_breakpoint (struct gd
 int
 single_step_breakpoints_inserted (void)
 {
-  return (single_step_breakpoints[0] != NULL
-          || single_step_breakpoints[1] != NULL);
+  int i;
+
+  for (i = 0; i < MAX_SINGLE_STEP_BREAKPOINTS; i++)
+    if (single_step_breakpoints[i] != NULL)
+      return 1;
+
+  return 0;
 }
 
 /* Remove and delete any breakpoints used for software single step.  */
@@ -14796,22 +14798,21 @@ single_step_breakpoints_inserted (void)
 void
 remove_single_step_breakpoints (void)
 {
+  int i;
+
   gdb_assert (single_step_breakpoints[0] != NULL);
 
   /* See insert_single_step_breakpoint for more about this deprecated
      call.  */
-  deprecated_remove_raw_breakpoint (single_step_gdbarch[0],
-				    single_step_breakpoints[0]);
-  single_step_gdbarch[0] = NULL;
-  single_step_breakpoints[0] = NULL;
 
-  if (single_step_breakpoints[1] != NULL)
-    {
-      deprecated_remove_raw_breakpoint (single_step_gdbarch[1],
-					single_step_breakpoints[1]);
-      single_step_gdbarch[1] = NULL;
-      single_step_breakpoints[1] = NULL;
-    }
+  for (i = 0; i < MAX_SINGLE_STEP_BREAKPOINTS; i++)
+    if (single_step_breakpoints[i] != NULL)
+      {
+	deprecated_remove_raw_breakpoint (single_step_gdbarch[i],
+					  single_step_breakpoints[i]);
+	single_step_gdbarch[i] = NULL;
+	single_step_breakpoints[i] = NULL;
+      }
 }
 
 /* Delete software single step breakpoints without removing them from
@@ -14824,7 +14825,7 @@ cancel_single_step_breakpoints (void)
 {
   int i;
 
-  for (i = 0; i < 2; i++)
+  for (i = 0; i < MAX_SINGLE_STEP_BREAKPOINTS; i++)
     if (single_step_breakpoints[i])
       {
 	xfree (single_step_breakpoints[i]);
@@ -14841,7 +14842,7 @@ detach_single_step_breakpoints (void)
 {
   int i;
 
-  for (i = 0; i < 2; i++)
+  for (i = 0; i < MAX_SINGLE_STEP_BREAKPOINTS; i++)
     if (single_step_breakpoints[i])
       target_remove_breakpoint (single_step_gdbarch[i],
 				single_step_breakpoints[i]);
@@ -14856,7 +14857,7 @@ single_step_breakpoint_inserted_here_p (
 {
   int i;
 
-  for (i = 0; i < 2; i++)
+  for (i = 0; i < MAX_SINGLE_STEP_BREAKPOINTS; i++)
     {
       struct bp_target_info *bp_tgt = single_step_breakpoints[i];
       if (bp_tgt
Index: b/gdb/breakpoint.h
===================================================================
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -1408,8 +1408,10 @@ extern int is_catchpoint (struct breakpo
    deletes all breakpoints.  */
 extern void delete_command (char *arg, int from_tty);
 
-/* Manage a software single step breakpoint (or two).  Insert may be
-   called twice before remove is called.  */
+/* Manage software single step breakpoints.  Insert may be
+   called multiple times before remove is called.  */
+#define MAX_SINGLE_STEP_BREAKPOINTS 4
+
 extern void insert_single_step_breakpoint (struct gdbarch *,
 					   struct address_space *, 
 					   CORE_ADDR);

Re: [PATCH 3/3] Add multiple branches to single step through atomic sequence testcase

[Anton Blanchard]

Hi Joel,

> I won't pretend I understand PowerPC code all that well, I only know
> the basics. My only concern with this change is whether you might have
> reduced the number of conditions being tested. I would have expected
> this  patch to *add* more code, and associated changes in the .exp
> file.  But apparently, the code update is sufficient. How does the
> change work? I see two labels used twice, for instance?

We don't have to update the .exp file because the entire larx/stcx
appears as a single stepi, regardless of what we put inside it. I added
the duplicate labels to test the duplicate branch detection in the
previous patch:

+      /* Check for duplicated breakpoints.  */
+      for (index2 = 0; index2 < index; index2++)

The gdb single step code has always handled this but since I was
shifting code around I figured it was a good idea to test it.

Anton

Re: [PATCH 1/3] Fix ppc64 single step over atomic sequence testcase

[Edjunior Barbosa Machado]

On 07/16/2012 03:23 AM, Anton Blanchard wrote:
> Sorry for taking so long to respond. I tried that but srcdir wasn't an
> absolute path so it failed. Using the existing asm macro was only
> saving us a couple of lines so how about we just open code it?

Hi Anton,

I had the chance to test this latest version of your patch and it worked
fine.

Joel, Luis, any comments? Is it ok to commit?

Thanks,
-- 
Edjunior

Re: [PATCH 1/3] Fix ppc64 single step over atomic sequence testcase

[Luis Gustavo]

On 09/26/2012 08:20 PM, Edjunior Barbosa Machado wrote:
> On 07/16/2012 03:23 AM, Anton Blanchard wrote:
>> Sorry for taking so long to respond. I tried that but srcdir wasn't an
>> absolute path so it failed. Using the existing asm macro was only
>> saving us a couple of lines so how about we just open code it?
>
> Hi Anton,
>
> I had the chance to test this latest version of your patch and it worked
> fine.
>
> Joel, Luis, any comments? Is it ok to commit?
>
> Thanks,

Looks good to me.

Re: [PATCH 1/3] Fix ppc64 single step over atomic sequence testcase

[Joel Brobecker]

> > Can you just add -I${srcdir}/${subdir} to the compilation flags,
> > and then .include "common.inc" instead of gdb.arch/common.inc?
> 
> Sorry for taking so long to respond. I tried that but srcdir wasn't an
> absolute path so it failed. Using the existing asm macro was only
> saving us a couple of lines so how about we just open code it?

I'd rather not. I know it's only a few lines, but I'm sure these files
are there for a reason.

I think it might have failed because ${srcdir}/${subdir} wasn't
the right path. Your testcase is in gdb.arch, so I am guessing
that subdir was "gdb.arch". However, your include file is in gdb.asm.
So instead, use -I${srcdir}/gdb.asm instead.

-- 
Joel

Re: [PATCH 2/3] Support up to 3 conditional branches in an atomic sequence

[Joel Brobecker]

> Here is a new version.
[...]
> 2012-06-05  Anton Blanchard  <anton@samba.org>
> 
> 	* gdb/breakpoint.h: Define MAX_SINGLE_STEP_BREAKPOINTS
> 	* rs6000-tdep.c (ppc_deal_with_atomic_sequence): Allow for more
> 	than two breakpoints.
> 	* gdb/breakpoint.c (insert_single_step_breakpoint): Likewise
> 	(insert_single_step_breakpoint): Likewise
> 	(single_step_breakpoints_inserted): Likewise
> 	(cancel_single_step_breakpoints): Likewise
> 	(detach_single_step_breakpoints): Likewise
> 	(single_step_breakpoint_inserted_here_p): Likewise

Thanks for sending the new version, and sorry for dropping the ball on
this one.

This version of the patch is OK.

> Index: b/gdb/rs6000-tdep.c
> ===================================================================
> --- a/gdb/rs6000-tdep.c
> +++ b/gdb/rs6000-tdep.c
> @@ -1087,7 +1087,7 @@ ppc_deal_with_atomic_sequence (struct fr
>    struct address_space *aspace = get_frame_address_space (frame);
>    enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
>    CORE_ADDR pc = get_frame_pc (frame);
> -  CORE_ADDR breaks[2] = {-1, -1};
> +  CORE_ADDR breaks[MAX_SINGLE_STEP_BREAKPOINTS];
>    CORE_ADDR loc = pc;
>    CORE_ADDR closing_insn; /* Instruction that closes the atomic sequence.  */
>    int insn = read_memory_integer (loc, PPC_INSN_SIZE, byte_order);
> @@ -1096,7 +1096,6 @@ ppc_deal_with_atomic_sequence (struct fr
>    int last_breakpoint = 0; /* Defaults to 0 (no breakpoints placed).  */  
>    const int atomic_sequence_length = 16; /* Instruction sequence length.  */
>    int opcode; /* Branch instruction's OPcode.  */
> -  int bc_insn_count = 0; /* Conditional branch instruction count.  */
>  
>    /* Assume all atomic sequences start with a lwarx/ldarx instruction.  */
>    if ((insn & LWARX_MASK) != LWARX_INSTRUCTION
> @@ -1110,24 +1109,20 @@ ppc_deal_with_atomic_sequence (struct fr
>        loc += PPC_INSN_SIZE;
>        insn = read_memory_integer (loc, PPC_INSN_SIZE, byte_order);
>  
> -      /* Assume that there is at most one conditional branch in the atomic
> -         sequence.  If a conditional branch is found, put a breakpoint in 
> -         its destination address.  */
>        if ((insn & BRANCH_MASK) == BC_INSN)
>          {
>            int immediate = ((insn & 0xfffc) ^ 0x8000) - 0x8000;
>            int absolute = insn & 2;
>  
> -          if (bc_insn_count >= 1)
> -            return 0; /* More than one conditional branch found, fallback 
> +          if (last_breakpoint >= MAX_SINGLE_STEP_BREAKPOINTS - 1)
> +            return 0; /* too many conditional branches found, fallback
>                           to the standard single-step code.  */
>   
>  	  if (absolute)
> -	    breaks[1] = immediate;
> +	    breaks[last_breakpoint] = immediate;
>  	  else
> -	    breaks[1] = loc + immediate;
> +	    breaks[last_breakpoint] = loc + immediate;
>  
> -	  bc_insn_count++;
>  	  last_breakpoint++;
>          }
>  
> @@ -1146,18 +1141,29 @@ ppc_deal_with_atomic_sequence (struct fr
>    insn = read_memory_integer (loc, PPC_INSN_SIZE, byte_order);
>  
>    /* Insert a breakpoint right after the end of the atomic sequence.  */
> -  breaks[0] = loc;
> +  breaks[last_breakpoint] = loc;
>  
> -  /* Check for duplicated breakpoints.  Check also for a breakpoint
> -     placed (branch instruction's destination) anywhere in sequence.  */
> -  if (last_breakpoint
> -      && (breaks[1] == breaks[0]
> -	  || (breaks[1] >= pc && breaks[1] <= closing_insn)))
> -    last_breakpoint = 0;
> -
> -  /* Effectively inserts the breakpoints.  */
>    for (index = 0; index <= last_breakpoint; index++)
> -    insert_single_step_breakpoint (gdbarch, aspace, breaks[index]);
> +    {
> +      int index2;
> +      int insert_bp = 1;
> +
> +      /* Check for a breakpoint placed (branch instruction's destination)
> +	 anywhere in sequence.  */
> +      if (breaks[index] >= pc && breaks[index] <= closing_insn)
> +	continue;
> +
> +      /* Check for duplicated breakpoints.  */
> +      for (index2 = 0; index2 < index; index2++)
> +        {
> +	  if (breaks[index] == breaks[index2])
> +	    insert_bp = 0;
> +	}
> +
> +      /* insert the breakpoint.  */
> +      if (insert_bp)
> +        insert_single_step_breakpoint (gdbarch, aspace, breaks[index]);
> +    }
>  
>    return 1;
>  }
> Index: b/gdb/breakpoint.c
> ===================================================================
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -14741,11 +14741,10 @@ deprecated_remove_raw_breakpoint (struct
>    return ret;
>  }
>  
> -/* One (or perhaps two) breakpoints used for software single
> -   stepping.  */
> +/* Breakpoints used for software single stepping.  */
>  
> -static void *single_step_breakpoints[2];
> -static struct gdbarch *single_step_gdbarch[2];
> +static void *single_step_breakpoints[MAX_SINGLE_STEP_BREAKPOINTS];
> +static struct gdbarch *single_step_gdbarch[MAX_SINGLE_STEP_BREAKPOINTS];
>  
>  /* Create and insert a breakpoint for software single step.  */
>  
> @@ -14754,19 +14753,17 @@ insert_single_step_breakpoint (struct gd
>  			       struct address_space *aspace, 
>  			       CORE_ADDR next_pc)
>  {
> +  int i;
>    void **bpt_p;
>  
> -  if (single_step_breakpoints[0] == NULL)
> -    {
> -      bpt_p = &single_step_breakpoints[0];
> -      single_step_gdbarch[0] = gdbarch;
> -    }
> -  else
> -    {
> -      gdb_assert (single_step_breakpoints[1] == NULL);
> -      bpt_p = &single_step_breakpoints[1];
> -      single_step_gdbarch[1] = gdbarch;
> -    }
> +  for (i = 0; i < MAX_SINGLE_STEP_BREAKPOINTS; i++)
> +    if (single_step_breakpoints[i] == NULL)
> +        break;
> +
> +  gdb_assert (i < MAX_SINGLE_STEP_BREAKPOINTS);
> +
> +  bpt_p = &single_step_breakpoints[i];
> +  single_step_gdbarch[i] = gdbarch;
>  
>    /* NOTE drow/2006-04-11: A future improvement to this function would
>       be to only create the breakpoints once, and actually put them on
> @@ -14787,8 +14784,13 @@ insert_single_step_breakpoint (struct gd
>  int
>  single_step_breakpoints_inserted (void)
>  {
> -  return (single_step_breakpoints[0] != NULL
> -          || single_step_breakpoints[1] != NULL);
> +  int i;
> +
> +  for (i = 0; i < MAX_SINGLE_STEP_BREAKPOINTS; i++)
> +    if (single_step_breakpoints[i] != NULL)
> +      return 1;
> +
> +  return 0;
>  }
>  
>  /* Remove and delete any breakpoints used for software single step.  */
> @@ -14796,22 +14798,21 @@ single_step_breakpoints_inserted (void)
>  void
>  remove_single_step_breakpoints (void)
>  {
> +  int i;
> +
>    gdb_assert (single_step_breakpoints[0] != NULL);
>  
>    /* See insert_single_step_breakpoint for more about this deprecated
>       call.  */
> -  deprecated_remove_raw_breakpoint (single_step_gdbarch[0],
> -				    single_step_breakpoints[0]);
> -  single_step_gdbarch[0] = NULL;
> -  single_step_breakpoints[0] = NULL;
>  
> -  if (single_step_breakpoints[1] != NULL)
> -    {
> -      deprecated_remove_raw_breakpoint (single_step_gdbarch[1],
> -					single_step_breakpoints[1]);
> -      single_step_gdbarch[1] = NULL;
> -      single_step_breakpoints[1] = NULL;
> -    }
> +  for (i = 0; i < MAX_SINGLE_STEP_BREAKPOINTS; i++)
> +    if (single_step_breakpoints[i] != NULL)
> +      {
> +	deprecated_remove_raw_breakpoint (single_step_gdbarch[i],
> +					  single_step_breakpoints[i]);
> +	single_step_gdbarch[i] = NULL;
> +	single_step_breakpoints[i] = NULL;
> +      }
>  }
>  
>  /* Delete software single step breakpoints without removing them from
> @@ -14824,7 +14825,7 @@ cancel_single_step_breakpoints (void)
>  {
>    int i;
>  
> -  for (i = 0; i < 2; i++)
> +  for (i = 0; i < MAX_SINGLE_STEP_BREAKPOINTS; i++)
>      if (single_step_breakpoints[i])
>        {
>  	xfree (single_step_breakpoints[i]);
> @@ -14841,7 +14842,7 @@ detach_single_step_breakpoints (void)
>  {
>    int i;
>  
> -  for (i = 0; i < 2; i++)
> +  for (i = 0; i < MAX_SINGLE_STEP_BREAKPOINTS; i++)
>      if (single_step_breakpoints[i])
>        target_remove_breakpoint (single_step_gdbarch[i],
>  				single_step_breakpoints[i]);
> @@ -14856,7 +14857,7 @@ single_step_breakpoint_inserted_here_p (
>  {
>    int i;
>  
> -  for (i = 0; i < 2; i++)
> +  for (i = 0; i < MAX_SINGLE_STEP_BREAKPOINTS; i++)
>      {
>        struct bp_target_info *bp_tgt = single_step_breakpoints[i];
>        if (bp_tgt
> Index: b/gdb/breakpoint.h
> ===================================================================
> --- a/gdb/breakpoint.h
> +++ b/gdb/breakpoint.h
> @@ -1408,8 +1408,10 @@ extern int is_catchpoint (struct breakpo
>     deletes all breakpoints.  */
>  extern void delete_command (char *arg, int from_tty);
>  
> -/* Manage a software single step breakpoint (or two).  Insert may be
> -   called twice before remove is called.  */
> +/* Manage software single step breakpoints.  Insert may be
> +   called multiple times before remove is called.  */
> +#define MAX_SINGLE_STEP_BREAKPOINTS 4
> +
>  extern void insert_single_step_breakpoint (struct gdbarch *,
>  					   struct address_space *, 
>  					   CORE_ADDR);

-- 
Joel

Re: [PATCH 3/3] Add multiple branches to single step through atomic sequence testcase

[Joel Brobecker]

> 2012-06-05  Anton Blanchard  <anton@samba.org>
> 
> 	* gdb.arch/ppc64-atomic-inst.s: Add second and third branch
> 	inside atomic sequence.

Luis gave his approval, so this patch is OK.

-- 
Joel

Re: [PATCH 2/3] Support up to 3 conditional branches in an atomic sequence

[Pedro Alves]

Hi Anton,

Just some notes on the ChangeLog formatting:

On 07/16/2012 07:33 AM, Anton Blanchard wrote:

> 2012-06-05  Anton Blanchard  <anton@samba.org>
> 
> 	* gdb/breakpoint.h: Define MAX_SINGLE_STEP_BREAKPOINTS
> 	* rs6000-tdep.c (ppc_deal_with_atomic_sequence): Allow for more
> 	than two breakpoints.
> 	* gdb/breakpoint.c (insert_single_step_breakpoint): Likewise
> 	(insert_single_step_breakpoint): Likewise
> 	(single_step_breakpoints_inserted): Likewise
> 	(cancel_single_step_breakpoints): Likewise
> 	(detach_single_step_breakpoints): Likewise
> 	(single_step_breakpoint_inserted_here_p): Likewise

- Please add a period at the end of these sentences.
- Remove the gdb/ from the breakpoint.h|c entries.

I suggest:

2012-09-28  Anton Blanchard  <anton@samba.org>

	* breakpoint.h (MAX_SINGLE_STEP_BREAKPOINTS): New define.
	* rs6000-tdep.c (ppc_deal_with_atomic_sequence): Allow for more
	than two breakpoints.
	* breakpoint.c (insert_single_step_breakpoint): Likewise.
	(insert_single_step_breakpoint): Likewise.
	(single_step_breakpoints_inserted): Likewise.
	(cancel_single_step_breakpoints): Likewise.
	(detach_single_step_breakpoints): Likewise.
	(single_step_breakpoint_inserted_here_p): Likewise.

-- 
Pedro Alves

Re: [PATCH 1/3] Fix ppc64 single step over atomic sequence testcase.

[Ulrich Weigand]

Anton Blanchard wrote:

> gdb/testsuite/
> 2014-03-31  Anton Blanchard  <anton@samba.org>
> 
> 	* 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.

This is OK.

Thanks,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com

[PATCH 1/3] Fix ppc64 single step over atomic sequence testcase.

[Anton Blanchard]

The current ppc64 single step over atomic sequence testcase is written
in C and breaks with some versions of gcc. Convert the test to
assembly and use stepi to step through it.

gdb/testsuite/
2014-03-31  Anton Blanchard  <anton@samba.org>

	* 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.
---
 gdb/testsuite/gdb.arch/ppc64-atomic-inst.S   | 61 ++++++++++++++++++++++++++++
 gdb/testsuite/gdb.arch/ppc64-atomic-inst.c   | 44 --------------------
 gdb/testsuite/gdb.arch/ppc64-atomic-inst.exp | 15 +++++--
 3 files changed, 72 insertions(+), 48 deletions(-)
 create mode 100644 gdb/testsuite/gdb.arch/ppc64-atomic-inst.S
 delete mode 100644 gdb/testsuite/gdb.arch/ppc64-atomic-inst.c

diff --git a/gdb/testsuite/gdb.arch/ppc64-atomic-inst.S b/gdb/testsuite/gdb.arch/ppc64-atomic-inst.S
new file mode 100644
index 0000000..15ccfd9
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/ppc64-atomic-inst.S
@@ -0,0 +1,61 @@
+/* This file is part of GDB, the GNU debugger.
+
+   Copyright 2008-2014 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 <http://www.gnu.org/licenses/>.  */
+
+	.align 2
+	.globl main
+#if _CALL_ELF == 2
+	.type main,@function
+main:
+#else
+	.section ".opd","aw"
+	.align 3
+main:
+	.quad .main,.TOC.@tocbase,0
+	.size main,.-main
+	.previous
+	.globl .main
+	.type .main,@function
+.main:
+#endif
+
+	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
+
+#if _CALL_ELF == 2
+	.size main,.-main
+#else
+	.size .main,.-.main
+#endif
diff --git a/gdb/testsuite/gdb.arch/ppc64-atomic-inst.c b/gdb/testsuite/gdb.arch/ppc64-atomic-inst.c
deleted file mode 100644
index 303e383..0000000
--- a/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-2014 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 <http://www.gnu.org/licenses/>.  */
-
-#include <stdio.h>
-
-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;
-}
diff --git a/gdb/testsuite/gdb.arch/ppc64-atomic-inst.exp b/gdb/testsuite/gdb.arch/ppc64-atomic-inst.exp
index f5f3b40..cefdfc9 100644
--- a/gdb/testsuite/gdb.arch/ppc64-atomic-inst.exp
+++ b/gdb/testsuite/gdb.arch/ppc64-atomic-inst.exp
@@ -27,7 +27,7 @@ if {![istarget "powerpc*"] || ![is_lp64_target]} {
 }
 
 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"
-- 
1.8.3.2

Re: [PATCH 1/3] Fix ppc64 single step over atomic sequence testcase

[Anton Blanchard]

Hi Uli,

> I think it might be best to just get rid of those .include statements;
> depending on an .inc file from another directory seems surprising.
> 
> It seems you only need it for the gdbasm_declare macro; since this
> file is ppc64 specific anyway, why don't you just hard-code the
> .opd generation in this source file?

Considering the issues we have had with getting the correct include
paths passed into the assembler, I tend to agree. It only saves a couple
of lines.

> Also, the assembler source file probably ought to keep the
> copyright header.   A comment why this test needs to use
> assembler source also would be good.

Thanks, will incorporate that into the next spin.

Anton

Re: [PATCH 1/3] Fix ppc64 single step over atomic sequence testcase

[Ulrich Weigand]

Anton Blanchard wrote:

> > This seems to happen because -I option is not passed to the
> > assembler. I've tried adding this parameter using -Wa as:
> > 
> > set compile_flags "debug quiet
> >     additional_flags=-Wa,-I${srcdir}/gdb.asm"
> > 
> > and it fixes the build failure. With this change, the testcase passes
> > OK, also including the other 2 patches you sent on this thread.
> 
> Thanks for the fix! Incorporated below.

I think it might be best to just get rid of those .include statements;
depending on an .inc file from another directory seems surprising.

It seems you only need it for the gdbasm_declare macro; since this
file is ppc64 specific anyway, why don't you just hard-code the
.opd generation in this source file?

Also, the assembler source file probably ought to keep the
copyright header.   A comment why this test needs to use
assembler source also would be good.

Thanks,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com

Re: [PATCH 1/3] Fix ppc64 single step over atomic sequence testcase

[Anton Blanchard]

Hi Edjunior,

> This seems to happen because -I option is not passed to the
> assembler. I've tried adding this parameter using -Wa as:
> 
> set compile_flags "debug quiet
>     additional_flags=-Wa,-I${srcdir}/gdb.asm"
> 
> and it fixes the build failure. With this change, the testcase passes
> OK, also including the other 2 patches you sent on this thread.

Thanks for the fix! Incorporated below.

Anton
--

The current ppc64 single step over atomic sequence testcase is broken.
Convert the test into assembly and use stepi to step through it.

--

2013-07-29  Anton Blanchard  <anton@samba.org>

	* 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: b/gdb/testsuite/gdb.arch/ppc64-atomic-inst.exp
===================================================================
--- a/gdb/testsuite/gdb.arch/ppc64-atomic-inst.exp
+++ b/gdb/testsuite/gdb.arch/ppc64-atomic-inst.exp
@@ -27,9 +27,9 @@ 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}
+set compile_flags "debug quiet additional_flags=-Wa,-I${srcdir}/gdb.asm"
 
 if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable $compile_flags] != "" } {
     unsupported "Testcase compile failed."
@@ -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: b/gdb/testsuite/gdb.arch/ppc64-atomic-inst.c
===================================================================
--- a/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-2013 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 <http://www.gnu.org/licenses/>.  */
-
-#include <stdio.h>
-
-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: b/gdb/testsuite/gdb.arch/ppc64-atomic-inst.s
===================================================================
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/ppc64-atomic-inst.s
@@ -0,0 +1,26 @@
+        .include "common.inc"
+        .include "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

Re: [PATCH 1/3] Fix ppc64 single step over atomic sequence testcase

[Edjunior Barbosa Machado]

On 07/29/2013 04:38 AM, Anton Blanchard wrote:
> 
> (My apologies, I thought I'd got these patches merged but obviously I
> hadn't).
> 
> The current ppc64 single step over atomic sequence testcase is broken.
> Convert the test into assembly and use stepi to step through it.

Hi Anton,

thanks for reworking this patch! I do recall we discussed this in the
mailing list last year
(http://sourceware.org/ml/gdb-patches/2012-06/msg00162.html).

> 
> Anton
> --
> 
> 2013-07-29  Anton Blanchard  <anton@samba.org>
> 
> 	* 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 
> 

(just a nitpick: missing period at the end of the lines.)

> 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,9 +27,9 @@ 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}
> +set compile_flags "debug quiet additional_flags=-I${srcdir}/gdb.asm"

I've noticed it fails to build with the following:

spawn -ignore SIGHUP gcc ../../../gdb.git/gdb/testsuite/gdb.arch/ppc64-atomic-inst.s -I../../../gdb.git/gdb/testsuite/gdb.asm -g -lm -o /home/emachado/gdb/build/gdb/testsuite/gdb.arch/ppc64-atomic-inst^M
../../../gdb.git/gdb/testsuite/gdb.arch/ppc64-atomic-inst.s: Assembler messages:^M
../../../gdb.git/gdb/testsuite/gdb.arch/ppc64-atomic-inst.s:1: Error: can't open common.inc for reading: No such file or directory^M
../../../gdb.git/gdb/testsuite/gdb.arch/ppc64-atomic-inst.s:2: Error: can't open powerpc64.inc for reading: No such file or directory^M
../../../gdb.git/gdb/testsuite/gdb.arch/ppc64-atomic-inst.s:5: Error: Unrecognized opcode: `gdbasm_declare'^M
compiler exited with status 1

This seems to happen because -I option is not passed to the assembler. I've
tried adding this parameter using -Wa as:

set compile_flags "debug quiet additional_flags=-Wa,-I${srcdir}/gdb.asm"

and it fixes the build failure. With this change, the testcase passes OK, also
including the other 2 patches you sent on this thread.

Thanks and regards,
-- 
Edjunior

[PATCH 1/3] Fix ppc64 single step over atomic sequence testcase

[Anton Blanchard]

(My apologies, I thought I'd got these patches merged but obviously I
hadn't).

The current ppc64 single step over atomic sequence testcase is broken.
Convert the test into assembly and use stepi to step through it.

Anton
--

2013-07-29  Anton Blanchard  <anton@samba.org>

	* 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,9 +27,9 @@ 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}
+set compile_flags "debug quiet additional_flags=-I${srcdir}/gdb.asm"
 
 if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable $compile_flags] != "" } {
     unsupported "Testcase compile failed."
@@ -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-2013 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 <http://www.gnu.org/licenses/>.  */
-
-#include <stdio.h>
-
-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 "common.inc"
+        .include "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