Fix for PR tdep/16397: SystemTap SDT probe support for x86 doesn't work with "triplet...
authorSergio Durigan Junior <sergiodj@redhat.com>
Thu, 20 Feb 2014 21:49:09 +0000 (18:49 -0300)
committerSergio Durigan Junior <sergiodj@redhat.com>
Thu, 20 Feb 2014 21:49:09 +0000 (18:49 -0300)
This is the continuation of what Joel proposed on:

<https://sourceware.org/ml/gdb-patches/2013-12/msg00977.html>

Now that I have already submitted and pushed the patch to split
i386_stap_parse_special_token into two smaller functions, it is indeed
simpler to understand this patch.

It occurs because, on x86, triplet displacement operands are allowed
(like "-4+8-20(%rbp)"), and the current parser for this expression is
buggy.  It does not correctly extract the register name from the
expression, which leads to incorrect evaluation.  The parser was also
being very "generous" with the expression, so I included a few more
checks to ensure that we're indeed dealing with a triplet displacement
operand.

This patch also includes testcases for the two different kind of
expressions that can be encountered on x86: the triplet displacement
(explained above) and the three-argument displacement (as in
"(%rbx,%ebx,-8)").  The tests are obviously arch-dependent and are
placed under gdb.arch/.

Message-ID: <m3mwj1j12v.fsf@redhat.com>
URL: <https://sourceware.org/ml/gdb-patches/2014-01/msg00310.html>

gdb/
2014-02-20  Sergio Durigan Junior  <sergiodj@redhat.com>

PR tdep/16397
* i386-tdep.c (i386_stap_parse_special_token_triplet): Check if a
number comes after the + or - signs.  Adjust length of register
name to be extracted.

gdb/testsuite/
2014-02-20  Sergio Durigan Junior  <sergiodj@redhat.com>

PR tdep/16397
* gdb.arch/amd64-stap-special-operands.exp: New file.
* gdb.arch/amd64-stap-three-arg-disp.S: Likewise.
* gdb.arch/amd64-stap-three-arg-disp.c: Likewise.
* gdb.arch/amd64-stap-triplet.S: Likewise.
* gdb.arch/amd64-stap-triplet.c: Likewise.

gdb/ChangeLog
gdb/i386-tdep.c
gdb/testsuite/ChangeLog
gdb/testsuite/gdb.arch/amd64-stap-special-operands.exp [new file with mode: 0644]
gdb/testsuite/gdb.arch/amd64-stap-three-arg-disp.S [new file with mode: 0644]
gdb/testsuite/gdb.arch/amd64-stap-three-arg-disp.c [new file with mode: 0644]
gdb/testsuite/gdb.arch/amd64-stap-triplet.S [new file with mode: 0644]
gdb/testsuite/gdb.arch/amd64-stap-triplet.c [new file with mode: 0644]

index b9a46eb70ef48b3821ca83f6d3f74978431de232..ac3844f188e2eede63882cbfdb1061bf454d2b19 100644 (file)
@@ -1,3 +1,10 @@
+2014-02-20  Sergio Durigan Junior  <sergiodj@redhat.com>
+
+       PR tdep/16397
+       * i386-tdep.c (i386_stap_parse_special_token_triplet): Check if a
+       number comes after the + or - signs.  Adjust length of register
+       name to be extracted.
+
 2014-02-20  Tom Tromey  <tromey@redhat.com>
 
        * varobj.h (c_varobj_ops, cplus_varobj_ops, java_varobj_ops)
index 57f18cbab8d43505c8f3078ec5bbc1d284f55ed2..bb414b6ad6d184ca9e615f94047aa86f20c0d1de 100644 (file)
@@ -3694,6 +3694,9 @@ i386_stap_parse_special_token_triplet (struct gdbarch *gdbarch,
          got_minus[0] = 1;
        }
 
+      if (!isdigit ((unsigned char) *s))
+       return 0;
+
       displacements[0] = strtol (s, &endp, 10);
       s = endp;
 
@@ -3712,6 +3715,9 @@ i386_stap_parse_special_token_triplet (struct gdbarch *gdbarch,
          got_minus[1] = 1;
        }
 
+      if (!isdigit ((unsigned char) *s))
+       return 0;
+
       displacements[1] = strtol (s, &endp, 10);
       s = endp;
 
@@ -3730,6 +3736,9 @@ i386_stap_parse_special_token_triplet (struct gdbarch *gdbarch,
          got_minus[2] = 1;
        }
 
