From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 22832 invoked by alias); 2 May 2014 19:25:46 -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 22816 invoked by uid 89); 2 May 2014 19:25:45 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.6 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; Fri, 02 May 2014 19:25:43 +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 s42JPgFc028184 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Fri, 2 May 2014 15:25:42 -0400 Received: from psique ([10.3.113.6]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id s42JPdwD006613 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Fri, 2 May 2014 15:25:40 -0400 From: Sergio Durigan Junior To: Pedro Alves Cc: GDB Patches Subject: Re: [PATCH 2/2] Extend recognized types of SDT probe's arguments References: <1398981131-11720-1-git-send-email-sergiodj@redhat.com> <1398981131-11720-3-git-send-email-sergiodj@redhat.com> <53636A1C.8000806@redhat.com> X-URL: http://www.redhat.com Date: Fri, 02 May 2014 19:25:00 -0000 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.1 (gnu/linux) MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-IsSubscribed: yes X-SW-Source: 2014-05/txt/msg00022.txt.bz2 --=-=-= Content-Type: text/plain Content-length: 1453 On Friday, May 02 2014, Pedro Alves wrote: > On 05/01/2014 10:52 PM, Sergio Durigan Junior wrote: >> This commit is actually an update to make the parser in >> gdb/stap-probe.c be aware of all the possible prefixes that a probe >> argument can have. According to the section "Argument Format" in: >> >> >> >> The bitness of the arguments can be 8, 16, 32 or 64 bits, signed or >> unsigned. Currently GDB recognizes only 32 and 64-bit arguments. > > Looks good. Thanks. >> This commit extends this. Since this is a straightforward extension, >> I am not submitting a testcase; I can do that if anybody wants. > > I think it'd be good to have a test -- the code that triggered the other > bug was also supposedly straightforward. :-) Can we do this in C ? > Ideally we'd also test that we don't crash with an invalid bitness > (the complaint path). Hm, OK, here is the testcase then. >> >> gdb/ >> 2014-05-01 Sergio Durigan Junior >> >> stap-probe.c (enum stap_arg_bitness): New enums to represent 8 > > Missing '*'- Writing ChangeLog entries while at "git commit" buffer is tricky... Second mistake in a row! Thanks. >> and 16-bit signed and unsigned arguments. >> (stap_parse_probe_arguments): Extend code to handle such >> arguments. > > -- > Pedro Alves Here is what I will push if there are no other comments. Thanks, -- Sergio --=-=-= Content-Type: text/x-patch Content-Disposition: inline; filename=111.patch Content-length: 7532 commit 4fdce9c1c58588209e26cd690e7e653b6056d458 Author: Sergio Durigan Junior Date: Thu May 1 18:39:28 2014 -0300 Extend recognized types of SDT probe's arguments This commit is actually an update to make the parser in gdb/stap-probe.c be aware of all the possible prefixes that a probe argument can have. According to the section "Argument Format" in: The bitness of the arguments can be 8, 16, 32 or 64 bits, signed or unsigned. Currently GDB recognizes only 32 and 64-bit arguments. This commit extends this. Since this is a straightforward extension, I am not submitting a testcase; I can do that if anybody wants. gdb/ 2014-05-02 Sergio Durigan Junior * stap-probe.c (enum stap_arg_bitness): New enums to represent 8 and 16-bit signed and unsigned arguments. Update comment. (stap_parse_probe_arguments): Extend code to handle such arguments. Use warning instead of complaint to notify about unrecognized bitness. gdb/testsuite/ 2014-05-02 Sergio Durigan Junior * gdb.arch/amd64-stap-optional-prefix.S (main): Add several probes to test for bitness recognition. * gdb.arch/amd64-stap-optional-prefix.exp (test_probe_value_without_reg): New procedure. Add code to test for different kinds of bitness. diff --git a/gdb/stap-probe.c b/gdb/stap-probe.c index ef45495..84714b5 100644 --- a/gdb/stap-probe.c +++ b/gdb/stap-probe.c @@ -60,6 +60,10 @@ static unsigned int stap_expression_debug = 0; The relationship is: - STAP_ARG_BITNESS_UNDEFINED: The user hasn't specified the bitness. + - STAP_ARG_BITNESS_8BIT_UNSIGNED: argument string starts with `1@'. + - STAP_ARG_BITNESS_8BIT_SIGNED: argument string starts with `-1@'. + - STAP_ARG_BITNESS_16BIT_UNSIGNED: argument string starts with `2@'. + - STAP_ARG_BITNESS_16BIT_SIGNED: argument string starts with `-2@'. - STAP_ARG_BITNESS_32BIT_UNSIGNED: argument string starts with `4@'. - STAP_ARG_BITNESS_32BIT_SIGNED: argument string starts with `-4@'. - STAP_ARG_BITNESS_64BIT_UNSIGNED: argument string starts with `8@'. @@ -68,6 +72,10 @@ static unsigned int stap_expression_debug = 0; enum stap_arg_bitness { STAP_ARG_BITNESS_UNDEFINED, + STAP_ARG_BITNESS_8BIT_UNSIGNED, + STAP_ARG_BITNESS_8BIT_SIGNED, + STAP_ARG_BITNESS_16BIT_UNSIGNED, + STAP_ARG_BITNESS_16BIT_SIGNED, STAP_ARG_BITNESS_32BIT_UNSIGNED, STAP_ARG_BITNESS_32BIT_SIGNED, STAP_ARG_BITNESS_64BIT_UNSIGNED, @@ -329,6 +337,18 @@ stap_get_expected_argument_type (struct gdbarch *gdbarch, else return builtin_type (gdbarch)->builtin_uint64; + case STAP_ARG_BITNESS_8BIT_UNSIGNED: + return builtin_type (gdbarch)->builtin_uint8; + + case STAP_ARG_BITNESS_8BIT_SIGNED: + return builtin_type (gdbarch)->builtin_int8; + + case STAP_ARG_BITNESS_16BIT_UNSIGNED: + return builtin_type (gdbarch)->builtin_uint16; + + case STAP_ARG_BITNESS_16BIT_SIGNED: + return builtin_type (gdbarch)->builtin_int16; + case STAP_ARG_BITNESS_32BIT_SIGNED: return builtin_type (gdbarch)->builtin_int32; @@ -1095,7 +1115,7 @@ stap_parse_probe_arguments (struct stap_probe *probe, struct gdbarch *gdbarch) N@OP - Where `N' can be [+,-][4,8]. This is not mandatory, so + Where `N' can be [+,-][1,2,4,8]. This is not mandatory, so we check it here. If we don't find it, go to the next state. */ if ((cur[0] == '-' && isdigit (cur[1]) && cur[2] == '@') @@ -1108,20 +1128,37 @@ stap_parse_probe_arguments (struct stap_probe *probe, struct gdbarch *gdbarch) got_minus = 1; } - if (*cur == '4') - b = (got_minus ? STAP_ARG_BITNESS_32BIT_SIGNED - : STAP_ARG_BITNESS_32BIT_UNSIGNED); - else if (*cur == '8') - b = (got_minus ? STAP_ARG_BITNESS_64BIT_SIGNED - : STAP_ARG_BITNESS_64BIT_UNSIGNED); - else + /* Defining the bitness. */ + switch (*cur) { - /* We have an error, because we don't expect anything - except 4 and 8. */ - complaint (&symfile_complaints, - _("unrecognized bitness `%c' for probe `%s'"), - *cur, probe->p.name); - return; + case '1': + b = (got_minus ? STAP_ARG_BITNESS_8BIT_SIGNED + : STAP_ARG_BITNESS_8BIT_UNSIGNED); + break; + + case '2': + b = (got_minus ? STAP_ARG_BITNESS_16BIT_SIGNED + : STAP_ARG_BITNESS_16BIT_UNSIGNED); + break; + + case '4': + b = (got_minus ? STAP_ARG_BITNESS_32BIT_SIGNED + : STAP_ARG_BITNESS_32BIT_UNSIGNED); + break; + + case '8': + b = (got_minus ? STAP_ARG_BITNESS_64BIT_SIGNED + : STAP_ARG_BITNESS_64BIT_UNSIGNED); + break; + + default: + { + /* We have an error, because we don't expect anything + except 1, 2, 4 and 8. */ + warning (_("unrecognized bitness %s%c' for probe `%s'"), + got_minus ? "`-" : "`", *cur, probe->p.name); + return; + } } arg.bitness = b; diff --git a/gdb/testsuite/gdb.arch/amd64-stap-optional-prefix.S b/gdb/testsuite/gdb.arch/amd64-stap-optional-prefix.S index 66689cb..3cdb4c2 100644 --- a/gdb/testsuite/gdb.arch/amd64-stap-optional-prefix.S +++ b/gdb/testsuite/gdb.arch/amd64-stap-optional-prefix.S @@ -28,5 +28,15 @@ main: STAP_PROBE1(probe, foo_prefix, 8@(%rsp)) STAP_PROBE1(probe, bar_prefix, 8@-8(%rbp)) mov %rbp,%rsp + STAP_PROBE1(probe, uint8_probe, 1@$8) + STAP_PROBE1(probe, int8_probe, -1@$-8) + STAP_PROBE1(probe, uint16_probe, 2@$16) + STAP_PROBE1(probe, int16_probe, -2@$-16) + STAP_PROBE1(probe, uint32_probe, 4@$32) + STAP_PROBE1(probe, int32_probe, -4@$-32) + STAP_PROBE1(probe, uint64_probe, 8@$64) + STAP_PROBE1(probe, int64_probe, -8@$-64) + STAP_PROBE1(probe, fail_probe, -7@$16) + STAP_PROBE1(probe, fail2_probe, 23-@$16) xor %rax,%rax ret diff --git a/gdb/testsuite/gdb.arch/amd64-stap-optional-prefix.exp b/gdb/testsuite/gdb.arch/amd64-stap-optional-prefix.exp index cc9d6c3..b7f1505 100644 --- a/gdb/testsuite/gdb.arch/amd64-stap-optional-prefix.exp +++ b/gdb/testsuite/gdb.arch/amd64-stap-optional-prefix.exp @@ -43,6 +43,11 @@ proc test_probe_value { value reg_val } { gdb_test "print \$_probe_arg0 == *((unsigned int *) (${reg_val}))" "= 1" } +proc test_probe_value_without_reg { value } { + gdb_test "print \$_probe_argc" "= 1" + gdb_test "print \$_probe_arg0" "= $value" +} + if { ![runto_main] } { return -1 } @@ -55,3 +60,32 @@ foreach probe_name [list "foo" "bar" "foo_prefix" "bar_prefix"] \ test_probe_value $probe_val $probe_reg_val } } + +# Testing normal probes, with several prefixes. + +set normal_probes_names { } + +foreach bit [list 8 16 32 64] { + lappend normal_probes_names "uint${bit}_probe" + lappend normal_probes_names "int${bit}_probe" +} + +foreach probe_name $normal_probes_names \ + probe_val [list 8 -8 16 -16 32 -32 64 -64] { + with_test_prefix $probe_name { + goto_probe $probe_name + test_probe_value_without_reg $probe_val + } +} + +# Testing the fail probes. + +with_test_prefix "fail_probe" { + goto_probe "fail_probe" + gdb_test "print \$_probe_arg0" "warning: unrecognized bitness `-7' for probe `fail_probe'\r\nInvalid probe argument 0 -- probe has 0 arguments available" +} + +with_test_prefix "fail2_probe" { + goto_probe "fail2_probe" + gdb_test "print \$_probe_arg0" "Unknown numeric token on expression `23-@\\\$16'." +} --=-=-=--