Re: Emitting marker names instead of hex statement addresses for SDT probes?

[Craig Ringer <craig@2ndquadrant.com>]

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