+      if (!isdigit ((unsigned char) *s))
+       return 0;
+
       displacements[2] = strtol (s, &endp, 10);
       s = endp;
 
@@ -3745,7 +3754,7 @@ i386_stap_parse_special_token_triplet (struct gdbarch *gdbarch,
       if (*s++ != ')')
        return 0;
 
-      len = s - start;
+      len = s - start - 1;
       regname = alloca (len + 1);
 
       strncpy (regname, start, len);
index d1d99f8be6e7be523b233123d2080e13071ee015..d7e86106c4af6a8186e62fdc792121f1ff15752a 100644 (file)
@@ -1,3 +1,12 @@
+2014-02-20  Sergio Durigan Junior  <sergiodj@redhat.com>
+
+       PR tdep/16397
+       * gdb.arch/amd64-stap-special-operands.exp: New file.
+       * gdb.arch/amd64-stap-three-arg-disp.S: Likewise.
+       * gdb.arch/amd64-stap-three-arg-disp.c: Likewise.
+       * gdb.arch/amd64-stap-triplet.S: Likewise.
+       * gdb.arch/amd64-stap-triplet.c: Likewise.
+
 2014-02-20  Joel Brobecker  <brobecker@adacore.com>
 
        * gdb.dwarf2/dw2-icycle.S: Remove second and third parameters
diff --git a/gdb/testsuite/gdb.arch/amd64-stap-special-operands.exp b/gdb/testsuite/gdb.arch/amd64-stap-special-operands.exp
new file mode 100644 (file)
index 0000000..a6ce7f5
--- /dev/null
@@ -0,0 +1,47 @@
+# Copyright 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/>.
+
+if { ![istarget "x86_64-*-*"] && ![istarget "i?86-*-*"] } {
+    verbose "Skipping amd64-stap-special-operands.exp"
+    return
+}
+
+proc test_probe { probe_name } {
+    if { ![runto "-pstap $probe_name"] } {
+       fail "run to probe $probe_name"
+       return
+    }
+
+    gdb_test "print \$_probe_argc" " = 1"
+    gdb_test "print \$_probe_arg0" " = 10"
+}
+
+standard_testfile amd64-stap-triplet.S
+
+if { [prepare_for_testing $testfile.exp $testfile $srcfile] } {
+    untested amd64-stap-special-operands.exp
+    return -1
+}
+
+test_probe "triplet"
+
+standard_testfile amd64-stap-three-arg-disp.S
+
+if { [prepare_for_testing $testfile.exp $testfile $srcfile] } {
+    untested amd64-stap-special-operands.exp
+    return -1
+}
+
+test_probe "three_arg"
diff --git a/gdb/testsuite/gdb.arch/amd64-stap-three-arg-disp.S b/gdb/testsuite/gdb.arch/amd64-stap-three-arg-disp.S
new file mode 100644 (file)
index 0000000..cf3856f
--- /dev/null
@@ -0,0 +1,58 @@
+       .file   "amd64-stap-three-arg-disp.c"
+       .text
+       .globl  main
+       .type   main, @function
+main:
+.LFB0:
+       .cfi_startproc
+# BLOCK 2 seq:0
+# PRED: ENTRY (fallthru)
+       pushq   %rbp
+       .cfi_def_cfa_offset 16
+       .cfi_offset 6, -16
+       movq    %rsp, %rbp
+       .cfi_def_cfa_register 6
+       movl    %edi, -20(%rbp)
+       movq    %rsi, -32(%rbp)
+       movl    $10, -4(%rbp)
+#APP
+# 8 "amd64-stap-three-arg-disp.c" 1
+       990: nop
+.pushsection .note.stapsdt,"?","note"
+.balign 4
+.4byte 992f-991f,994f-993f,3
+991: .asciz "stapsdt"
+992: .balign 4
+993: .8byte 990b
+.8byte _.stapsdt.base
+.8byte 0
+.asciz "test"
+.asciz "three_arg"
+.asciz "-4@-4(%rbp,%ebx,0)"
+994: .balign 4
+.popsection
+
+# 0 "" 2
+# 8 "amd64-stap-three-arg-disp.c" 1
+       .ifndef _.stapsdt.base
+.pushsection .stapsdt.base,"aG","progbits",.stapsdt.base,comdat
+.weak _.stapsdt.base
+.hidden _.stapsdt.base
+_.stapsdt.base: .space 1
+.size _.stapsdt.base,1
+.popsection
+.endif
+
+# 0 "" 2
+#NO_APP
+       movl    $0, %eax
+       movl    $0, %ebx
+       popq    %rbp
+       .cfi_def_cfa 7, 8
+# SUCC: EXIT [100.0%] 
+       ret
+       .cfi_endproc
+.LFE0:
+       .size   main, .-main
+       .ident  "GCC: (GNU) 4.7.2 20120921 (Red Hat 4.7.2-2)"
+       .section        .note.GNU-stack,"",@progbits
diff --git a/gdb/testsuite/gdb.arch/amd64-stap-three-arg-disp.c b/gdb/testsuite/gdb.arch/amd64-stap-three-arg-disp.c
new file mode 100644 (file)
index 0000000..f32d495
--- /dev/null
@@ -0,0 +1,31 @@
+/* Copyright (C) 2013 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   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/>.
+
+   This file is not used directly.  Please, see the equivalent .S file
+   for more details.  */
+
+#include <sys/sdt.h>
+
+int
+main (int argc, char *argv[])
+{
+  int a = 10;
+
+  STAP_PROBE1 (test, three_arg, a);
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.arch/amd64-stap-triplet.S b/gdb/testsuite/gdb.arch/amd64-stap-triplet.S
new file mode 100644 (file)
index 0000000..f3a6bf7
--- /dev/null
@@ -0,0 +1,88 @@
+/* Copyright (C) 2013 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   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/>.
+
+   This file was generated from the equivalent .c file using the
+   following command:
+
+     #> gcc -S amd64-stap-triplet.c -o amd64-stap-triplet.S
+
+   Then, the SystemTap SDT probe definition below was tweaked.  See below
+   for more details.  */
+
+       .file   "amd64-stap-triplet.c"
+       .text
+       .globl  main
+       .type   main, @function
+main:
+.LFB0:
+       .cfi_startproc
+# BLOCK 2 seq:0
+# PRED: ENTRY (fallthru)
+       pushq   %rbp
+       .cfi_def_cfa_offset 16
+       .cfi_offset 6, -16
+       movq    %rsp, %rbp
+       .cfi_def_cfa_register 6
+       movl    %edi, -20(%rbp)
+       movq    %rsi, -32(%rbp)
+       movl    $10, -4(%rbp)
+#APP
+# 8 "amd64-stap-triplet.c" 1
+       990: nop
+.pushsection .note.stapsdt,"?","note"
+.balign 4
+.4byte 992f-991f,994f-993f,3
+991: .asciz "stapsdt"
+992: .balign 4
+993: .8byte 990b
+.8byte _.stapsdt.base
+.8byte 0
+.asciz "test"
+.asciz "triplet"
+/* The probe's argument definition below was tweaked in order to mimic a
+   triplet displacement in x86 asm.  The original probe argument was:
+
+     -4@-4(%rbp)
+
+   The argument below is equivalent to that.  */
+.asciz "-4@-4+16-16(%rbp)"
+994: .balign 4
+.popsection
+
+# 0 "" 2
+# 8 "amd64-stap-triplet.c" 1
+       .ifndef _.stapsdt.base
+.pushsection .stapsdt.base,"aG","progbits",.stapsdt.base,comdat
+.weak _.stapsdt.base
+.hidden _.stapsdt.base
+_.stapsdt.base: .space 1
+.size _.stapsdt.base,1
+.popsection
+.endif
+
+# 0 "" 2
+#NO_APP
+       movl    $0, %eax
+       popq    %rbp
+       .cfi_def_cfa 7, 8
+# SUCC: EXIT [100.0%] 
+       ret
+       .cfi_endproc
+.LFE0:
+       .size   main, .-main
+       .ident  "GCC: (GNU) 4.7.2 20120921 (Red Hat 4.7.2-2)"
+       .section        .note.GNU-stack,"",@progbits
diff --git a/gdb/testsuite/gdb.arch/amd64-stap-triplet.c b/gdb/testsuite/gdb.arch/amd64-stap-triplet.c
new file mode 100644 (file)
index 0000000..7ca64bf
--- /dev/null
@@ -0,0 +1,31 @@
+/* Copyright (C) 2013 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   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/>.
+
+   This file is not used directly.  Please, see the equivalent .S file
+   for more details.  */
+
+#include <sys/sdt.h>
+
+int
+main (int argc, char *argv[])
+{
+  int a = 10;
+
+  STAP_PROBE1 (test, triplet, a);
+
+  return 0;
+}