public inbox for systemtap@sourceware.org
 help / color / mirror / Atom feed
* [Bug testsuite/14574] New: Add test coverage for stapdyn's runtime
@ 2012-09-12 21:26 jistone at redhat dot com
  2012-09-19 15:50 ` [Bug testsuite/14574] " fche at redhat dot com
                   ` (11 more replies)
  0 siblings, 12 replies; 13+ messages in thread
From: jistone at redhat dot com @ 2012-09-12 21:26 UTC (permalink / raw)
  To: systemtap

http://sourceware.org/bugzilla/show_bug.cgi?id=14574

             Bug #: 14574
           Summary: Add test coverage for stapdyn's runtime
           Product: systemtap
           Version: unspecified
            Status: NEW
          Severity: normal
          Priority: P2
         Component: testsuite
        AssignedTo: systemtap@sourceware.org
        ReportedBy: jistone@redhat.com
    Classification: Unclassified


We need to add stapdyn to the testsuite!

This may include specific tests for stapdyn, but most of the runtime
functionality is supposed to work identically, so we should also reuse as many
of those tests as we can.

Perhaps start with a couple helpers in lib/systemtap.exp, answering (a) is
dyninst enabled? and (b) what are all enabled --runtimes?  Then tests which are
generally applicable / runtime-agnostic can loop over that runtime list.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Bug testsuite/14574] Add test coverage for stapdyn's runtime
  2012-09-12 21:26 [Bug testsuite/14574] New: Add test coverage for stapdyn's runtime jistone at redhat dot com
@ 2012-09-19 15:50 ` fche at redhat dot com
  2012-09-20 18:53 ` dsmith at redhat dot com
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: fche at redhat dot com @ 2012-09-19 15:50 UTC (permalink / raw)
  To: systemtap


http://sourceware.org/bugzilla/show_bug.cgi?id=14574

Frank Ch. Eigler <fche at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |fche at redhat dot com

