diff mbox

Use 2-byte instead of 4-byte NOP on S390 in 'bp-permanent' test case

Message ID 874mtxssee.fsf@br87z6lw.de.ibm.com
State New, archived
Headers

Commit Message

Andreas Arnez Nov. 17, 2014, 4:28 p.m. UTC 
  The bp-permanent test case assumes that a NOP is exactly as long as a
software breakpoint.  This is not the case for the S390 "nop"
instruction, which is 4 bytes long, while a software breakpoint is
just 2 bytes long.  The "nopr" instruction has the right size and can
be used instead.

Without this patch the test case fails on S390 when trying to continue
after SIGTRAP on the permanent breakpoint:

  ...
  Continuing.

  Program received signal SIGILL, Illegal instruction.
  test () at /home/arnez/src/binutils-gdb/gdb/testsuite/gdb.base/bp-permanent.c:40
  40	  NOP; /* after permanent bp */
  (gdb)
  FAIL: gdb.base/bp-permanent.exp: always_inserted=off, sw_watchpoint=0:
    basics: stop at permanent breakpoint

With this patch the test case succeeds without any FAILs.

gdb/testsuite/ChangeLog:

	* gdb.base/bp-permanent.c (NOP): Define as 2-byte instead of
	4-byte instruction on S390.
---
 gdb/testsuite/gdb.base/bp-permanent.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Pedro Alves Nov. 17, 2014, 5:18 p.m. UTC | #1 
Hi Andreas,

Thanks.

On 11/17/2014 04:28 PM, Andreas Arnez wrote:
> The bp-permanent test case assumes that a NOP is exactly as long as a
> software breakpoint.  

Ah.  It'd be good to add a comment mentioning this assumption,
where the NOPs are defined.  Could you do that?

> This is not the case for the S390 "nop"
> instruction, which is 4 bytes long, while a software breakpoint is
> just 2 bytes long.  The "nopr" instruction has the right size and can
> be used instead.
> 
> Without this patch the test case fails on S390 when trying to continue
> after SIGTRAP on the permanent breakpoint:
> 
>   ...
>   Continuing.
> 
>   Program received signal SIGILL, Illegal instruction.
>   test () at /home/arnez/src/binutils-gdb/gdb/testsuite/gdb.base/bp-permanent.c:40
>   40	  NOP; /* after permanent bp */
>   (gdb)

Yeah, if the instruction was originally 4 bytes, and then we poke a 2-byte
breakpoint, when GDB skips the breakpoint manually, advancing 2 bytes,
then the PC ends up pointing at the middle of the original instruction...

The patch is OK.

Thanks,
Pedro Alves
diff mbox

Patch

diff --git a/gdb/testsuite/gdb.base/bp-permanent.c b/gdb/testsuite/gdb.base/bp-permanent.c
index a45a922..e8b4b6b 100644
--- a/gdb/testsuite/gdb.base/bp-permanent.c
+++ b/gdb/testsuite/gdb.base/bp-permanent.c
@@ -21,7 +21,11 @@ 
 #include <unistd.h>
 #endif
 
+#if defined(__s390__) || defined(__s390x__)
+#define NOP asm("nopr 0")
+#else
 #define NOP asm("nop")
+#endif
 
 /* Buffer holding the breakpoint instruction.  */
 unsigned char buffer[16];