[PATCH] Fix for PR tdep/16397: SystemTap SDT probe support for x86 doesn't work with "triplet operands"

[PATCH] Fix for PR tdep/16397: SystemTap SDT probe support for x86 doesn't work with "triplet operands"

[Sergio Durigan Junior]

Hi,

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

OK to apply?

-- 
Sergio

gdb/
2014-01-12  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-01-12  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.

diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index 9d1d9e0..26d5d8f 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -3639,6 +3639,9 @@ i386_stap_parse_special_token_triplet (struct gdbarch *gdbarch,
 	  got_minus[0] = 1;
 	}
 
+      if (!isdigit (*s))
+	return 0;
+
       displacements[0] = strtol (s, &endp, 10);
       s = endp;
 
@@ -3657,6 +3660,9 @@ i386_stap_parse_special_token_triplet (struct gdbarch *gdbarch,
 	  got_minus[1] = 1;
 	}
 
+      if (!isdigit (*s))
+	return 0;
+
       displacements[1] = strtol (s, &endp, 10);
       s = endp;
 
@@ -3675,6 +3681,9 @@ i386_stap_parse_special_token_triplet (struct gdbarch *gdbarch,
 	  got_minus[2] = 1;
 	}
 
+      if (!isdigit (*s))
+	return 0;
+
       displacements[2] = strtol (s, &endp, 10);
       s = endp;
 