--- Comment #1 from Frank Ch. Eigler <fche at redhat dot com> 2012-09-19 15:49:53 UTC ---
test

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Bug testsuite/14574] Add test coverage for stapdyn's runtime
  2012-09-12 21:26 [Bug testsuite/14574] New: Add test coverage for stapdyn's runtime jistone at redhat dot com
  2012-09-19 15:50 ` [Bug testsuite/14574] " fche at redhat dot com
@ 2012-09-20 18:53 ` dsmith at redhat dot com
  2012-09-20 19:12 ` dsmith at redhat dot com
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: dsmith at redhat dot com @ 2012-09-20 18:53 UTC (permalink / raw)
  To: systemtap


http://sourceware.org/bugzilla/show_bug.cgi?id=14574

--- Comment #2 from David Smith <dsmith at redhat dot com> 2012-09-20 18:52:28 UTC ---
Created attachment 6640
  --> http://sourceware.org/bugzilla/attachment.cgi?id=6640
Alpha (non-working) patch.

Here's a patch that implements a 'dyninst_p' function and a function that
returns the list of supported runtimes.

However, the testcase that has been modified to use the new functions doesn't
work, for several reasons (some listed in comments in the patch):

- The dyninst runtime requires a target executable. Finding an appropriate one
is tricky (on 32-bit x86 at least), and I'm having trouble with redirection. 
The latter is probably a tcl problem.

- Since end probes aren't currently working with the dyninst runtime, the test
scripts themselves are failing (since the map_hash.exp testcase uses the auto
end variable printing feature).

Comments on the general approach would still be appreciated.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Bug testsuite/14574] Add test coverage for stapdyn's runtime
  2012-09-12 21:26 [Bug testsuite/14574] New: Add test coverage for stapdyn's runtime jistone at redhat dot com
  2012-09-19 15:50 ` [Bug testsuite/14574] " fche at redhat dot com
  2012-09-20 18:53 ` dsmith at redhat dot com
@ 2012-09-20 19:12 ` dsmith at redhat dot com
  2012-09-20 20:14 ` jistone at redhat dot com
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: dsmith at redhat dot com @ 2012-09-20 19:12 UTC (permalink / raw)
  To: systemtap


http://sourceware.org/bugzilla/show_bug.cgi?id=14574

David Smith <dsmith at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dsmith at redhat dot com

--- Comment #3 from David Smith <dsmith at redhat dot com> 2012-09-20 19:12:10 UTC ---
I've also been looking at modifying testsuite/systemtap.pass1-4/buildok.exp. 
We've got 2 main problems here:

- Several of the test files buildok.exp operates on in testsuite/buildok/
aren't really systemtap scripts but shell scripts (i.e. they start with "#!
/bin/sh").  I don't see any way of passing in '--runtime=dyninst' to those
testcases.  The affected test files are:

  testsuite/buildok/cmdline01.stp:#!/bin/sh
  testsuite/buildok/fortytwo.stp:#! /bin/sh
  testsuite/buildok/oldlocals01.stp:#! /bin/sh
  testsuite/buildok/scsi-detailed.stp:#! /bin/sh
  testsuite/buildok/thirtythree.stp:#! /bin/sh
  testsuite/buildok/thirtytwo.stp:#! /bin/sh
  testsuite/buildok/two.stp:#! /bin/sh

- The builok.exp testcase uses 'stap_run_batch' (from
testsuite/lib/systemtap.exp).  You can pass as many arguments to that function
as you like, but it only looks at the first arg (as a filename).  So, passing
in the extra "--runtime=dyninst" argument will mean some changes to
'stap_run_batch'.  (This might be a problem for systemtap.server/client.exp,
since it tries to pass extra args to 'stap_run_batch'.)

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Bug testsuite/14574] Add test coverage for stapdyn's runtime
  2012-09-12 21:26 [Bug testsuite/14574] New: Add test coverage for stapdyn's runtime jistone at redhat dot com
                   ` (2 preceding siblings ...)
  2012-09-20 19:12 ` dsmith at redhat dot com
@ 2012-09-20 20:14 ` jistone at redhat dot com
  2012-09-21 16:07 ` dsmith at redhat dot com
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: jistone at redhat dot com @ 2012-09-20 20:14 UTC (permalink / raw)
  To: systemtap


http://sourceware.org/bugzilla/show_bug.cgi?id=14574

--- Comment #4 from Josh Stone <jistone at redhat dot com> 2012-09-20 20:14:07 UTC ---
(In reply to comment #2)
> Created attachment 6640 [details]
> Alpha (non-working) patch.
> 
> Here's a patch that implements a 'dyninst_p' function and a function that
> returns the list of supported runtimes.
> 
> However, the testcase that has been modified to use the new functions doesn't
> work, for several reasons (some listed in comments in the patch):
> 
> - The dyninst runtime requires a target executable. Finding an appropriate one
> is tricky (on 32-bit x86 at least), and I'm having trouble with redirection. 
> The latter is probably a tcl problem.

From your patch:
+# FIXME: For dyninst, we currently have to have a target executable
+# (even though the script doesn't actually need one). A first attempt
+# was something simple like '/bin/true'. However, on x86 that gets
+# you:
+#
+#   arch-x86.C[4377]:  invalid immediate size 0 in insn
+#   symtab.C[1848]:  failed to getAnnotations here [main]
+#   symtab.C[1881]:  failed to getAnnotations here [main]

These sorts of errors should be fixed now in dyninst.git, and I'll be updating
Fedora packages with a new snapshot soon.  (both in official F18/rawhide and in
http://repos.fedorapeople.org/repos/jistone/dyninst/ for F16/17)

+# Sigh. So, we'll use 'stap' instead. Notice we've got to get rid of
+# stap's output, to avoid confusing expect.
+#
+#   stap_runtimes_run2 $srcdir/$subdir/$test -c "stap -V > /dev/null 2>&1"
+#
+# Sigh again. The above doesn't work, I get "ambiguous redirect".
+#
+# Here's another stab:
+stap_runtimes_run2 $srcdir/$subdir/$test -c "/bin/sh < /dev/null"
+# Sigh, sigh, sigh.  the above doesn't work either - FAIL.

Curious.  Do these loads work for normal kernel-mode stap?  I wonder if your
space before /dev/null is confusing something?

Another problem is that because of the redirects, stapio would usually back off
and run these indirectly as sh -c '...'.  Even the second sh load would do this
to get the redirect.  I tried to do this for stapdyn too, not sure how well,
but since we aren't doing multiprocess yet, we'll *only* attach to that outer
"sh -c" process.  Maybe that's ok for what you're doing, but worth noting.

> - Since end probes aren't currently working with the dyninst runtime, the test
> scripts themselves are failing (since the map_hash.exp testcase uses the auto
> end variable printing feature).

Ok, well let's just focus on test that don't need "end" for now. :/

> Comments on the general approach would still be appreciated.

I worry a little about how often "stap -V" will run for every dyninst_p call -
can that memoize itself in a global?  Or maybe be set at configure time through
a HAVE_DYNINST replacement?

(In reply to comment #3)
> I've also been looking at modifying testsuite/systemtap.pass1-4/buildok.exp. 
> We've got 2 main problems here:
> 
> - Several of the test files buildok.exp operates on in testsuite/buildok/
> aren't really systemtap scripts but shell scripts (i.e. they start with "#!
> /bin/sh").  I don't see any way of passing in '--runtime=dyninst' to those
> testcases.  The affected test files are:
> 
>   testsuite/buildok/cmdline01.stp:#!/bin/sh
>   testsuite/buildok/fortytwo.stp:#! /bin/sh
>   testsuite/buildok/oldlocals01.stp:#! /bin/sh
>   testsuite/buildok/scsi-detailed.stp:#! /bin/sh
>   testsuite/buildok/thirtythree.stp:#! /bin/sh
>   testsuite/buildok/thirtytwo.stp:#! /bin/sh
>   testsuite/buildok/two.stp:#! /bin/sh

They'd have to be modified appropriately, but can't they just use their
parameters as $1 or even "$@" in the script?  

Also, how do you propose filtering kernel-specific buildok tests out?

> - The builok.exp testcase uses 'stap_run_batch' (from
> testsuite/lib/systemtap.exp).  You can pass as many arguments to that function
> as you like, but it only looks at the first arg (as a filename).  So, passing
> in the extra "--runtime=dyninst" argument will mean some changes to
> 'stap_run_batch'.  (This might be a problem for systemtap.server/client.exp,
> since it tries to pass extra args to 'stap_run_batch'.)

Do you mean that client.exp is broken now, since its extra args are ignored? 
Sounds like fixing stap_run_batch is a good idea though.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Bug testsuite/14574] Add test coverage for stapdyn's runtime
  2012-09-12 21:26 [Bug testsuite/14574] New: Add test coverage for stapdyn's runtime jistone at redhat dot com
                   ` (3 preceding siblings ...)
  2012-09-20 20:14 ` jistone at redhat dot com
@ 2012-09-21 16:07 ` dsmith at redhat dot com
  2012-09-21 16:17 ` dsmith at redhat dot com
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: dsmith at redhat dot com @ 2012-09-21 16:07 UTC (permalink / raw)
  To: systemtap


http://sourceware.org/bugzilla/show_bug.cgi?id=14574

--- Comment #5 from David Smith <dsmith at redhat dot com> 2012-09-21 16:06:35 UTC ---
(In reply to comment #4)
> (In reply to comment #2)
> > Created attachment 6640 [details]
> > Alpha (non-working) patch.
> > 
> > Here's a patch that implements a 'dyninst_p' function and a function that
> > returns the list of supported runtimes.
> > 
> > However, the testcase that has been modified to use the new functions doesn't
> > work, for several reasons (some listed in comments in the patch):
> > 
> > - The dyninst runtime requires a target executable. Finding an appropriate one
> > is tricky (on 32-bit x86 at least), and I'm having trouble with redirection. 
> > The latter is probably a tcl problem.
> 
> From your patch:
> +# FIXME: For dyninst, we currently have to have a target executable
> +# (even though the script doesn't actually need one). A first attempt
> +# was something simple like '/bin/true'. However, on x86 that gets
> +# you:
> +#
> +#   arch-x86.C[4377]:  invalid immediate size 0 in insn
> +#   symtab.C[1848]:  failed to getAnnotations here [main]
> +#   symtab.C[1881]:  failed to getAnnotations here [main]
> 
> These sorts of errors should be fixed now in dyninst.git, and I'll be updating
> Fedora packages with a new snapshot soon.  (both in official F18/rawhide and in
> http://repos.fedorapeople.org/repos/jistone/dyninst/ for F16/17)


Good deal, I'll check out the new packages.


> +# Sigh. So, we'll use 'stap' instead. Notice we've got to get rid of
> +# stap's output, to avoid confusing expect.
> +#
> +#   stap_runtimes_run2 $srcdir/$subdir/$test -c "stap -V > /dev/null 2>&1"
> +#
> +# Sigh again. The above doesn't work, I get "ambiguous redirect".
> +#
> +# Here's another stab:
> +stap_runtimes_run2 $srcdir/$subdir/$test -c "/bin/sh < /dev/null"
> +# Sigh, sigh, sigh.  the above doesn't work either - FAIL.
> 
> Curious.  Do these loads work for normal kernel-mode stap?  I wonder if your
> space before /dev/null is confusing something?
> 
> Another problem is that because of the redirects, stapio would usually back off
> and run these indirectly as sh -c '...'.  Even the second sh load would do this
> to get the redirect.  I tried to do this for stapdyn too, not sure how well,
> but since we aren't doing multiprocess yet, we'll *only* attach to that outer
> "sh -c" process.  Maybe that's ok for what you're doing, but worth noting.


I'm sure the errors I was getting here with my redirection efforts are tcl
quoting problems, since redirections like that work fine from the command line.


> > - Since end probes aren't currently working with the dyninst runtime, the test
> > scripts themselves are failing (since the map_hash.exp testcase uses the auto
> > end variable printing feature).
> 
> Ok, well let's just focus on test that don't need "end" for now. :/
> 
> > Comments on the general approach would still be appreciated.
> 
> I worry a little about how often "stap -V" will run for every dyninst_p call -
> can that memoize itself in a global?  Or maybe be set at configure time through
> a HAVE_DYNINST replacement?


Running 'stap -V' too often could slow things down a bit.  We could cache the
fact whether dyninst is enabled in setup_systemtap_environment() (which is run
once I believe) in an environment variable, then check that environment
variable in dyninst_p().


> (In reply to comment #3)
> > I've also been looking at modifying testsuite/systemtap.pass1-4/buildok.exp. 
> > We've got 2 main problems here:
> > 
> > - Several of the test files buildok.exp operates on in testsuite/buildok/
> > aren't really systemtap scripts but shell scripts (i.e. they start with "#!
> > /bin/sh").  I don't see any way of passing in '--runtime=dyninst' to those
> > testcases.  The affected test files are:
> > 
> >   testsuite/buildok/cmdline01.stp:#!/bin/sh
> >   testsuite/buildok/fortytwo.stp:#! /bin/sh
> >   testsuite/buildok/oldlocals01.stp:#! /bin/sh
> >   testsuite/buildok/scsi-detailed.stp:#! /bin/sh
> >   testsuite/buildok/thirtythree.stp:#! /bin/sh
> >   testsuite/buildok/thirtytwo.stp:#! /bin/sh
> >   testsuite/buildok/two.stp:#! /bin/sh
> 
> They'd have to be modified appropriately, but can't they just use their
> parameters as $1 or even "$@" in the script?  


Hmm, I hadn't thought of that I'll give it a shot.


> Also, how do you propose filtering kernel-specific buildok tests out?


I'm about to post a new version of buildok.exp.


> > - The builok.exp testcase uses 'stap_run_batch' (from
> > testsuite/lib/systemtap.exp).  You can pass as many arguments to that function
> > as you like, but it only looks at the first arg (as a filename).  So, passing
> > in the extra "--runtime=dyninst" argument will mean some changes to
> > 'stap_run_batch'.  (This might be a problem for systemtap.server/client.exp,
> > since it tries to pass extra args to 'stap_run_batch'.)
> 
> Do you mean that client.exp is broken now, since its extra args are ignored? 
> Sounds like fixing stap_run_batch is a good idea though.


It is possible that client.exp is broken.  I think I've fixed stap_run_batch,
and I'll see what that does to client.exp.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Bug testsuite/14574] Add test coverage for stapdyn's runtime
  2012-09-12 21:26 [Bug testsuite/14574] New: Add test coverage for stapdyn's runtime jistone at redhat dot com
                   ` (4 preceding siblings ...)
  2012-09-21 16:07 ` dsmith at redhat dot com
@ 2012-09-21 16:17 ` dsmith at redhat dot com
  2012-09-21 18:26 ` dsmith at redhat dot com
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: dsmith at redhat dot com @ 2012-09-21 16:17 UTC (permalink / raw)
  To: systemtap


http://sourceware.org/bugzilla/show_bug.cgi?id=14574

--- Comment #6 from David Smith <dsmith at redhat dot com> 2012-09-21 16:16:52 UTC ---
Created attachment 6643
  --> http://sourceware.org/bugzilla/attachment.cgi?id=6643
buildok testsuite patch

Here's a patch that modifies testsuite/systemtap.pass1-4/buildok.exp to work
for the default (linux) runtime and the dyninst runtime.

However, now that I've done it, I don't really like it.  There are about 18
lines of shared tcl code, and the rest (225 lines) is runtime-specific tcl
code.  This runtime-specific tcl code sets up kfails for each runtime.

I think it might make more sense to leave buildok.exp alone, and have a
separate dyninst-buildok.exp testcase.  Obviously splitting the testcase makes
it much easier to just test one specific runtime.

Comments welcome.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Bug testsuite/14574] Add test coverage for stapdyn's runtime
  2012-09-12 21:26 [Bug testsuite/14574] New: Add test coverage for stapdyn's runtime jistone at redhat dot com
                   ` (5 preceding siblings ...)
  2012-09-21 16:17 ` dsmith at redhat dot com
@ 2012-09-21 18:26 ` dsmith at redhat dot com
  2012-09-27 16:31 ` dsmith at redhat dot com
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: dsmith at redhat dot com @ 2012-09-21 18:26 UTC (permalink / raw)
  To: systemtap


http://sourceware.org/bugzilla/show_bug.cgi?id=14574

--- Comment #7 from David Smith <dsmith at redhat dot com> 2012-09-21 18:26:34 UTC ---
Created attachment 6646
  --> http://sourceware.org/bugzilla/attachment.cgi?id=6646
buildok dyninst test

Here's a buildok testcase that just tests the dyninst runtime individually. I'm
liking it better than the combined linux and dyninst buildok testcase in the
last patch.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Bug testsuite/14574] Add test coverage for stapdyn's runtime
  2012-09-12 21:26 [Bug testsuite/14574] New: Add test coverage for stapdyn's runtime jistone at redhat dot com
                   ` (6 preceding siblings ...)
  2012-09-21 18:26 ` dsmith at redhat dot com
@ 2012-09-27 16:31 ` dsmith at redhat dot com
  2012-09-27 17:26 ` jistone at redhat dot com
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: dsmith at redhat dot com @ 2012-09-27 16:31 UTC (permalink / raw)
  To: systemtap


http://sourceware.org/bugzilla/show_bug.cgi?id=14574

--- Comment #8 from David Smith <dsmith at redhat dot com> 2012-09-27 16:30:56 UTC ---
Commit 8c94efa modifies the systemtap.pass1-4/*.exp tests to also include
dyninst.

Most of the cases are modified like this:

====
foreach runtime [get_runtime_list] {
    if {$runtime != ""} {
    # run test, add '--runtime=$runtime'
    } else {
    # run test
    }
}
===

Each testcase (where it makes sense) will need to be modified to add dyninst
support.  Hopefully if we get more runtimes the testsuite will accommodate them
much easier.

There are 2 remaining obstacles to getting the testsuite working well with
dyninst:

1) Dyninst outputs some debug messages that interferes with the testsuite
output matching.  Lines like:

  Setting bs state to 1
  found target /usr/lib/libpthread-2.16.so 0x7074
  found target /usr/lib/libpthread-2.16.so 0x7985
  found target /usr/lib/libpthread-2.16.so 0x7a3d
  found target /usr/lib/libpthread-2.16.so 0x6aed

2) Dyninst requires a test executable.  Now that probing /bin/true works, this
one isn't a "must have", but it would be nice to fix this one.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Bug testsuite/14574] Add test coverage for stapdyn's runtime
  2012-09-12 21:26 [Bug testsuite/14574] New: Add test coverage for stapdyn's runtime jistone at redhat dot com
                   ` (7 preceding siblings ...)
  2012-09-27 16:31 ` dsmith at redhat dot com
@ 2012-09-27 17:26 ` jistone at redhat dot com
  2012-09-27 17:48 ` dsmith at redhat dot com
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: jistone at redhat dot com @ 2012-09-27 17:26 UTC (permalink / raw)
  To: systemtap


http://sourceware.org/bugzilla/show_bug.cgi?id=14574

--- Comment #9 from Josh Stone <jistone at redhat dot com> 2012-09-27 17:26:25 UTC ---
(In reply to comment #8)
> Commit 8c94efa modifies the systemtap.pass1-4/*.exp tests to also include
> dyninst.
> 
> Most of the cases are modified like this:
> 
> ====
> foreach runtime [get_runtime_list] {
>     if {$runtime != ""} {
>     # run test, add '--runtime=$runtime'
>     } else {
>     # run test
>     }
> }
> ===

Must we have the if-else duplicated in every test?  Why can't get_runtime_list
return the full option already prepared?  If it helps, we could be explicit
about the default - instead of "" go ahead and spell out "--runtime=kernel".

Side question: in tapset/ and runtime/ subdirectories I chose linux/ for our
traditional kernel mode.  Do you think the --runtime option should do the same?
 Or perhaps both --runtime=kernel and --runtime=linux should be accepted as
aliases?

> Each testcase (where it makes sense) will need to be modified to add dyninst
> support.  Hopefully if we get more runtimes the testsuite will accommodate them
> much easier.
> 
> There are 2 remaining obstacles to getting the testsuite working well with
> dyninst:
> 
> 1) Dyninst outputs some debug messages that interferes with the testsuite
> output matching.  Lines like:
> 
>   Setting bs state to 1

This line is indeed from dyninst.  It's already been removed upstream, but that
was after I made my latest rpm snapshot.

>   found target /usr/lib/libpthread-2.16.so 0x7074
>   found target /usr/lib/libpthread-2.16.so 0x7985
>   found target /usr/lib/libpthread-2.16.so 0x7a3d
>   found target /usr/lib/libpthread-2.16.so 0x6aed

These are from stapdyn.  I really need to clean these up and hide them behind
-v flags.  If they're causing testing trouble, that's more reason for me to do
this sooner than later.

> 2) Dyninst requires a test executable.  Now that probing /bin/true works, this
> one isn't a "must have", but it would be nice to fix this one.

Part of the metadata rework I'm doing involves dlopen'ing the module in
stapdyn.  With that loaded, we could easily call the init/exit functions
directly when there's no target.  In fact, once we get multiprocess and shared
memory going, we'll probably always call init/exit this way.

So while targetless stapdyn has no practical use in general, the sort of toy
scripts used in testing are important enough that we should make it work.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Bug testsuite/14574] Add test coverage for stapdyn's runtime
  2012-09-12 21:26 [Bug testsuite/14574] New: Add test coverage for stapdyn's runtime jistone at redhat dot com
                   ` (8 preceding siblings ...)
  2012-09-27 17:26 ` jistone at redhat dot com
@ 2012-09-27 17:48 ` dsmith at redhat dot com
  2012-09-27 20:31 ` jistone at redhat dot com
  2012-10-08 21:44 ` jistone at redhat dot com
  11 siblings, 0 replies; 13+ messages in thread
