| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20150427050818.GC6113@naverao1-tp.in.ibm.com>
Date: Mon, 27 Apr 2015 10:38:18 +0530
From: "Naveen N. Rao" <naveen.n.rao@...ux.vnet.ibm.com>
To: Arnaldo Carvalho de Melo <acme@...nel.org>
Cc: linuxppc-dev@...ts.ozlabs.org, linux-kernel@...r.kernel.org,
mpe@...erman.id.au, ananth@...ibm.com
Subject: Re: [PATCHv2 5/8] perf probe powerpc: Allow matching against dot
symbols
On 2015/03/12 05:30PM, Arnaldo Carvalho de Melo wrote:
> Em Mon, Dec 15, 2014 at 08:20:35PM +0530, Naveen N. Rao escreveu:
> > Allow perf probe to work on powerpc ABIv1 without the need to specify
> > the leading dot '.' for functions. 'perf probe do_fork' works with this
> > patch.
> >
> > Introduce HAVE_ARCH_SYMBOL_HANDLING to indicate need for special
> > handling of symbols. In this patch, we override probe_function_filter()
> > on powerpc to account for dot symbols.
>
> This one looks better, does arch specific stuff in tools/perf/arch,
> good, some nits below.
>
> > Signed-off-by: Naveen N. Rao <naveen.n.rao@...ux.vnet.ibm.com>
> > ---
> > Changes from the previous patchset:
> > Introduced arch helper to override the way probe function filter works.
> >
> > tools/perf/arch/powerpc/Makefile | 1 +
> > tools/perf/arch/powerpc/util/sym-handling.c | 28 ++++++++++++++++++++++++++++
> > tools/perf/config/Makefile | 1 +
> > tools/perf/util/probe-event.c | 10 +++++-----
> > tools/perf/util/probe-event.h | 5 +++++
> > 5 files changed, 40 insertions(+), 5 deletions(-)
> > create mode 100644 tools/perf/arch/powerpc/util/sym-handling.c
> >
> > diff --git a/tools/perf/arch/powerpc/Makefile b/tools/perf/arch/powerpc/Makefile
> > index 6f7782b..1c3d435 100644
> > --- a/tools/perf/arch/powerpc/Makefile
> > +++ b/tools/perf/arch/powerpc/Makefile
> > @@ -3,4 +3,5 @@ PERF_HAVE_DWARF_REGS := 1
> > LIB_OBJS += $(OUTPUT)arch/$(ARCH)/util/dwarf-regs.o
> > LIB_OBJS += $(OUTPUT)arch/$(ARCH)/util/skip-callchain-idx.o
> > endif
> > +LIB_OBJS += $(OUTPUT)arch/$(ARCH)/util/sym-handling.o
> > LIB_OBJS += $(OUTPUT)arch/$(ARCH)/util/header.o
> > diff --git a/tools/perf/arch/powerpc/util/sym-handling.c b/tools/perf/arch/powerpc/util/sym-handling.c
> > new file mode 100644
> > index 0000000..0a77825
> > --- /dev/null
> > +++ b/tools/perf/arch/powerpc/util/sym-handling.c
> > @@ -0,0 +1,28 @@
> > +/*
> > + * Special symbol handling for PowerPC:
> > + * - Handle dot symbols on ABIv1
> > + *
> > + * Copyright (C) 2014 Naveen N Rao, IBM Corporation.
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public License
> > + * as published by the Free Software Foundation; either version
> > + * 2 of the License, or (at your option) any later version.
> > + */
> > +
> > +#include "map.h"
> > +#include "symbol.h"
> > +#include "probe-event.h"
> > +
> > +int probe_function_filter(struct map *map __maybe_unused, struct symbol *sym)
> > +{
> > + if (sym->binding == STB_GLOBAL || sym->binding == STB_LOCAL) {
> > + if ((strcmp(looking_function_name, sym->name) == 0) ||
> > + (sym->name[0] == '.' && looking_function_name[0] != '.' &&
> > + strcmp(looking_function_name, sym->name+1) == 0)) {
> > + num_matched_functions++;
> > + return 0;
> > + }
> > + }
> > + return 1;
> > +}
> > diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
> > index 5d4b039..35cf934 100644
> > --- a/tools/perf/config/Makefile
> > +++ b/tools/perf/config/Makefile
> > @@ -383,6 +383,7 @@ ifeq ($(ARCH),powerpc)
> > ifndef NO_DWARF
> > CFLAGS += -DHAVE_SKIP_CALLCHAIN_IDX
> > endif
> > + CFLAGS += -DHAVE_ARCH_SYMBOL_HANDLING
>
>
> Dunno about this naming, looks too general: SYMBOL_HANDLING, but can't
> come to some better one now, anyone?
>
> > endif
> >
> > ifndef NO_LIBUNWIND
> > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> > index 74b7fef..7eb9b27 100644
> > --- a/tools/perf/util/probe-event.c
> > +++ b/tools/perf/util/probe-event.c
> > @@ -50,6 +50,8 @@
> > #define PERFPROBE_GROUP "probe"
> >
> > bool probe_event_dry_run; /* Dry run flag */
> > +char *looking_function_name;
> > +int num_matched_functions;
> >
> > #define semantic_error(msg ...) pr_err("Semantic error :" msg)
> >
> > @@ -2210,11 +2212,8 @@ static int __add_probe_trace_events(struct perf_probe_event *pev,
> > return ret;
> > }
> >
> > -static char *looking_function_name;
> > -static int num_matched_functions;
> > -
> > -static int probe_function_filter(struct map *map __maybe_unused,
> > - struct symbol *sym)
> > +#ifndef HAVE_ARCH_SYMBOL_HANDLING
> > +int probe_function_filter(struct map *map __maybe_unused, struct symbol *sym)
> > {
> > if ((sym->binding == STB_GLOBAL || sym->binding == STB_LOCAL) &&
> > strcmp(looking_function_name, sym->name) == 0) {
> > @@ -2223,6 +2222,7 @@ static int probe_function_filter(struct map *map __maybe_unused,
> > }
> > return 1;
> > }
> > +#endif /* HAVE_ARCH_SYMBOL_HANDLING */
>
> Can't we do something like providing a weak function and let the linked
> to its work? I guess we have cases like this in tools/ already. I.e. not
> using the ifndef block. Minor nit tho.
That sounds like a good idea. I will move these over to use __weak
functions.
>
> >
> > #define strdup_or_goto(str, label) \
> > ({ char *__p = strdup(str); if (!__p) goto label; __p; })
> > diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
> > index e01e994..8564451 100644
> > --- a/tools/perf/util/probe-event.h
> > +++ b/tools/perf/util/probe-event.h
> > @@ -7,6 +7,8 @@
> > #include "strfilter.h"
> >
> > extern bool probe_event_dry_run;
> > +extern char *looking_function_name;
> > +extern int num_matched_functions;
>
> >
> > /* kprobe-tracer and uprobe-tracer tracing point */
> > struct probe_trace_point {
> > @@ -136,6 +138,9 @@ extern int show_available_vars(struct perf_probe_event *pevs, int npevs,
> > extern int show_available_funcs(const char *module, struct strfilter *filter,
> > bool user);
> >
> > +extern int probe_function_filter(struct map *map __maybe_unused,
> > + struct symbol *sym);
> > +
>
> Please do not prefix function declarations with 'extern', even when we
> have one just before, its not needed, patches removing the existing ones
> would be accepted.
Sure.
- Naveen
>
> > /* Maximum index number of event-name postfix */
> > #define MAX_EVENT_INDEX 1024
> >
> > --
> > 2.1.3
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists