From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 29507 invoked by alias); 12 Jan 2014 04:06:45 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 29491 invoked by uid 89); 12 Jan 2014 04:06:40 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.5 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sun, 12 Jan 2014 04:06:38 +0000 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s0C46boW014383 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Sat, 11 Jan 2014 23:06:37 -0500 Received: from psique (ovpn-113-176.phx2.redhat.com [10.3.113.176]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id s0C46X8p016618 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO) for ; Sat, 11 Jan 2014 23:06:35 -0500 From: Sergio Durigan Junior To: GDB Patches Subject: [PATCH] Fix for PR tdep/16397: SystemTap SDT probe support for x86 doesn't work with "triplet operands" X-URL: http://www.redhat.com Date: Sun, 12 Jan 2014 04:06:00 -0000 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-IsSubscribed: yes X-SW-Source: 2014-01/txt/msg00310.txt.bz2 Hi, This is the continuation of what Joel proposed on: 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 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 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 . + +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 . + + 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 . + + This file is not used directly. Please, see the equivalent .S file + for more details. */ + +#include + +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 . + + 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 . + + This file is not used directly. Please, see the equivalent .S file + for more details. */ + +#include + +int +main (int argc, char *argv[]) +{ + int a = 10; + + STAP_PROBE1 (test, triplet, a); + + return 0; +}