From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 1715 invoked by alias); 27 Jun 2013 19:35:02 -0000 Mailing-List: contact systemtap-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Post: List-Help: , Sender: systemtap-owner@sourceware.org Received: (qmail 1620 invoked by uid 89); 27 Jun 2013 19:34:59 -0000 X-Spam-SWARE-Status: No, score=-4.1 required=5.0 tests=BAYES_00,FREEMAIL_FROM,KHOP_THREADED,RCVD_IN_DNSWL_LOW,RCVD_IN_HOSTKARMA_YE,SPF_PASS autolearn=ham version=3.3.1 Received: from mail-ob0-f178.google.com (HELO mail-ob0-f178.google.com) (209.85.214.178) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Thu, 27 Jun 2013 19:34:57 +0000 Received: by mail-ob0-f178.google.com with SMTP id fb19so1164362obc.9 for ; Thu, 27 Jun 2013 12:34:56 -0700 (PDT) MIME-Version: 1.0 X-Received: by 10.60.123.112 with SMTP id lz16mr3665776oeb.88.1372361696140; Thu, 27 Jun 2013 12:34:56 -0700 (PDT) Received: by 10.182.65.195 with HTTP; Thu, 27 Jun 2013 12:34:56 -0700 (PDT) In-Reply-To: <51CB6DDB.7080203@redhat.com> References: <51C8EF57.2050801@redhat.com> <1372225343-3618-1-git-send-email-agentzh@gmail.com> <1372225343-3618-2-git-send-email-agentzh@gmail.com> <51CB6DDB.7080203@redhat.com> Date: Thu, 27 Jun 2013 19:35:00 -0000 Message-ID: Subject: Re: [PATCH v3 1/1] PR11096: Add support for the "module" argument to @var From: "Yichun Zhang (agentzh)" To: Josh Stone Cc: systemtap@sourceware.org Content-Type: text/plain; charset=UTF-8 X-SW-Source: 2013-q2/txt/msg00389.txt.bz2 Hello Josh! On Wed, Jun 26, 2013 at 3:40 PM, Josh Stone wrote: > I have just two small comments, which you can probably just fix without > further review, then I think this is ready to push! > Thank you for your suggestions! I've fixed these and also added some tests for them. Below is the diff against my local branch with my v3 patch applied. Please have a look :) BTW, the newly added sdt_global_var.exp test file also uncovers PR15688: http://sourceware.org/bugzilla/show_bug.cgi?id=15688 I've marked the corresponding subtests with the PR number. Thanks! -agentzh diff --git a/tapsets.cxx b/tapsets.cxx index c67beaf..05dbd9a 100644 --- a/tapsets.cxx +++ b/tapsets.cxx @@ -3688,23 +3688,18 @@ dwarf_var_expanding_visitor::visit_target_symbol_context (target_symbol* e) void dwarf_var_expanding_visitor::visit_atvar_op (atvar_op *e) { - if (e->module.empty() && e->cu_name.empty()) - { - // Fill in the module name so that even if there is no match among - // local variables, dwarf_atvar_expanding_visitor can still scan - // all the CUs in the current module for a global variable match. - e->module = q.dw.module_name; + // Fill in our current module context if needed + if (e->module.empty()) + e->module = q.dw.module_name; + if (e->module == q.dw.module_name && e->cu_name.empty()) + { // process like any other local // e->sym_name() will do the right thing visit_target_symbol(e); return; } - // Fill in our current module context if needed - if (e->module.empty()) - e->module = q.dw.module_name; - var_expanding_visitor::visit_atvar_op(e); } @@ -4299,9 +4294,9 @@ dwarf_atvar_expanding_visitor::visit_atvar_op (atvar_op* e) /* Unable to find the variable in the current module, so we chain * an error in atvar_op */ - semantic_error er(_F("unable to find global '%s' in %s, %s %s", + semantic_error er(_F("unable to find global '%s' in %s%s%s", e->sym_name().c_str(), module.c_str(), - e->cu_name.empty() ? "" : _("in"), + e->cu_name.empty() ? "" : _(", in "), e->cu_name.c_str())); e->chain (er); } @@ -6129,13 +6124,7 @@ sdt_uprobe_var_expanding_visitor::visit_target_symbol (target_symbol* e) void sdt_uprobe_var_expanding_visitor::visit_atvar_op (atvar_op* e) { - if (e->module.empty() && e->cu_name.empty()) - { - // process like any other local - // e->sym_name() will do the right thing - visit_target_symbol(e); - return; - } + need_debug_info = true; // Fill in our current module context if needed if (e->module.empty()) diff --git a/testsuite/systemtap.base/at_var_func.exp b/testsuite/systemtap.base/at_var_func.exp index f515bab..cf12d6e 100644 --- a/testsuite/systemtap.base/at_var_func.exp +++ b/testsuite/systemtap.base/at_var_func.exp @@ -16,7 +16,8 @@ set ::result_string {@var("foo", @1)->bar: 42 @var("foo", @1)$$: {.bar=42} @defined(@var("foo", "badmodle")->bar): NO @defined(@var("foo", @3)->bar): NO -@defined(@var("foo@blah.c", @1)->bar): NO} +@defined(@var("foo@blah.c", @1)->bar): NO +@var("foo")->bar: 42} # Only run on make installcheck and uprobes present. if {! [installtest_p]} { untested "$test"; return } diff --git a/testsuite/systemtap.base/at_var_func.stp b/testsuite/systemtap.base/at_var_func.stp index 3da6536..5d12656 100644 --- a/testsuite/systemtap.base/at_var_func.stp +++ b/testsuite/systemtap.base/at_var_func.stp @@ -18,4 +18,5 @@ function f() probe process.function("sub") { f() + printf("@var(\"foo\")->bar: %d\n", @var("foo")->bar); } diff --git a/testsuite/systemtap.base/at_var_mark.exp b/testsuite/systemtap.base/at_var_mark.exp index a3b1199..7b2074f 100644 --- a/testsuite/systemtap.base/at_var_mark.exp +++ b/testsuite/systemtap.base/at_var_mark.exp @@ -4,7 +4,7 @@ set testpath "$srcdir/$subdir" set stap_path $env(SYSTEMTAP_PATH)/stap # Check @var, even when var doesn't exist, works in process.mark probes. -set output_string "pass:yes:0\r\n" +set output_string "s = \\d+\r\n.*pass:yes:0\r\n" set invoke "$stap_path -e 'probe begin \{ exit() \}'" # Only run on make installcheck and uprobes present. diff --git a/testsuite/systemtap.base/at_var_mark.stp b/testsuite/systemtap.base/at_var_mark.stp index fe47bf9..0481cb0 100644 --- a/testsuite/systemtap.base/at_var_mark.stp +++ b/testsuite/systemtap.base/at_var_mark.stp @@ -5,11 +5,12 @@ probe process.mark("pass*") if (more_addr == 0) { log("systemtap starting probe"); + log("systemtap ending probe"); more_addr = @var("morehelp@session.cxx"); + printf("s = %d\n", @var("s")) } else { - log("systemtap ending probe"); name = substr($$name, 0, 4); correct = @defined(@var("no_such_var_really_not")) ? "no" : "yes"; diff = more_addr - @var("morehelp@session.cxx"); diff --git a/testsuite/systemtap.base/at_var_pie.exp b/testsuite/systemtap.base/at_var_pie.exp index 2cabb8f..9433571 100644 --- a/testsuite/systemtap.base/at_var_pie.exp +++ b/testsuite/systemtap.base/at_var_pie.exp @@ -13,7 +13,8 @@ set ::result_string {@var("foo", @1)->bar: 42 @var("foo", @1)$$: {.bar=42} @defined(@var("foo", "badmodle")->bar): NO @defined(@var("foo", @3)->bar): NO -@defined(@var("foo@blah.c", @1)->bar): NO} +@defined(@var("foo@blah.c", @1)->bar): NO +@var("foo")->bar: 42} # Only run on make installcheck and uprobes present. if {! [installtest_p]} { untested "$test"; return } diff --git a/testsuite/systemtap.base/at_var_unresolved.exp b/testsuite/systemtap.base/at_var_unresolved.exp index 10db59d..8247224 100644 --- a/testsuite/systemtap.base/at_var_unresolved.exp +++ b/testsuite/systemtap.base/at_var_unresolved.exp @@ -13,7 +13,7 @@ set errs 0 expect { -timeout 180 - -re {^semantic error: unable to find global 'var_really_not_exist'.*? in kernel} { + -re {^semantic error: unable to find global 'var_really_not_exist'.*? in kernel: operator '@var'} { incr errs; exp_continue } diff --git a/testsuite/systemtap.base/sdt.c b/testsuite/systemtap.base/sdt.c index 39f4e88..58ea705 100644 --- a/testsuite/systemtap.base/sdt.c +++ b/testsuite/systemtap.base/sdt.c @@ -1,8 +1,11 @@ #include "sys/sdt.h" +int my_global_var = 56; + static void call0(void) { STAP_PROBE(test, mark_z); + my_global_var++; } static void call1(int a) diff --git a/testsuite/systemtap.base/sdt_global_var.exp b/testsuite/systemtap.base/sdt_global_var.exp new file mode 100644 index 0000000..e29148c --- /dev/null +++ b/testsuite/systemtap.base/sdt_global_var.exp @@ -0,0 +1,122 @@ +set test "sdt_global_var" + +# Test accessing global variables in SDT probes. +set result_string10 "z: @(\"my_global_var\") = 56 +z: \$my_global_var = 56 +j: @(\"my_global_var\") = 57 +j: \$my_global_var = 57" +set result_string12 " +l: @(\"my_global_var\") = 57 +l: \$my_global_var = 57" + +set extra_flags {{additional_flags=-O2} {additional_flags=-fPIE additional_flags=-pie additional_flags=-O2}} +set extra_mssgs {-O2} + +set pbtype_flags {{""} {additional_flags=-DSTAP_SDT_V2}} +set pbtype_mssgs {{uprobe} {V2_uprobe}} + +proc sdt_stap_run { TEST_NAME TEST_FILE FAIL args } { + foreach runtime [get_runtime_list] { + set full_test_name $TEST_NAME + if {$runtime != ""} { + if {$runtime == "dyninst" && [regexp "\-pie" $TEST_NAME]} { + setup_kfail 15052 "*-*-*" + } + + lappend full_test_name "($runtime)" + set cmd [concat stap --runtime=$runtime $TEST_FILE $args] + } elseif {[uprobes_p]} { + if {[regexp "\-pie" $TEST_NAME]} { + setup_kfail 15688 "*-*-*" + } + + set cmd [concat stap $TEST_FILE $args] + } else { + untested $full_test_name + continue + } + send_log "executing: $cmd\n" + catch {eval exec $cmd} res + + set skip 0 + set n 0 + set expected [split $::result_string "\n"] + foreach line [split $res "\n"] { + if {![string equal $line [lindex $expected $n]]} { + $FAIL "$full_test_name" + send_log "line [expr $n + 1]: expected \"[lindex $expected $n]\"\n" + send_log "Got \"$line\"\n" + set skip 1 + break + } + incr n + } + if {$skip != 0} { + continue + } + if {[expr $n == [llength $expected]]} { + pass "$full_test_name" + } else { + $FAIL "$full_test_name" + send_log "too few lines of output, got $n, expected [llength $expected]\n" + } + } +} + +# Iterate pbtype_flags +for {set p 0} {$p < [llength $pbtype_flags]} {incr p} { +set pbtype_flag [lindex $pbtype_flags $p] +set pbtype_mssg [lindex $pbtype_mssgs $p] + +# Iterate extra_flags, trying the cases with and without PIE +for {set i 0} {$i < [llength $extra_flags]} {incr i} { +set extra_flag [lindex $extra_flags $i] +set extra_mssg [lindex $extra_mssgs $i] +set testprog "sdt.c.exe.$i" + +set test_flags "additional_flags=-g" +set test_flags "$test_flags [sdt_includes]" +set test_flags "$test_flags additional_flags=-Wall" +set test_flags "$test_flags additional_flags=-Wextra" +set test_flags "$test_flags additional_flags=-Werror $pbtype_flag" + +set saveidx 0 + +set res [target_compile $srcdir/$subdir/sdt.c $testprog executable "$test_flags $extra_flag"] +if { $res != "" } { + verbose "target_compile failed: $res" 2 + if {[string first "unrecognized command line option" $res] == -1} { + fail "compiling sdt.c $extra_mssg $pbtype_mssg" + } else { + untested "compiling sdt.c $extra_mssg $pbtype_mssg" + } + untested "$test $extra_mssg $pbtype_mssg" + continue +} else { + pass "compiling sdt.c $extra_mssg $pbtype_mssg" +} + +if {[installtest_p]} { + if {[regexp "^(ppc64|s390x)$" $::tcl_platform(machine)] \ + && [regexp "^(-O)" $extra_mssg]} { + # register name and constant is ambiguous in ppc/s390 asm syntax + # thus constant folding can throw the result off + set FAIL xfail + } else { + set FAIL fail + } + set ::result_string $result_string10 + if { $pbtype_mssg == "uprobe" } { + append ::result_string $result_string12 + } + sdt_stap_run "$test $extra_mssg $pbtype_mssg $extra_flag" \ + -w $FAIL $srcdir/$subdir/$test.stp $testprog -c ./$testprog +} else { + untested "$test $extra_mssg $pbtype_mssg" +} +catch {exec rm -f $testprog} + +} ; # for {set i 0} {$i < [llength $extra_flags]} + +} ; # for {set i 0} {$i < [llength $pbtype_flags]} + diff --git a/testsuite/systemtap.base/sdt_global_var.stp b/testsuite/systemtap.base/sdt_global_var.stp new file mode 100644 index 0000000..c259f34 --- /dev/null +++ b/testsuite/systemtap.base/sdt_global_var.stp @@ -0,0 +1,17 @@ +probe process(@1).mark("mark_z") +{ + printf("z: @(\"my_global_var\") = %d\n", @var("my_global_var")); + printf("z: $my_global_var = %d\n", $my_global_var); +} + +probe process(@1).mark("mark_j") +{ + printf("j: @(\"my_global_var\") = %d\n", @var("my_global_var")); + printf("j: $my_global_var = %d\n", $my_global_var); +} + +probe process(@1).mark("mark_l") ? +{ + printf("l: @(\"my_global_var\") = %d\n", @var("my_global_var")); + printf("l: $my_global_var = %d\n", $my_global_var); +}