From: dsmith at redhat dot com @ 2012-09-27 17:48 UTC (permalink / raw)
  To: systemtap


http://sourceware.org/bugzilla/show_bug.cgi?id=14574

--- Comment #10 from David Smith <dsmith at redhat dot com> 2012-09-27 17:48:03 UTC ---
(In reply to comment #9)
> (In reply to comment #8)
> > Commit 8c94efa modifies the systemtap.pass1-4/*.exp tests to also include
> > dyninst.
> > 
> > Most of the cases are modified like this:
> > 
> > ====
> > foreach runtime [get_runtime_list] {
> >     if {$runtime != ""} {
> >     # run test, add '--runtime=$runtime'
> >     } else {
> >     # run test
> >     }
> > }
> > ===
> 
> Must we have the if-else duplicated in every test?  Why can't get_runtime_list
> return the full option already prepared?  If it helps, we could be explicit
> about the default - instead of "" go ahead and spell out "--runtime=kernel".

Originally I had something like the following:

====
foreach runtime [get_runtime_list] {
    if {$runtime != ""} {
      args = "--runtime=$runtime"
    } else {
      args = ""
    }
    # run test, passing $args
}
===

But, I had problems with passing the empty arg. Stap actually got an empty arg,
which confused it.

I went back and forth on whether 'get_runtime_list' should return the full
option. I went the current solution because I append the runtime name to the
test name.  That means you get passes/fails that look like this:

===
FAIL: buildok/string-embedded.stp (dyninst)
FAIL: buildok/system-embedded.stp (dyninst)
===

That output looked a bit better to me, but I could be argued the other way.

As far as doing something like "--runtime=linux", I'm unsure.  One possible
problem here would be with comparing old test results (since we'd be changing
the all the test names).

> Side question: in tapset/ and runtime/ subdirectories I chose linux/ for our
> traditional kernel mode.  Do you think the --runtime option should do the same?
>  Or perhaps both --runtime=kernel and --runtime=linux should be accepted as
> aliases?
> 
> > Each testcase (where it makes sense) will need to be modified to add dyninst
> > support.  Hopefully if we get more runtimes the testsuite will accommodate them
> > much easier.
> > 
> > There are 2 remaining obstacles to getting the testsuite working well with
> > dyninst:
> > 
> > 1) Dyninst outputs some debug messages that interferes with the testsuite
> > output matching.  Lines like:
> > 
> >   Setting bs state to 1
> 
> This line is indeed from dyninst.  It's already been removed upstream, but that
> was after I made my latest rpm snapshot.
> 
> >   found target /usr/lib/libpthread-2.16.so 0x7074
> >   found target /usr/lib/libpthread-2.16.so 0x7985
> >   found target /usr/lib/libpthread-2.16.so 0x7a3d
> >   found target /usr/lib/libpthread-2.16.so 0x6aed
> 
> These are from stapdyn.  I really need to clean these up and hide them behind
> -v flags.  If they're causing testing trouble, that's more reason for me to do
> this sooner than later.

Glad to hear this is being worked on.

> > 2) Dyninst requires a test executable.  Now that probing /bin/true works, this
> > one isn't a "must have", but it would be nice to fix this one.
> 
> Part of the metadata rework I'm doing involves dlopen'ing the module in
> stapdyn.  With that loaded, we could easily call the init/exit functions
> directly when there's no target.  In fact, once we get multiprocess and shared
> memory going, we'll probably always call init/exit this way.
> 
> So while targetless stapdyn has no practical use in general, the sort of toy
> scripts used in testing are important enough that we should make it work.

Glad to hear this is being worked on also.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Bug testsuite/14574] Add test coverage for stapdyn's runtime
  2012-09-12 21:26 [Bug testsuite/14574] New: Add test coverage for stapdyn's runtime jistone at redhat dot com
                   ` (9 preceding siblings ...)
  2012-09-27 17:48 ` dsmith at redhat dot com
@ 2012-09-27 20:31 ` jistone at redhat dot com
  2012-10-08 21:44 ` jistone at redhat dot com
  11 siblings, 0 replies; 13+ messages in thread
From: jistone at redhat dot com @ 2012-09-27 20:31 UTC (permalink / raw)
  To: systemtap


http://sourceware.org/bugzilla/show_bug.cgi?id=14574

--- Comment #11 from Josh Stone <jistone at redhat dot com> 2012-09-27 20:31:01 UTC ---
(In reply to comment #10)
> > So while targetless stapdyn has no practical use in general, the sort of toy
> > scripts used in testing are important enough that we should make it work.
> 
> Glad to hear this is being worked on also.

Commit fe3f046 should let basic begin/end/error scripts work without a command.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Bug testsuite/14574] Add test coverage for stapdyn's runtime
  2012-09-12 21:26 [Bug testsuite/14574] New: Add test coverage for stapdyn's runtime jistone at redhat dot com
                   ` (10 preceding siblings ...)
  2012-09-27 20:31 ` jistone at redhat dot com
@ 2012-10-08 21:44 ` jistone at redhat dot com
  11 siblings, 0 replies; 13+ messages in thread
From: jistone at redhat dot com @ 2012-10-08 21:44 UTC (permalink / raw)
  To: systemtap


http://sourceware.org/bugzilla/show_bug.cgi?id=14574

--- Comment #12 from Josh Stone <jistone at redhat dot com> 2012-10-08 21:43:43 UTC ---
(In reply to comment #10)
> > > 1) Dyninst outputs some debug messages that interferes with the testsuite
> > > output matching.  Lines like:
> > > 
> > >   Setting bs state to 1
> > 
> > This line is indeed from dyninst.  It's already been removed upstream, but that
> > was after I made my latest rpm snapshot.

As mentioned, this is fixed in dyninst.git HEAD, and Fedora's 0.26 snapshot
should now have this too.

> > >   found target /usr/lib/libpthread-2.16.so 0x7074
> > >   found target /usr/lib/libpthread-2.16.so 0x7985
> > >   found target /usr/lib/libpthread-2.16.so 0x7a3d
> > >   found target /usr/lib/libpthread-2.16.so 0x6aed
> > 
> > These are from stapdyn.  I really need to clean these up and hide them behind
> > -v flags.  If they're causing testing trouble, that's more reason for me to do
> > this sooner than later.
> 
> Glad to hear this is being worked on.

stap commit 41300e6d cleaned this up.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2012-10-08 21:44 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-09-12 21:26 [Bug testsuite/14574] New: Add test coverage for stapdyn's runtime jistone at redhat dot com
2012-09-19 15:50 ` [Bug testsuite/14574] " fche at redhat dot com
2012-09-20 18:53 ` dsmith at redhat dot com
2012-09-20 19:12 ` dsmith at redhat dot com
2012-09-20 20:14 ` jistone at redhat dot com
2012-09-21 16:07 ` dsmith at redhat dot com
2012-09-21 16:17 ` dsmith at redhat dot com
2012-09-21 18:26 ` dsmith at redhat dot com
2012-09-27 16:31 ` dsmith at redhat dot com
2012-09-27 17:26 ` jistone at redhat dot com
2012-09-27 17:48 ` dsmith at redhat dot com
2012-09-27 20:31 ` jistone at redhat dot com
2012-10-08 21:44 ` jistone at redhat dot com

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).