@@ -3690,7 +3699,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);
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
index 0000000..f80dfdf
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/amd64-stap-special-operands.exp
@@ -0,0 +1,47 @@
+# Copyright 2013-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/>.
+
+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
index 0000000..1e2905c
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/amd64-stap-three-arg-disp.S
@@ -0,0 +1,91 @@
+/* Copyright (C) 2013-2014 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-three-arg-disp.c -o amd64-stap-three-arg-disp.S
+
+   Then, the SystemTap SDT probe definition below was tweaked.  See below
+   for more details.  */
+
+	.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"
+/* The probe's argument definition below was tweaked in order to mimic a
+   three arg displacement in x86 asm.  The original probe argument was:
+
+     -4@-4(%rbp)
+
+   The argument below is equivalent to that.  */
+.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
+/* Here, we zero-out %ebx in order to use it safely when evaluating
+   the probe argument.  */
+	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
index 0000000..666e5ec
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/amd64-stap-three-arg-disp.c
@@ -0,0 +1,31 @@
+/* Copyright (C) 2013-2014 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
index 0000000..be22250
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/amd64-stap-triplet.S
@@ -0,0 +1,88 @@
+/* Copyright (C) 2013-2014 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
index 0000000..0ae2730
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/amd64-stap-triplet.c
@@ -0,0 +1,31 @@
+/* Copyright (C) 2013-2014 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;
+}

Re: [PATCH] Fix for PR tdep/16397: SystemTap SDT probe support for x86 doesn't work with "triplet operands"

[Sergio Durigan Junior]

On Sunday, January 12 2014, I wrote:

> Hi,
>
> This is the continuation of what Joel proposed on:
>
> <https://sourceware.org/ml/gdb-patches/2013-12/msg00977.html>

Ping.

> 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/.
>
> OK to apply?
>
> -- 
> Sergio
>
> gdb/
> 2014-01-12  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-01-12  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.
>
> diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
> index 9d1d9e0..26d5d8f 100644
> --- a/gdb/i386-tdep.c
> +++ b/gdb/i386-tdep.c
> @@ -3639,6 +3639,9 @@ i386_stap_parse_special_token_triplet (struct gdbarch *gdbarch,
>  	  got_minus[0] = 1;
>  	}
>  
> +      if (!isdigit (*s))
> +	return 0;
> +
>        displacements[0] = strtol (s, &endp, 10);
>        s = endp;
>  
> @@ -3657,6 +3660,9 @@ i386_stap_parse_special_token_triplet (struct gdbarch *gdbarch,
>  	  got_minus[1] = 1;
>  	}
>  
> +      if (!isdigit (*s))
> +	return 0;
> +
>        displacements[1] = strtol (s, &endp, 10);
>        s = endp;
>  
> @@ -3675,6 +3681,9 @@ i386_stap_parse_special_token_triplet (struct gdbarch *gdbarch,
>  	  got_minus[2] = 1;
>  	}
>  
> +      if (!isdigit (*s))
> +	return 0;
> +
>        displacements[2] = strtol (s, &endp, 10);
>        s = endp;
>  
> @@ -3690,7 +3699,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);
> 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
> index 0000000..f80dfdf
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/amd64-stap-special-operands.exp
> @@ -0,0 +1,47 @@
> +# Copyright 2013-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/>.
> +
> +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
> index 0000000..1e2905c
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/amd64-stap-three-arg-disp.S
> @@ -0,0 +1,91 @@
> +/* Copyright (C) 2013-2014 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-three-arg-disp.c -o amd64-stap-three-arg-disp.S
> +
> +   Then, the SystemTap SDT probe definition below was tweaked.  See below
> +   for more details.  */
> +
> +	.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"
> +/* The probe's argument definition below was tweaked in order to mimic a
> +   three arg displacement in x86 asm.  The original probe argument was:
> +
> +     -4@-4(%rbp)
> +
> +   The argument below is equivalent to that.  */
> +.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
> +/* Here, we zero-out %ebx in order to use it safely when evaluating
> +   the probe argument.  */
> +	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
> index 0000000..666e5ec
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/amd64-stap-three-arg-disp.c
> @@ -0,0 +1,31 @@
> +/* Copyright (C) 2013-2014 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
> index 0000000..be22250
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/amd64-stap-triplet.S
> @@ -0,0 +1,88 @@
> +/* Copyright (C) 2013-2014 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
> index 0000000..0ae2730
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/amd64-stap-triplet.c
> @@ -0,0 +1,31 @@
> +/* Copyright (C) 2013-2014 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;
> +}

-- 
Sergio

Re: [PATCH] Fix for PR tdep/16397: SystemTap SDT probe support for x86 doesn't work with "triplet operands"

[Sergio Durigan Junior]

On Friday, January 17 2014, I wrote:

> On Sunday, January 12 2014, I wrote:
>
>> Hi,
>>
>> This is the continuation of what Joel proposed on:
>>
>> <https://sourceware.org/ml/gdb-patches/2013-12/msg00977.html>
>
> Ping.

Ping^2.

>> 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/.
>>
>> OK to apply?
>>
>> -- 
>> Sergio
>>
>> gdb/
>> 2014-01-12  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-01-12  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.
>>
>> diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
>> index 9d1d9e0..26d5d8f 100644
>> --- a/gdb/i386-tdep.c
>> +++ b/gdb/i386-tdep.c
>> @@ -3639,6 +3639,9 @@ i386_stap_parse_special_token_triplet (struct gdbarch *gdbarch,
>>  	  got_minus[0] = 1;
>>  	}
>>  
>> +      if (!isdigit (*s))
>> +	return 0;
>> +
>>        displacements[0] = strtol (s, &endp, 10);
>>        s = endp;
>>  
>> @@ -3657,6 +3660,9 @@ i386_stap_parse_special_token_triplet (struct gdbarch *gdbarch,
>>  	  got_minus[1] = 1;
>>  	}
>>  
>> +      if (!isdigit (*s))
>> +	return 0;
>> +
>>        displacements[1] = strtol (s, &endp, 10);
>>        s = endp;
>>  
>> @@ -3675,6 +3681,9 @@ i386_stap_parse_special_token_triplet (struct gdbarch *gdbarch,
>>  	  got_minus[2] = 1;
>>  	}
>>  
>> +      if (!isdigit (*s))
>> +	return 0;
>> +
>>        displacements[2] = strtol (s, &endp, 10);
>>        s = endp;
>>  
>> @@ -3690,7 +3699,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);
>> 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
>> index 0000000..f80dfdf
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.arch/amd64-stap-special-operands.exp
>> @@ -0,0 +1,47 @@
>> +# Copyright 2013-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/>.
>> +
>> +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
>> index 0000000..1e2905c
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.arch/amd64-stap-three-arg-disp.S
>> @@ -0,0 +1,91 @@
>> +/* Copyright (C) 2013-2014 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-three-arg-disp.c -o amd64-stap-three-arg-disp.S
>> +
>> +   Then, the SystemTap SDT probe definition below was tweaked.  See below
>> +   for more details.  */
>> +
>> +	.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"
>> +/* The probe's argument definition below was tweaked in order to mimic a
>> +   three arg displacement in x86 asm.  The original probe argument was:
>> +
>> +     -4@-4(%rbp)
>> +
>> +   The argument below is equivalent to that.  */
>> +.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
>> +/* Here, we zero-out %ebx in order to use it safely when evaluating
>> +   the probe argument.  */
>> +	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
>> index 0000000..666e5ec
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.arch/amd64-stap-three-arg-disp.c
>> @@ -0,0 +1,31 @@
>> +/* Copyright (C) 2013-2014 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
>> index 0000000..be22250
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.arch/amd64-stap-triplet.S
>> @@ -0,0 +1,88 @@
>> +/* Copyright (C) 2013-2014 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
>> index 0000000..0ae2730
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.arch/amd64-stap-triplet.c
>> @@ -0,0 +1,31 @@
>> +/* Copyright (C) 2013-2014 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;
>> +}
>
> -- 
> Sergio

-- 
Sergio

Re: [PATCH] Fix for PR tdep/16397: SystemTap SDT probe support for x86 doesn't work with "triplet operands"

[Mark Kettenis]

> From: Sergio Durigan Junior <sergiodj@redhat.com>
> Date: Thu, 30 Jan 2014 13:16:15 -0200
> 
> On Friday, January 17 2014, I wrote:
> 
> > On Sunday, January 12 2014, I wrote:
> >
> >> Hi,
> >>
> >> This is the continuation of what Joel proposed on:
> >>
> >> <https://sourceware.org/ml/gdb-patches/2013-12/msg00977.html>
> >
> > Ping.
> 
> Ping^2.

No objection to this going in (other than the unsafe use of
isdigit(3)) but I don't claim to know wnything of the SDT stuff.

> >> 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/.
> >>
> >> OK to apply?
> >>
> >> -- 
> >> Sergio
> >>
> >> gdb/
> >> 2014-01-12  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-01-12  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.
> >>
> >> diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
> >> index 9d1d9e0..26d5d8f 100644
> >> --- a/gdb/i386-tdep.c
> >> +++ b/gdb/i386-tdep.c
> >> @@ -3639,6 +3639,9 @@ i386_stap_parse_special_token_triplet (struct gdbarch *gdbarch,
> >>  	  got_minus[0] = 1;
> >>  	}
> >>  
> >> +      if (!isdigit (*s))
> >> +	return 0;
> >> +
> >>        displacements[0] = strtol (s, &endp, 10);
> >>        s = endp;
> >>  
> >> @@ -3657,6 +3660,9 @@ i386_stap_parse_special_token_triplet (struct gdbarch *gdbarch,
> >>  	  got_minus[1] = 1;
> >>  	}
> >>  
> >> +      if (!isdigit (*s))
> >> +	return 0;
> >> +
> >>        displacements[1] = strtol (s, &endp, 10);
> >>        s = endp;
> >>  
> >> @@ -3675,6 +3681,9 @@ i386_stap_parse_special_token_triplet (struct gdbarch *gdbarch,
> >>  	  got_minus[2] = 1;
> >>  	}
> >>  
> >> +      if (!isdigit (*s))
> >> +	return 0;
> >> +
> >>        displacements[2] = strtol (s, &endp, 10);
> >>        s = endp;
> >>  
> >> @@ -3690,7 +3699,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);
> >> 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
> >> index 0000000..f80dfdf
> >> --- /dev/null
> >> +++ b/gdb/testsuite/gdb.arch/amd64-stap-special-operands.exp
> >> @@ -0,0 +1,47 @@
> >> +# Copyright 2013-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/>.
> >> +
> >> +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
> >> index 0000000..1e2905c
> >> --- /dev/null
> >> +++ b/gdb/testsuite/gdb.arch/amd64-stap-three-arg-disp.S
> >> @@ -0,0 +1,91 @@
> >> +/* Copyright (C) 2013-2014 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-three-arg-disp.c -o amd64-stap-three-arg-disp.S
> >> +
> >> +   Then, the SystemTap SDT probe definition below was tweaked.  See below
> >> +   for more details.  */
> >> +
> >> +	.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"
> >> +/* The probe's argument definition below was tweaked in order to mimic a
> >> +   three arg displacement in x86 asm.  The original probe argument was:
> >> +
> >> +     -4@-4(%rbp)
> >> +
> >> +   The argument below is equivalent to that.  */
> >> +.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
> >> +/* Here, we zero-out %ebx in order to use it safely when evaluating
> >> +   the probe argument.  */
> >> +	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
> >> index 0000000..666e5ec
> >> --- /dev/null
> >> +++ b/gdb/testsuite/gdb.arch/amd64-stap-three-arg-disp.c
> >> @@ -0,0 +1,31 @@
> >> +/* Copyright (C) 2013-2014 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
> >> index 0000000..be22250
> >> --- /dev/null
> >> +++ b/gdb/testsuite/gdb.arch/amd64-stap-triplet.S
> >> @@ -0,0 +1,88 @@
> >> +/* Copyright (C) 2013-2014 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
> >> index 0000000..0ae2730
> >> --- /dev/null
> >> +++ b/gdb/testsuite/gdb.arch/amd64-stap-triplet.c
> >> @@ -0,0 +1,31 @@
> >> +/* Copyright (C) 2013-2014 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;
> >> +}
> >
> > -- 
> > Sergio
> 
> -- 
> Sergio
> 

Re: [PATCH] Fix for PR tdep/16397: SystemTap SDT probe support for x86 doesn't work with "triplet operands"

[Sergio Durigan Junior]

On Thursday, January 30 2014, Mark Kettenis wrote:

>> From: Sergio Durigan Junior <sergiodj@redhat.com>
>> Date: Thu, 30 Jan 2014 13:16:15 -0200
>> 
>> On Friday, January 17 2014, I wrote:
>> 
>> > On Sunday, January 12 2014, I wrote:
>> >
>> >> Hi,
>> >>
>> >> This is the continuation of what Joel proposed on:
>> >>
>> >> <https://sourceware.org/ml/gdb-patches/2013-12/msg00977.html>
>> >
>> > Ping.
>> 
>> Ping^2.
>
> No objection to this going in (other than the unsafe use of
> isdigit(3))

Could you be more specific about the unsafe use of isdigit?

Thanks for the approval,

-- 
Sergio

Re: [PATCH] Fix for PR tdep/16397: SystemTap SDT probe support for x86 doesn't work with "triplet operands"

[Mark Kettenis]

> From: Sergio Durigan Junior <sergiodj@redhat.com>
> Date: Sun, 02 Feb 2014 14:29:01 -0200
> 
> On Thursday, January 30 2014, Mark Kettenis wrote:
> 
> >> From: Sergio Durigan Junior <sergiodj@redhat.com>
> >> Date: Thu, 30 Jan 2014 13:16:15 -0200
> >> 
> >> On Friday, January 17 2014, I wrote:
> >> 
> >> > On Sunday, January 12 2014, I wrote:
> >> >
> >> >> Hi,
> >> >>
> >> >> This is the continuation of what Joel proposed on:
> >> >>
> >> >> <https://sourceware.org/ml/gdb-patches/2013-12/msg00977.html>
> >> >
> >> > Ping.
> >> 
> >> Ping^2.
> >
> > No objection to this going in (other than the unsafe use of
> > isdigit(3))
> 
> Could you be more specific about the unsafe use of isdigit?

  char *s;
  ...
  if (isdigit(*s)) ...;

On most platforms chars are signed.  The argument of isdigit(3) is
(signed) int.  If *s is outside the 7-bi ASCII range, it will be
negative.  The conversion to int will not change this.  But calling
isdigit(3) with an argument that isn't in the range 0-255 and isn't
EOF (-1) is undefined.  Many C libraries will do a range check, but a
naive (but standards conforming) implementation might just look at a
negative array index and crash your program.

The correct way to use these functions in code like this is to
explicitly cast the argument to unsigned char.

Cheers,

Mark

Re: [PATCH] Fix for PR tdep/16397: SystemTap SDT probe support for x86 doesn't work with "triplet operands"

[Sergio Durigan Junior]

On Thursday, January 30 2014, Mark Kettenis wrote:

>> From: Sergio Durigan Junior <sergiodj@redhat.com>
>> Date: Thu, 30 Jan 2014 13:16:15 -0200
>> 
>> On Friday, January 17 2014, I wrote:
>> 
>> > On Sunday, January 12 2014, I wrote:
>> >
>> >> Hi,
>> >>
>> >> This is the continuation of what Joel proposed on:
>> >>
>> >> <https://sourceware.org/ml/gdb-patches/2013-12/msg00977.html>
>> >
>> > Ping.
>> 
>> Ping^2.
>
> No objection to this going in (other than the unsafe use of
> isdigit(3)) but I don't claim to know wnything of the SDT stuff.

Thanks, I finally pushed it with the necessary fixes.

        https://sourceware.org/ml/gdb-cvs/2014-02/msg00106.html

-- 
Sergio

[testsuite patch+7.8] gdb.arch/amd64-stap-special-operands.exp !is_lp64_target [Re: [PATCH] Fix for PR tdep/16397: SystemTap SDT probe support for x86 doesn't work with "triplet operands"]

[Jan Kratochvil]

[-- Attachment #1: Type: text/plain, Size: 961 bytes --]

On Thu, 20 Feb 2014 22:53:21 +0100, Sergio Durigan Junior wrote:
> Thanks, I finally pushed it with the necessary fixes.
>         https://sourceware.org/ml/gdb-cvs/2014-02/msg00106.html
# if { ![istarget "x86_64-*-*"] && ![istarget "i?86-*-*"] } {
#     verbose "Skipping amd64-stap-special-operands.exp"
#     return 
# }   

On x86_64 with -m32 or on i686 it will:

Running ./gdb.arch/amd64-stap-special-operands.exp ...
gdb compile failed, amd64-stap-triplet.c: Assembler messages:
amd64-stap-triplet.c:35: Error: bad register name `%rbp'
amd64-stap-triplet.c:38: Error: bad register name `%rsp'
amd64-stap-triplet.c:40: Error: bad register name `%rbp)'
amd64-stap-triplet.c:41: Error: bad register name `%rsi'
amd64-stap-triplet.c:42: Error: bad register name `%rbp)'
/tmp/ccjOdmpl.s:63: Error: bad register name `%rbp'

IMO there should be the patch below.  OK for check-in?
It would apply as [obv] but I rather ask Sergio if it was meant this way.


Jan

[-- Attachment #2: 1 --]
[-- Type: text/plain, Size: 765 bytes --]

gdb/testsuite/
2014-06-22  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.arch/amd64-stap-special-operands.exp: Use is_lp64_target.

diff --git a/gdb/testsuite/gdb.arch/amd64-stap-special-operands.exp b/gdb/testsuite/gdb.arch/amd64-stap-special-operands.exp
index a6ce7f5..988b5d6 100644
--- a/gdb/testsuite/gdb.arch/amd64-stap-special-operands.exp
+++ b/gdb/testsuite/gdb.arch/amd64-stap-special-operands.exp
@@ -13,7 +13,7 @@
 # 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-*-*"] } {
+if { ![istarget "x86_64-*-*"] || ![is_lp64_target] } {
     verbose "Skipping amd64-stap-special-operands.exp"
     return
 }

Re: [testsuite patch+7.8] gdb.arch/amd64-stap-special-operands.exp !is_lp64_target [Re: [PATCH] Fix for PR tdep/16397: SystemTap SDT probe support for x86 doesn't work with "triplet operands"]

[Sergio Durigan Junior]

On Sunday, June 22 2014, Jan Kratochvil wrote:

> On Thu, 20 Feb 2014 22:53:21 +0100, Sergio Durigan Junior wrote:
>> Thanks, I finally pushed it with the necessary fixes.
>>         https://sourceware.org/ml/gdb-cvs/2014-02/msg00106.html
> # if { ![istarget "x86_64-*-*"] && ![istarget "i?86-*-*"] } {
> #     verbose "Skipping amd64-stap-special-operands.exp"
> #     return 
> # }   
>
> On x86_64 with -m32 or on i686 it will:
>
> Running ./gdb.arch/amd64-stap-special-operands.exp ...
> gdb compile failed, amd64-stap-triplet.c: Assembler messages:
> amd64-stap-triplet.c:35: Error: bad register name `%rbp'
> amd64-stap-triplet.c:38: Error: bad register name `%rsp'
> amd64-stap-triplet.c:40: Error: bad register name `%rbp)'
> amd64-stap-triplet.c:41: Error: bad register name `%rsi'
> amd64-stap-triplet.c:42: Error: bad register name `%rbp)'
> /tmp/ccjOdmpl.s:63: Error: bad register name `%rbp'
>
> IMO there should be the patch below.  OK for check-in?
> It would apply as [obv] but I rather ask Sergio if it was meant this way.

Ops, my bad.

> Jan
> gdb/testsuite/
> 2014-06-22  Jan Kratochvil  <jan.kratochvil@redhat.com>
>
> 	* gdb.arch/amd64-stap-special-operands.exp: Use is_lp64_target.
>
> diff --git a/gdb/testsuite/gdb.arch/amd64-stap-special-operands.exp b/gdb/testsuite/gdb.arch/amd64-stap-special-operands.exp
> index a6ce7f5..988b5d6 100644
> --- a/gdb/testsuite/gdb.arch/amd64-stap-special-operands.exp
> +++ b/gdb/testsuite/gdb.arch/amd64-stap-special-operands.exp
> @@ -13,7 +13,7 @@
>  # 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-*-*"] } {
> +if { ![istarget "x86_64-*-*"] || ![is_lp64_target] } {
>      verbose "Skipping amd64-stap-special-operands.exp"
>      return
>  }

Yes, that's the right check.

BTW, gdb.arch/amd64-stap-optional-prefix.exp needs the same check for
![is_lp64_target], would you mind adding too?

Thanks,

-- 
Sergio
GPG key ID: 65FC5E36
Please send encrypted e-mail if possible
http://blog.sergiodj.net/

[testsuite obv+7.8] gdb.arch/amd64-stap-special-operands.exp !is_lp64_target [Re: [PATCH] Fix for PR tdep/16397: SystemTap SDT probe support for x86 doesn't work with "triplet operands"]

[Jan Kratochvil]

[-- Attachment #1: Type: text/plain, Size: 1334 bytes --]

On Mon, 23 Jun 2014 00:17:15 +0200, Sergio Durigan Junior wrote:
> On Sunday, June 22 2014, Jan Kratochvil wrote:
> > On Thu, 20 Feb 2014 22:53:21 +0100, Sergio Durigan Junior wrote:
> >> Thanks, I finally pushed it with the necessary fixes.
> >>         https://sourceware.org/ml/gdb-cvs/2014-02/msg00106.html
> > # if { ![istarget "x86_64-*-*"] && ![istarget "i?86-*-*"] } {
> > #     verbose "Skipping amd64-stap-special-operands.exp"
> > #     return 
> > # }   
> >
> > On x86_64 with -m32 or on i686 it will:
> >
> > Running ./gdb.arch/amd64-stap-special-operands.exp ...
> > gdb compile failed, amd64-stap-triplet.c: Assembler messages:
> > amd64-stap-triplet.c:35: Error: bad register name `%rbp'
> > amd64-stap-triplet.c:38: Error: bad register name `%rsp'
> > amd64-stap-triplet.c:40: Error: bad register name `%rbp)'
> > amd64-stap-triplet.c:41: Error: bad register name `%rsi'
> > amd64-stap-triplet.c:42: Error: bad register name `%rbp)'
> > /tmp/ccjOdmpl.s:63: Error: bad register name `%rbp'
[...]
> BTW, gdb.arch/amd64-stap-optional-prefix.exp needs the same check for
> ![is_lp64_target], would you mind adding too?

OK, done, also one other testfile (gdb.dwarf2/dw2-error.exp).

Checked in for master:
	b972bd9c133c8acd3f5ce8d05a4fdfc85aee2c15
and for gdb-7.8-branch:
	45e9b677bd79b001548b15921adf6bd1962562c0


Jan

[-- Attachment #2: 1 --]
[-- Type: text/plain, Size: 3247 bytes --]

commit b972bd9c133c8acd3f5ce8d05a4fdfc85aee2c15
Author: Jan Kratochvil <jan.kratochvil@redhat.com>
Date:   Mon Jun 23 08:24:36 2014 +0200

    testsuite: Use istarget and is_lp64_target for 3 testcases.
    
    On x86_64 with -m32 or on i686 it will:
    
    Running ./gdb.arch/amd64-stap-special-operands.exp ...
    gdb compile failed, amd64-stap-triplet.c: Assembler messages:
    amd64-stap-triplet.c:35: Error: bad register name `%rbp'
    amd64-stap-triplet.c:38: Error: bad register name `%rsp'
    amd64-stap-triplet.c:40: Error: bad register name `%rbp)'
    amd64-stap-triplet.c:41: Error: bad register name `%rsi'
    amd64-stap-triplet.c:42: Error: bad register name `%rbp)'
    /tmp/ccjOdmpl.s:63: Error: bad register name `%rbp'
    
    2014-06-23  Jan Kratochvil  <jan.kratochvil@redhat.com>
    
    	* gdb.arch/amd64-stap-special-operands.exp: Use is_lp64_target.
    	* gdb.arch/amd64-stap-optional-prefix.exp: Likewise.
    	* gdb.dwarf2/dw2-error.exp: Use istarget and is_lp64_target.
    
    Message-ID: <20140622211401.GA3716@host2.jankratochvil.net>

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 1e7ef7f..93cd6f6 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+2014-06-23  Jan Kratochvil  <jan.kratochvil@redhat.com>
+
+	* gdb.arch/amd64-stap-special-operands.exp: Use is_lp64_target.
+	* gdb.arch/amd64-stap-optional-prefix.exp: Likewise.
+	* gdb.dwarf2/dw2-error.exp: Use istarget and is_lp64_target.
+
 2014-06-20  Gary Benson  <gbenson@redhat.com>
 
 	* gdb.arch/i386-avx.exp: Fix include file location.
diff --git a/gdb/testsuite/gdb.arch/amd64-stap-optional-prefix.exp b/gdb/testsuite/gdb.arch/amd64-stap-optional-prefix.exp
index b7f1505..10d3e70 100644
--- a/gdb/testsuite/gdb.arch/amd64-stap-optional-prefix.exp
+++ b/gdb/testsuite/gdb.arch/amd64-stap-optional-prefix.exp
@@ -17,7 +17,7 @@
 
 standard_testfile ".S"
 
-if { ![istarget "x86_64-*-*"] } {
+if { ![istarget "x86_64-*-*"] || ![is_lp64_target] } {
     verbose "Skipping $testfile.exp"
     return
 }
diff --git a/gdb/testsuite/gdb.arch/amd64-stap-special-operands.exp b/gdb/testsuite/gdb.arch/amd64-stap-special-operands.exp
index a6ce7f5..988b5d6 100644
--- a/gdb/testsuite/gdb.arch/amd64-stap-special-operands.exp
+++ b/gdb/testsuite/gdb.arch/amd64-stap-special-operands.exp
@@ -13,7 +13,7 @@
 # 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-*-*"] } {
+if { ![istarget "x86_64-*-*"] || ![is_lp64_target] } {
     verbose "Skipping amd64-stap-special-operands.exp"
     return
 }
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-error.exp b/gdb/testsuite/gdb.dwarf2/dw2-error.exp
index 65eac6d..b6cd205 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-error.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-error.exp
@@ -22,6 +22,11 @@ if {![dwarf2_support]} {
 
 standard_testfile .S
 
+if { ![istarget "x86_64-*-*"] || ![is_lp64_target] } {
+    verbose "Skipping $testfile.exp"
+    return
+}
+
 # We can't use prepare_for_testing here because we need to check the
 # 'file' command's output.
 if {[build_executable $testfile.exp $testfile $srcfile {nodebug}]} {