[ktap] Re: [PATCH] add new symbol module to search symbols in DSO (for uprobes)

  • From: Azat Khuzhin <a3at.mail@xxxxxxxxx>
  • To: ktap <ktap@xxxxxxxxxxxxx>
  • Date: Tue, 5 Nov 2013 08:47:03 +0400

On 5 Nov 2013 08:34, "Jovi Zhangwei" <jovi.zhangwei@xxxxxxxxx> wrote:
>
> Hi Azat,

Hi Jovi,

>
> On Tue, Nov 5, 2013 at 1:57 AM, Azat Khuzhin <a3at.mail@xxxxxxxxx> wrote:
> > This will allow to don't hardcoding addresses of functions in ktap
> > scripts, that use uprobes, but instead, use normal function names.
> >
> > Before this patch:
> > $ ktap -e 'trace probe:/path/to/bin:0xDEADBEAF' # instead some real
address
> >
> > After this path:
> > $ ktap -e 'trace probe:/path/to/bin:foo'
> >
> > Works also for ret probes (%return)
> > Requires libelf.
> >
> > Signed-off-by: Azat Khuzhin <a3at.mail@xxxxxxxxx>
> > ---
> Really thanks your patch, I always want to support debugging info
> handling into ktap,
> but I didn't have enough time, I appreciate you can take this task.
>
> Some comments embedded below.
>
> >  Makefile             |   5 ++-
> >  include/ktap_types.h |   2 +
> >  userspace/eventdef.c |  34 +++++++++++++++
> >  userspace/symbol.c   | 114
+++++++++++++++++++++++++++++++++++++++++++++++++++
> >  userspace/symbol.h   |  32 +++++++++++++++
> >  5 files changed, 186 insertions(+), 1 deletion(-)
> >  create mode 100644 userspace/symbol.c
> >  create mode 100644 userspace/symbol.h
> >
> > diff --git a/Makefile b/Makefile
> > index 372b41a..344c82a 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -59,6 +59,8 @@ $(UDIR)/tstring.o: $(INTP)/tstring.c $(INC)/*
> >         $(QUIET_CC)$(CC) $(DEBUGINFO_FLAG) $(KTAPC_CFLAGS) -o $@ -c $<
> >  $(UDIR)/object.o: $(INTP)/object.c $(INC)/*
> >         $(QUIET_CC)$(CC) $(DEBUGINFO_FLAG) $(KTAPC_CFLAGS) -o $@ -c $<
> > +$(UDIR)/symbol.o: $(UDIR)/symbol.c $(INC)/*
> > +       $(QUIET_CC)$(CC) $(DEBUGINFO_FLAG) $(KTAPC_CFLAGS) -o $@ -c $<
> >
> >  KTAPOBJS =
> >  KTAPOBJS += $(UDIR)/lex.o
> > @@ -73,9 +75,10 @@ KTAPOBJS += $(UDIR)/opcode.o
> >  KTAPOBJS += $(UDIR)/table.o
> >  KTAPOBJS += $(UDIR)/tstring.o
> >  KTAPOBJS += $(UDIR)/object.o
> > +KTAPOBJS += $(UDIR)/symbol.o
> >
> >  ktap: $(KTAPOBJS)
> > -       $(QUIET_LINK)$(CC) $(KTAPC_CFLAGS) -o $@ $(KTAPOBJS) -lpthread
> > +       $(QUIET_LINK)$(CC) $(KTAPC_CFLAGS) -o $@ $(KTAPOBJS) -lpthread
-lelf
> >
> This will be a problem for embedded system which don't have libelf,
> then they cannot
> use ktap in those small system just because lack libelf.
>
> So I suggest let libelf linking to be an option, like perf Makefile,
> there have NO_LIBELF
> option in perf Makefile. If user compiler ktap without libelf support,
> then they cannot use
> debugging info, but still can use direct memory address or other
> useful functionality in ktap.
>
> You can take more reference on perf Makefile in Linux kernel.

Nice suggestion.
I was thinking about it, but in first version I left it more simple.
I will add such option into makefile.

>
> >  KMISC := /lib/modules/$(KVERSION)/ktapvm/
> >
> > diff --git a/include/ktap_types.h b/include/ktap_types.h
> > index 1dae9d8..4e19f0b 100644
> > --- a/include/ktap_types.h
> > +++ b/include/ktap_types.h
> > @@ -600,5 +600,7 @@ extern ktap_global_state dummy_global_state;
> >  #define KTAP_QL(x)      "'" x "'"
> >  #define KTAP_QS         KTAP_QL("%s")
> >
> > +#define STRINGIFY(type) #type
> > +
> >  #endif /* __KTAP_TYPES_H__ */
> >
> > diff --git a/userspace/eventdef.c b/userspace/eventdef.c
> > index 76a68ac..dab964a 100644
> > --- a/userspace/eventdef.c
> > +++ b/userspace/eventdef.c
> > @@ -29,6 +29,7 @@
> >  #include "../include/ktap_types.h"
> >  #include "../include/ktap_opcodes.h"
> >  #include "ktapc.h"
> > +#include "symbol.h"
> >
> >  static char tracing_events_path[] = "/sys/kernel/debug/tracing/events";
> >
> > @@ -305,6 +306,35 @@ static int parse_events_add_kprobe(char *old_event)
> >
> >  #define UPROBE_EVENTS_PATH "/sys/kernel/debug/tracing/uprobe_events"
> >
> > +static char* parse_events_resolve_symbol(char *event, char *r)
> > +{
> > +       char *colon;
> > +       colon = strchr(event, ':');
> > +
> > +       char *binary = strndup(event, colon - event);
> > +       char *symbol = strndup(colon + 1 /* skip ":" */,
> > +               r ? (r - 1 /* skip "%" */ - colon) : strlen((const char
*)colon));
> > +
>
> I guess this would be a problem if use argument fetching in uprobe, for
example,

Right.

>
>     trace probe:/lib/libc.so:foo $arg1 $arg2 {
>         print(argevent)
>     }
>
> Basically kprobe and uprobe tracing syntax was borrowed from perf-probe,
> and should be compatible with perf-probe syntax as much as possible.
>
> BTW, perf-probe was contributed by Masami(Maintainer of kprobe in Linux
kernel).
>
> So in current stage, it's fine to not care about the argument symbol
parsing,
> just parse the function symbol is enough.
>
> Would you like to change the code a little to handle parse symbol if
argument
> appended? Thanks.

Sure, I will make parsing more common. Thanks!

>
> More info could take reference by perf-probe command option.
>
> Jovi.
>

Other related posts: