[patch] Fix disp-step-syscall.exp on some i386 targets

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

Hi,

I had some problems on various kernels all on i686 kernel (both iron and KVM).
Like:

 stepi
-0xf7fdc421 in __kernel_vsyscall ()
+Detaching after fork from child process 5426.
+0x08048362 in _start ()
 1: x/i $pc
-=> 0xf7fdc421 <__kernel_vsyscall+1>:   push   %edx
-(gdb) PASS: gdb.base/disp-step-syscall.exp: fork: single step over fork
+=> 0x8048362 <_start+2>:       pop    %esi
+(gdb) FAIL: gdb.base/disp-step-syscall.exp: fork: single step over fork

This was completely bogus because the testcase matched "__kernel_vsyscall" as
instruction "syscall".

disp-step-syscall.exp
---------------------
So first I fixed the testcase instrution check.

Then there was another problem that it could not match x86_64 addresses.
It found adress 0x7ffff77e6544 but then it failed to match it:
stepi
0x00007ffff77e6544      131       pid = ARCH_FORK ();
So I fixed that part.

i386-linux-tdep.c
-----------------
Then there was FAIL on native i686: This is because its vDSO is unusual and GDB
gets confusied by 'ret' while still only the second part of 'int $0x80' gets
stepped.  More in the patch comment.

Dump of assembler code for function __kernel_vsyscall:
=> 0xb7fff414 <+0>:	int    $0x80
   0xb7fff416 <+2>:	ret    
End of assembler dump.

Fixed that.

i386-tdep.c
-----------
After fixing the testcase it started failing on x86_64 -m32 (Intel CPU)
because it was getting false PASSes by the broken testcase before.

This is because GDB did not recognize the 'sysenter' instruction in:

Dump of assembler code for function __kernel_vsyscall:
   0xf7fdc420 <+0>:     push   %ecx
   0xf7fdc421 <+1>:     push   %edx
   0xf7fdc422 <+2>:     push   %ebp
   0xf7fdc423 <+3>:     mov    %esp,%ebp
=> 0xf7fdc425 <+5>:     sysenter
   0xf7fdc427 <+7>:     nop
   0xf7fdc428 <+8>:     nop
   0xf7fdc429 <+9>:     nop
   0xf7fdc42a <+10>:    nop
   0xf7fdc42b <+11>:    nop
   0xf7fdc42c <+12>:    nop
   0xf7fdc42d <+13>:    nop
   0xf7fdc42e <+14>:    int    $0x80
=> 0xf7fdc430 <+16>:    pop    %ebp
   0xf7fdc431 <+17>:    pop    %edx
   0xf7fdc432 <+18>:    pop    %ecx
   0xf7fdc433 <+19>:    ret
End of assembler dump.

and as return PC has changed (by that 0x30-0x25 offset over those nops which
I do not understand why kernel does it all) GDB tried to relocate it back.
This created a very invalid address:

displaced: fixup (0xf7fdc425, 0x80483b2), insn = 0x0f 0x34 ...
displaced: relocated %eip from 0xf7fdc430 to 0xe7f704a3
0xe7f704a3 in ?? ()
Cannot access memory at address 0xe7f704a3

Therefore fixed GDB so that it recognizes also 'sysenter' (and put there also
'syscall' when at it).  Made the check more strict - 'int anything' ->
-> 'int 0x80'.  Are you aware of why it was not 0x80 before?


No regressions on {x86_64,x86_64-m32,i686}-fedora17-linux-gnu
(kernel-3.2.6-3.fc16.x86_64) and for i686-fedora17-linux-gnu in KVM
(kernel-PAE-3.3.0-0.rc5.git0.3.fc18.i686).

Posting as a single patch as the testcase fix can create false regressions for
'git bisect', it is separate by file anyway.


Thanks,
Jan


gdb/
2012-02-27  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Fix disp-step-syscall.exp: fork: single step over fork.
	* i386-linux-tdep.c (-i386_linux_get_syscall_number): Rename to ...
	(i386_linux_get_syscall_number_from_regcache): ... here, change
	parameters gdbarch and ptid to regcache.  Remove parameter regcache,
	initialize gdbarch from regcache here.
	(i386_linux_get_syscall_number, i386_linux_displaced_step_copy_insn):
	New function.
	(i386_linux_init_abi): Install i386_linux_displaced_step_copy_insn
	instead.
	* i386-tdep.c (i386_syscall_p): Check also for 'sysenter' and
	'syscall'.  Make the 'int' check more strict.

gdb/testsuite/
2012-02-27  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Fix disp-step-syscall.exp: fork: single step over fork.
	* gdb.base/disp-step-syscall.exp (syscall_insn): Anchor it by
	whitespaces.
	(single step over $syscall): Remove its check.
	(single step over $syscall final pc): New check.

