| 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: <50399556C9727B4D88A595C8584AAB3752627CD4@GSjpTKYDCembx32.service.hitachi.net>
Date: Fri, 20 Nov 2015 10:54:18 +0000
From: 平松雅巳 / HIRAMATU，MASAMI
<masami.hiramatsu.pt@...achi.com>
To: "'Wang Nan'" <wangnan0@...wei.com>,
"acme@...nel.org" <acme@...nel.org>
CC: "lizefan@...wei.com" <lizefan@...wei.com>,
"pi3orama@....com" <pi3orama@....com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Alexei Starovoitov <ast@...nel.org>,
"Arnaldo Carvalho de Melo" <acme@...hat.com>,
Namhyung Kim <namhyung@...nel.org>
Subject: RE: [PATCH 1/2] perf tools: Always give options even it not compiled
>From: Wang Nan [mailto:wangnan0@...wei.com]
>
>This patch keeps options of perf builtins same in all condition. If the
>option is disabled because of compiling options, users should be
>notified.
>
>This patch does it by introducing a series of new option macros, flags
>and field in struct options. For those options disabled by compiling,
>OPT_NOTBUILT_NOARG, OPT_NOTBUILT_OPTARG and OPT_NOTBUILT can be used
>to record the help messages and the reason why not built them.
>
>Options in 'perf record' and 'perf probe' are fixed by those new macros.
Hmm, OK, I agree the reason why this is useful. Could you reconsider
the implementation, because just cloning the code is ugly and not
maintainable?
It will be better if we can replace OPT_BOOLEAN with;
OPT_DEPENDS(HAVE_DWARF_SUPPORT, BOOLEAN, '\0', "no-inlines", &probe_conf.no_inlines, ...
This may be done by following macros ;
----
#define OPTMSG_HAVE_DWARF_SUPPORT "NO_DWARF"
#ifdef HAVE_DWARF_SUPPORT
#define OPTVAL_HAVE_DWARF_SUPPORT 1
#else
#define OPTVAL_HAVE_DWARF_SUPPORT 0
#endif
#define __OPT_DEPENDS(val, msg, opt, ...) \
{.type = OPTION_NEXT_DEPENDS, .value = (void *)val, .data = msg, }, opt(__VA_ARGS__)
#define _OPT_DEPENDS(val, msg, opt, ...) \
__OPT_DEPENDS(val, msg, opt, __VA_ARGS__)
#define OPT_DEPENDS(flag, opt, ...) \
_OPT_DEPENDS(OPTVAL_ ## flag, OPTMSG_ ## flag, OPT_ ## opt, __VA_ARGS__)
----
And if the parser find OPTION_NEXT_DEPENDS and .value != NULL, it skips next entry.
Thank you,
>
>Test result:
>
>- Normal result:
> # ./perf probe -L sys_write
> <SyS_write@...th/to/kernel/fs/read_write.c:0>
> 0 SYSCALL_DEFINE3(write, unsigned int, fd, const char __user *, buf,
> ...
>
>- Build with NO_DWARF=1:
>
> # ./perf probe -L sys_write
> ERROR: option '--line' is not built (disabled by NO_DWARF)
>
> Usage: perf probe [<options>] 'PROBEDEF' ['PROBEDEF' ...]
> or: perf probe [<options>] --add 'PROBEDEF' [--add 'PROBEDEF' ...]
> or: perf probe [<options>] --del '[GROUP:]EVENT' ...
> or: perf probe --list [GROUP:]EVENT ...
> or: perf probe [<options>] --funcs
>
> -L, --line <FUNC[:RLN[+NUM|-RLN2]]|SRC:ALN[+NUM|-ALN2]>
> Show source code lines.
> (disabled by NO_DWARF)
>
> # ./perf probe -k /tmp/vmlinux sys_write
> WARNING: option '--vmlinux' is not built (disabled by NO_DWARF)
> Added new event:
> probe:sys_write (on sys_write)
>
> You can now use it in all perf tools, such as:
>
> perf record -e probe:sys_write -aR sleep 1
>
> # ./perf probe -k sys_write
>
> Usage: perf probe [<options>] 'PROBEDEF' ['PROBEDEF' ...]
> or: perf probe [<options>] --add 'PROBEDEF' [--add 'PROBEDEF' ...]
> ...
>
>- Help messages:
>
> # ./perf probe -h
>
> Usage: perf probe [<options>] 'PROBEDEF' ['PROBEDEF' ...]
> or: perf probe [<options>] --add 'PROBEDEF' [--add 'PROBEDEF' ...]
> ...
> -k, --vmlinux <file> vmlinux pathname
> (disabled by NO_DWARF)
> -L, --line <FUNC[:RLN[+NUM|-RLN2]]|SRC:ALN[+NUM|-ALN2]>
> Show source code lines.
> (disabled by NO_DWARF)
> -l, --list <[GROUP:]EVENT>
> list up probe events
> ...
> -s, --source <directory>
> path to kernel source
> (disabled by NO_DWARF)
> -V, --vars <FUNC[@SRC][+OFF|%return|:RL|;PT]|SRC:AL|SRC;PT>
> Show accessible variables on PROBEDEF
> (disabled by NO_DWARF)
> ...
> --no-inlines Don't search inlined functions
> (disabled by NO_DWARF)
> --range Show variables location range in scope (with --vars only)
> (disabled by NO_DWARF)
>
>Signed-off-by: Wang Nan <wangnan0@...wei.com>
>Cc: Alexei Starovoitov <ast@...nel.org>
>Cc: Arnaldo Carvalho de Melo <acme@...hat.com>
>Cc: Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>
>Cc: Namhyung Kim <namhyung@...nel.org>
>Cc: Zefan Li <lizefan@...wei.com>
>Cc: pi3orama@....com
>---
> tools/perf/builtin-probe.c | 23 +++++++++++++++++++++++
> tools/perf/builtin-record.c | 7 +++++++
> tools/perf/util/parse-options.c | 30 +++++++++++++++++++++++++++++-
> tools/perf/util/parse-options.h | 16 ++++++++++++++++
> 4 files changed, 75 insertions(+), 1 deletion(-)
>
>diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
>index 132afc9..4d7cf57 100644
>--- a/tools/perf/builtin-probe.c
>+++ b/tools/perf/builtin-probe.c
>@@ -490,6 +490,29 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
> "directory", "path to kernel source"),
> OPT_BOOLEAN('\0', "no-inlines", &probe_conf.no_inlines,
> "Don't search inlined functions"),
>+#else
>+#define DISABLE_MSG "disabled by NO_DWARF"
>+ OPT_NOTBUILT('L', "line",
>+ "FUNC[:RLN[+NUM|-RLN2]]|SRC:ALN[+NUM|-ALN2]",
>+ "Show source code lines.",
>+ DISABLE_MSG, false),
>+ OPT_NOTBUILT('V', "vars",
>+ "FUNC[@SRC][+OFF|%return|:RL|;PT]|SRC:AL|SRC;PT",
>+ "Show accessible variables on PROBEDEF",
>+ DISABLE_MSG, false),
>+ OPT_NOTBUILT_NOARG('\0', "externs",
>+ "Show external variables too (with --vars only)",
>+ DISABLE_MSG, false),
>+ OPT_NOTBUILT_NOARG('\0', "range",
>+ "Show variables location range in scope (with --vars only)",
>+ DISABLE_MSG, false),
>+ OPT_NOTBUILT('k', "vmlinux", "file", "vmlinux pathname",
>+ DISABLE_MSG, true),
>+ OPT_NOTBUILT('s', "source", "directory", "path to kernel source",
>+ DISABLE_MSG, true),
>+ OPT_NOTBUILT_NOARG('\0', "no-inlines", "Don't search inlined functions",
>+ DISABLE_MSG, true),
>+#undef DISABLE_MSG
> #endif
> OPT__DRY_RUN(&probe_event_dry_run),
> OPT_INTEGER('\0', "max-probes", &probe_conf.max_probes,
>diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
>index 199fc31..b02d1ee 100644
>--- a/tools/perf/builtin-record.c
>+++ b/tools/perf/builtin-record.c
>@@ -1118,6 +1118,13 @@ struct option __record_options[] = {
> "clang binary to use for compiling BPF scriptlets"),
> OPT_STRING(0, "clang-opt", &llvm_param.clang_opt, "clang options",
> "options passed to clang when compiling BPF scriptlets"),
>+#else
>+ OPT_NOTBUILT(0, "clang-path", "clang path",
>+ "clang binary to use for compiling BPF scriptlets",
>+ "disabled by NO_LIBBPF", true),
>+ OPT_NOTBUILT(0, "clang-opt", "clang options",
>+ "options passed to clang when compiling BPF scriptlets",
>+ "disabled by NO_LIBBPF", true),
> #endif
> OPT_END()
> };
>diff --git a/tools/perf/util/parse-options.c b/tools/perf/util/parse-options.c
>index 9fca092..5c3eb7a 100644
>--- a/tools/perf/util/parse-options.c
>+++ b/tools/perf/util/parse-options.c
>@@ -1,4 +1,5 @@
> #include "util.h"
>+#include "debug.h"
> #include "parse-options.h"
> #include "cache.h"
> #include "header.h"
>@@ -69,6 +70,7 @@ static int get_value(struct parse_opt_ctx_t *p,
> if (!(flags & OPT_SHORT) && p->opt) {
> switch (opt->type) {
> case OPTION_CALLBACK:
>+ case OPTION_NOTBUILT:
> if (!(opt->flags & PARSE_OPT_NOARG))
> break;
> /* FALLTHROUGH */
>@@ -152,7 +154,28 @@ static int get_value(struct parse_opt_ctx_t *p,
> if (get_arg(p, opt, flags, &arg))
> return -1;
> return (*opt->callback)(opt, arg, 0) ? (-1) : 0;
>-
>+ case OPTION_NOTBUILT: {
>+ char short_str[2] = {opt->short_name, '\0'};
>+ int ret = -1;
>+
>+ pr_warning("%s: option '-%s%s' is not built (%s)\n",
>+ opt->flags & PARSE_OPT_CANSKIP ?
>+ "WARNING" : "ERROR",
>+ opt->long_name ? "-" : short_str,
>+ opt->long_name ? : "",
>+ opt->notbuild_reason);
>+ if (opt->flags & PARSE_OPT_CANSKIP)
>+ ret = 0;
>+ if (unset)
>+ return ret;
>+ if (opt->flags & PARSE_OPT_NOARG)
>+ return ret;
>+ if (opt->flags & PARSE_OPT_OPTARG && !p->opt)
>+ return ret;
>+ if (get_arg(p, opt, flags, &arg))
>+ return -1;
>+ return ret;
>+ }
> case OPTION_INTEGER:
> if (unset) {
> *(int *)opt->value = 0;
>@@ -607,6 +630,7 @@ static void print_option_help(const struct option *opts, int full)
> pos += fprintf(stderr, " <n>");
> break;
> case OPTION_CALLBACK:
>+ case OPTION_NOTBUILT:
> if (opts->flags & PARSE_OPT_NOARG)
> break;
> /* FALLTHROUGH */
>@@ -647,6 +671,10 @@ static void print_option_help(const struct option *opts, int full)
> pad = USAGE_OPTS_WIDTH;
> }
> fprintf(stderr, "%*s%s\n", pad + USAGE_GAP, "", opts->help);
>+ if (opts->type == OPTION_NOTBUILT)
>+ fprintf(stderr, "%*s(%s)\n",
>+ USAGE_OPTS_WIDTH + USAGE_GAP, "",
>+ opts->notbuild_reason);
> }
>
> static int option__cmp(const void *va, const void *vb)
>diff --git a/tools/perf/util/parse-options.h b/tools/perf/util/parse-options.h
>index a8e407b..08cc9b5 100644
>--- a/tools/perf/util/parse-options.h
>+++ b/tools/perf/util/parse-options.h
>@@ -22,6 +22,7 @@ enum parse_opt_type {
> OPTION_CALLBACK,
> OPTION_U64,
> OPTION_UINTEGER,
>+ OPTION_NOTBUILT,
> };
>
> enum parse_opt_flags {
>@@ -41,6 +42,7 @@ enum parse_opt_option_flags {
> PARSE_OPT_DISABLED = 32,
> PARSE_OPT_EXCLUSIVE = 64,
> PARSE_OPT_NOEMPTY = 128,
>+ PARSE_OPT_CANSKIP = 256,
> };
>
> struct option;
>@@ -96,6 +98,7 @@ struct option {
> void *value;
> const char *argh;
> const char *help;
>+ const char *notbuild_reason;
>
> int flags;
> parse_opt_cb *callback;
>@@ -146,6 +149,19 @@ struct option {
> .value = (v), (a), .help = (h), .callback = (f), \
> .flags = PARSE_OPT_OPTARG, .data = (d) }
>
>+#define OPT_NOTBUILT_NOARG(s, l, h, r, skip) \
>+ { .type = OPTION_NOTBUILT, .short_name = (s), .long_name = (l), \
>+ .help = (h), .notbuild_reason = (r), \
>+ .flags = PARSE_OPT_NOARG | (skip ? PARSE_OPT_CANSKIP : 0)}
>+#define OPT_NOTBUILT_OPTARG(s, l, a, h, r, skip) \
>+ { .type = OPTION_NOTBUILT, .short_name = (s), .long_name = (l), \
>+ .argh = (a), .help = (h), .notbuild_reason = (r), \
>+ .flags = PARSE_OPT_OPTARG | (skip ? PARSE_OPT_CANSKIP : 0)}
>+#define OPT_NOTBUILT(s, l, a, h, r, skip) \
>+ { .type = OPTION_NOTBUILT, .short_name = (s), .long_name = (l), \
>+ .argh = (a), .help = (h), .notbuild_reason = (r), \
>+ .flags = skip ? PARSE_OPT_CANSKIP : 0}
>+
> /* parse_options() will filter out the processed options and leave the
> * non-option argments in argv[].
> * Returns the number of arguments left in argv[].
>--
>1.8.3.4
Powered by blists - more mailing lists