public inbox for systemtap@sourceware.org
 help / color / mirror / Atom feed
* Emitting marker names instead of hex statement addresses for SDT probes?
@ 2021-01-13  4:49 Craig Ringer
  2021-01-19 22:21 ` Stan Cox
  0 siblings, 1 reply; 3+ messages in thread
From: Craig Ringer @ 2021-01-13  4:49 UTC (permalink / raw)
  To: systemtap; +Cc: Frank Ch. Eigler

Hi all

I'm looking for advice on how feasible it might be to have systemtap report
the SDT marker name in the probe-point name, instead of a
.statement(0xdeadbeef) string.

Because probes like process("foo").mark("bar") are converted to
process("foo").statement(0xdeadbeef), the monitor mode display is nearly
useless for SDTs. You just get a wall of undifferentiated hex statement
addresses. It also makes various errors and diagnostics much more difficult
to act on, and makes the output of pp() nearly useless for SDT markers.

Any advice on where I might start looking in the code if I wanted to change
this?

Also, ideally I'd also like to have $$name do something sensible for all
probe point types, so I don't need to

    defined($$name) ? $$name : ppfunc()

in macros and functions that can be invoked by both marker based and
DWARF-based probes.

Any thoughts on the most sensible value to assign to $$name for other probe
types? It could resolve to pp(), but that can be very long and verbose.
ppfunc() would be logical for function based probes but for others it's
less obvious.




-- 
 Craig Ringer                   http://www.2ndQuadrant.com/
 2ndQuadrant - PostgreSQL Solutions for the Enterprise

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

* Re: Emitting marker names instead of hex statement addresses for SDT probes?
  2021-01-13  4:49 Emitting marker names instead of hex statement addresses for SDT probes? Craig Ringer
@ 2021-01-19 22:21 ` Stan Cox
  2021-01-21  5:47   ` Craig Ringer
  0 siblings, 1 reply; 3+ messages in thread
From: Stan Cox @ 2021-01-19 22:21 UTC (permalink / raw)
  To: Craig Ringer, systemtap; +Cc: Frank Ch. Eigler

If I use --dyninst (which creates a probe source that is easy to modify 
and rebuild to try quick experiments) then this change forces the probe 
point name:
  #define STP_NEED_PROBE_NAME=1
  changed in enter_dyninst_uprobe:
  c->probe_point = sup->probe->pn; /* was ->pp */

% stapdyn *.so -c /work/scox/stap/sdt/tstclass.x
process("/work/scox/stap/sdt/tstclass.x").mark("test_probe_4") aa=0xa 
string1=abc
10 20 abc xyz
10 20 abc xyz

(where the original probe was doing: printf("%s aa=%#lx 
string1=%s\n",pp(),@cast($arg1,"struct A")->aa, ...)


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

* Re: Emitting marker names instead of hex statement addresses for SDT probes?
  2021-01-19 22:21 ` Stan Cox
@ 2021-01-21  5:47   ` Craig Ringer
  0 siblings, 0 replies; 3+ messages in thread
From: Craig Ringer @ 2021-01-21  5:47 UTC (permalink / raw)
  To: Stan Cox; +Cc: systemtap, Frank Ch. Eigler

On Wed, 20 Jan 2021 at 06:21, Stan Cox <scox@redhat.com> wrote:

> If I use --dyninst (which creates a probe source that is easy to modify
> and rebuild to try quick experiments) then this change forces the probe
> point name:
>   #define STP_NEED_PROBE_NAME=1
>   changed in enter_dyninst_uprobe:
>   c->probe_point = sup->probe->pn; /* was ->pp */
>
> % stapdyn *.so -c /work/scox/stap/sdt/tstclass.x
> process("/work/scox/stap/sdt/tstclass.x").mark("test_probe_4") aa=0xa
> string1=abc
> 10 20 abc xyz
> 10 20 abc xyz
>
> (where the original probe was doing: printf("%s aa=%#lx
> string1=%s\n",pp(),@cast($arg1,"struct A")->aa, ...)
>

Thanks so much for having a go!

I've since gone in circles in the C++ stap code trying to work out where
the generated probe name comes from. I can confirm that c->probe_name
contains what I want pp() to emit, and c->probe_point contains the
statement.

After some diversions through the DWARF code and various other places I
landed up at

          case uprobe3_type:
            // process("executable").statement(probe_arg)
            derived_comps.push_back
              (new probe_point::component(TOK_STATEMENT,
                                          new literal_number(pc, true)));
            break;

since it appears that a userspace SDT "process("foo").mark("bar")" is a
uprobe3_type:

matched probe_name test_marker probe type uprobe3 at 0x40110a
probe_type == uprobe3, use statement addr: 0x40110a
...
symbol resolution for derived-probe probe
process("/home/craig/projects/2Q/systemtap/test_sdt").statement(0x40110a){
printf("probe_point is \"%s\"\\n", %{  /* string */ c->probe_point  %});
}

AFAICS the issue may be that the marker in pp_mark for
sdt_query::convert_location() could be a wildcard so a statement address
was used to disambiguate.

This is already supposed to work.

        // replace the possibly wildcarded arg with the specific marker name
        *it = new probe_point::component(TOK_MARK,
                                         new literal_string(probe_name));

but doesn't appear to affect the printed probe specific name. If I jam some
code into convert_location()


  clog << "SPECIFIC NAME: " << specific_loc->str() << endl;
  clog << "BASE PROBE: " << base_loc->str() << endl;

I see output

SPECIFIC NAME:
process("/home/craig/projects/2Q/systemtap/test_sdt").mark("test_marker")
BASE PROBE: process("test_sdt").mark("test_marker")

which is what I expect, but somehow that doesn't translate into the final
probe_name.


see test script:


probe process("test_sdt").mark("test_marker") {
        printf("probe_point is \"%s\"\n", %{ /* string */ c->probe_point %}
);
}

and source file

#include <sys/sdt.h>
int main()
{
  DTRACE_PROBE(dummy,test_marker);
  return 0;
}


Also I keep
Since someone went to the effort of implementing STP_NEED_PROBE_NAME I
expect we should care about it. The probe names should be strings in
read-only ELF sections so memory use shouldn't be a concern though, it'd
only affect binary size. If they *aren't* in rodata, that's worth fixing.
It's not clear how much we should really care about the size of these
strings in the .ko. The original commit says "Add pp1 to stap_probe,
predicated on STP_NEED_PROBE_POINT_LISTING so we don't waste space." That
was later renamed to pp() and STP_NEED_PROBE_NAME. There's no detail on why
the space matters. The relevant commits appear to be dc575eac0, d48df0cfe,
2d767770c . I didn't want to force PROBE_NAME to be defined in this case
since I don't think there's a good reason that probe_point should have the
"statement()" form when we could emit the "mark()" form consistently.


On a side note, I was looking for how to get error() to emit the current
file and line number from the tapset but drew a blank on that. I can see
that the various visitors have access to a token (tok) with location member
containing file and line info and that there's a mechanism for visitors to
prepend bits of code, so I assume it'd be possible to transform references
to "@__FILE__" and "@__LINE__" in stap code into the appropriate constants
with one of the various walker visitors, but I'm wondering if its feasible
to do so at the macro processing level to keep it simpler.

-- 
 Craig Ringer                   http://www.2ndQuadrant.com/
 2ndQuadrant - PostgreSQL Solutions for the Enterprise

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

end of thread, other threads:[~2021-01-21  5:47 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-13  4:49 Emitting marker names instead of hex statement addresses for SDT probes? Craig Ringer
2021-01-19 22:21 ` Stan Cox
2021-01-21  5:47   ` Craig Ringer

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