--- a/gdb/i386-linux-tdep.c
+++ b/gdb/i386-linux-tdep.c
@@ -492,10 +492,9 @@ i386_linux_record_signal (struct gdbarch *gdbarch,
 
 
 static LONGEST
-i386_linux_get_syscall_number (struct gdbarch *gdbarch,
-                               ptid_t ptid)
+i386_linux_get_syscall_number_from_regcache (struct regcache *regcache)
 {
-  struct regcache *regcache = get_thread_regcache (ptid);
+  struct gdbarch *gdbarch = get_regcache_arch (regcache);
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   /* The content of a register.  */
   gdb_byte buf[4];
@@ -512,6 +511,15 @@ i386_linux_get_syscall_number (struct gdbarch *gdbarch,
   return ret;
 }
 
+static LONGEST
+i386_linux_get_syscall_number (struct gdbarch *gdbarch,
+                               ptid_t ptid)
+{
+  struct regcache *regcache = get_thread_regcache (ptid);
+
+  return i386_linux_get_syscall_number_from_regcache (regcache);
+}
+
 /* The register sets used in GNU/Linux ELF core-dumps are identical to
    the register sets in `struct user' that are used for a.out
    core-dumps.  These are also used by ptrace(2).  The corresponding
@@ -643,6 +651,35 @@ i386_linux_core_read_description (struct gdbarch *gdbarch,
     return tdesc_i386_mmx_linux;
 }
 
+/* Linux kernel shows PC value after the 'int $0x80' instruction even if
+   inferior is still inside the syscall.  On next PTRACE_SINGLESTEP it will
+   finish the syscall but PC will not change.  Some vDSOs contain
+   'int $0x80; ret' and i386_displaced_step_fixup would keep PC at the displaced
+   location expecting it just executed 'ret' despite it finished the syscall.
+   Hide the 'ret' instruction by 'nop'.  */
+
+struct displaced_step_closure *
+i386_linux_displaced_step_copy_insn (struct gdbarch *gdbarch,
+				     CORE_ADDR from, CORE_ADDR to,
+				     struct regcache *regs)
+{
+  struct displaced_step_closure *closure;
+  
+  closure = i386_displaced_step_copy_insn (gdbarch, from, to, regs);
+
+  if (i386_linux_get_syscall_number_from_regcache (regs) != -1)
+    {
+      /* Since we use simple_displaced_step_copy_insn, our closure is a
+	 copy of the instruction.  */
+      gdb_byte *insn = (gdb_byte *) closure;
+
+      /* Fake nop.  */
+      insn[0] = 0x90;
+    }
+
+  return closure;
+}
+
 static void
 i386_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 {
@@ -890,7 +927,7 @@ i386_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 
   /* Displaced stepping.  */
   set_gdbarch_displaced_step_copy_insn (gdbarch,
-                                        i386_displaced_step_copy_insn);
+                                        i386_linux_displaced_step_copy_insn);
   set_gdbarch_displaced_step_fixup (gdbarch, i386_displaced_step_fixup);
   set_gdbarch_displaced_step_free_closure (gdbarch,
                                            simple_displaced_step_free_closure);
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -521,7 +521,12 @@ i386_call_p (const gdb_byte *insn)
 static int
 i386_syscall_p (const gdb_byte *insn, int *lengthp)
 {
-  if (insn[0] == 0xcd)
+  /* Is it 'int $0x80'?  */
+  if ((insn[0] == 0xcd && insn[1] == 0x80)
+      /* Or is it 'sysenter'?  */
+      || (insn[0] == 0x0f && insn[1] == 0x34)
+      /* Or is it 'syscall'?  */
+      || (insn[0] == 0x0f && insn[1] == 0x05))
     {
       *lengthp = 2;
       return 1;
--- a/gdb/testsuite/gdb.base/disp-step-syscall.exp
+++ b/gdb/testsuite/gdb.base/disp-step-syscall.exp
@@ -25,7 +25,7 @@ set syscall_insn ""
 # Define the syscall instruction for each target.
 
 if { [istarget "i\[34567\]86-*-linux*"] || [istarget "x86_64-*-linux*"] } {
-    set syscall_insn "(int|syscall|sysenter)"
+    set syscall_insn "\[ \t\](int|syscall|sysenter)\[ \t\]"
 } else {
     return -1
 }
@@ -118,7 +117,16 @@ proc disp_step_cross_syscall { syscall } { with_test_prefix "$syscall" {
     gdb_test_no_output "set displaced-stepping on"
 
     # Check the address of next instruction of syscall.
-    gdb_test "stepi" ".*$syscall_insn_next_addr.*" "single step over $syscall"
+    gdb_test "stepi" ".*" "single step over $syscall"
+    set syscall_insn_next_addr_found [get_hexadecimal_valueof "\$pc" "0"]
+
+    set test "single step over $syscall final pc"
+    if {$syscall_insn_next_addr != 0
+	&& $syscall_insn_next_addr == $syscall_insn_next_addr_found} {
+      pass $test
+    } else {
+      fail $test
+    }
 
     # Delete breakpoint syscall insns to avoid interference to other syscalls.
     gdb_test_no_output "delete $syscall_insn_bp" "delete break $syscall insn"