This is the mail archive of the
systemtap@sourceware.org
mailing list for the systemtap project.
Re: Accessing user-space global variables in timer.profile?
- From: Josh Stone <jistone at redhat dot com>
- To: agentzh <agentzh at gmail dot com>
- Cc: David Smith <dsmith at redhat dot com>, systemtap at sourceware dot org
- Date: Mon, 03 Jun 2013 15:41:30 -0700
- Subject: Re: Accessing user-space global variables in timer.profile?
- References: <CAB4Tn6OYi=Vx19H4BLojphuuvjSp2drjvJFd3HxzAGj7=yD5cA at mail dot gmail dot com> <51755D3D dot 6020004 at redhat dot com> <CAB4Tn6OubWF71F9b3hNqXo85h96sr1Rp77-NmZAjmLPt=vAzyA at mail dot gmail dot com> <5175E28B dot 9050500 at redhat dot com> <CAB4Tn6ORHjkv+CuAN90Ju+gKEohSyNvpdvoq-fEsEEdUgAzjgg at mail dot gmail dot com>
On 05/29/2013 09:00 PM, agentzh wrote:
> Hello!
>
> On Mon, Apr 22, 2013 at 6:23 PM, Josh Stone wrote:
>> So we'll need a @var which at least hints which module holds it, as
>> described in http://sourceware.org/bugzilla/show_bug.cgi?id=11096#c5
>>
>> That's talking about functions, but timer.profile is similarly context-free.
>>
>
> Okay, I've come up with a (naive) patch to implement
> @var("somevar@somefile", "somemodule") for stap user functions and
> context-free probe handlers (see below). It works for me on Fedora 17,
> both in a function or in timer.profile.
>
> There's some minor things that I'm not very sure though:
>
> 1. It may not be a good idea to expand such @var in
> dwarf_cast_expanding_visitor. Maybe I should add another ad-hoc
> visitor like dwarf_var_early_expanding_visitor?
Yeah, it will be small, just as dwarf_cast_expanding_visitor is small,
but let's keep them separate. The session::code_filters is made for
this, even though @cast is the only one there so far.
I'm not sure what's "early" about this though. I do see we have an
unfortunate collision against existing dwarf_var_expanding_visitor --
maybe call this as atvar, e.g. dwarf_atvar_expanding_visitor and
dwarf_atvar_query. (FWIW even I don't think "atvar" is a *great* name,
but "var" is just too general.)
> 2. There's some code duplication between this early @var expansion and
> the existing dwarf_var_expanding_visitor. Maybe we could merge them
> two or just abstract out some common logic?
I think that could be helped a little by deferring the current @var
expansion into your new visitor. This would be much like how the
dwarf_var_expanding_visitor::visit_cast_op() only sets the module name,
so dwarf_cast_expanding_visitor has the right context later.
> Because I'm very new to the systemtap code base, I'd highly appreciate
> any suggestions or other comments!
On the note of deferring as @cast does, I also think that @var should
really be its own target_symbol subclass, rather than being a weird
special-case flavor of target_symbol as it is now. I know this is not
your invention, but "target_name", "cu_name", and now "module" are all
specific to @var needs. It's a bigger change to make a new atvar_op, as
many tree visitors will need to be expanded, but I think it may be
cleaner overall.
...
> +void
> +dwarf_var_query::getscopes(target_symbol *e)
I can see this is copied from dwarf_var_expanding_visitor::getcuscope.
If we let dwarf_var_expanding_visitor defer @var processing until your
visitor, then yours becomes the only copy needed. (Or equivalently,
just rename the existing function into your visitor class
...
> +void
> +dwarf_cast_expanding_visitor::visit_target_symbol(target_symbol* e)
> +{
I see this similar to dwarf_cast_expanding_visitor::visit_cast_op. So
first it could be dwarf_atvar_expanding_visitor::visit_atvar_op as I
said. The duplication here is not too bad, but a few tweaks:
> + //cerr << "accessing target_symbol " << e->sym_name() << endl;
> +
> + if (is_active_lvalue(e)
> + || e->name != "@var"
> + || e->target_name.empty()
> + || e->module.empty()
> + || (!e->components.empty()
> + && e->components.back().type == target_symbol::comp_pretty_print))
> + {
> + provide(e);
> + return;
> + }
I'm not sure why you're skipping lvalues - does the existing @var
prevent writes? And why punt on pretty_print?
Also, module.empty() might as well default to "kernel", the same way
that cast_op does.
> +
> + functioncall* result = NULL;
> +
> + // split the module string by ':' for alternatives
> + vector<string> modules;
> + tokenize(e->module, modules, ":");
> + bool userspace_p = false;
> + for (unsigned i = 0; !result && i < modules.size(); ++i)
> + {
> + string& module = modules[i];
> + filter_special_modules(module);
The "special" modules don't apply here. This is where @cast actually
compiles new modules from headers to get type debuginfo. That doesn't
make sense for @var, as we'd never ever see that module loaded to get
access to any of its variables.