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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAM9d7cjAHdZ3-NbZQgR5i9NmiBW3U5pY6crtvb7RQWG5QvmHqg@mail.gmail.com>
Date:   Wed, 6 Jul 2022 14:27:08 -0700
From:   Namhyung Kim <namhyung@...nel.org>
To:     Raul Silvera <rsilvera@...gle.com>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Jiri Olsa <jolsa@...nel.org>,
        linux-perf-users <linux-perf-users@...r.kernel.org>,
        linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] perf inject: Add a command line option to specify build ids.

Hi Raul,

On Wed, Jun 29, 2022 at 5:18 PM Raul Silvera <rsilvera@...gle.com> wrote:
>
> This commit adds the option --known-build-ids to perf inject.
> It allows the user to explicitly specify the build id for a given
> path, instead of retrieving it from the current system. This is
> useful in cases where a perf.data file is processed on a different
> system from where it was collected, or if some of the binaries are
> no longer available.
>
> The build ids and paths are specified in pairs in the command line.
> Using the file:// specifier, build ids can be loaded from a file
> directly generated by perf buildid-list. This is convenient to copy
> build ids from one perf.data file to another.
>
> ** Example: In this example we use perf record to create two
> perf.data files, one with build ids and another without, and use
> perf buildid-list and perf inject to copy the build ids from the
> first file to the second.
>
> $ perf record ls /tmp                  # Create perf.data file
> $ perf record --no-buildid ls /tmp -o perf.data.no-buildid
> $ perf buildid-list > /tmp/build-ids.txt
> $ perf inject -b --known-build-ids='file:///tmp/build-ids.txt' \
> $  -i perf.data.no-buildid -o perf.data.buildid

You'd better indent the block for the readability and to prevent
possible interference from the shell (for comments usually).

The last line is a continuation from the above line and it used
to print other prompts like ">".

>
> Signed-off-by: Raul Silvera <rsilvera@...gle.com>
> ---
>  tools/perf/builtin-inject.c | 57 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 57 insertions(+)
>
> diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
> index a75bf11585b5..667b942f870e 100644
> --- a/tools/perf/builtin-inject.c
> +++ b/tools/perf/builtin-inject.c
> @@ -21,6 +21,7 @@
>  #include "util/data.h"
>  #include "util/auxtrace.h"
>  #include "util/jit.h"
> +#include "util/string2.h"
>  #include "util/symbol.h"
>  #include "util/synthetic-events.h"
>  #include "util/thread.h"
> @@ -35,6 +36,7 @@
>
>  #include <linux/list.h>
>  #include <linux/string.h>
> +#include <ctype.h>
>  #include <errno.h>
>  #include <signal.h>
>
> @@ -59,6 +61,8 @@ struct perf_inject {
>         struct itrace_synth_opts itrace_synth_opts;
>         char                    event_copy[PERF_SAMPLE_MAX_SIZE];
>         struct perf_file_section secs[HEADER_FEAT_BITS];
> +       const char              *known_build_ids_source;
> +       struct strlist          *known_build_ids;
>  };
>
>  struct event_entry {
> @@ -570,9 +574,43 @@ static int dso__read_build_id(struct dso *dso)
>         return dso->has_build_id ? 0 : -1;
>  }
>
> +static bool perf_inject__lookup_known_build_id(struct perf_inject *inject,
> +                                              struct dso *dso)
> +{
> +       struct str_node *pos;
> +       int bid_len;
> +
> +       strlist__for_each_entry(pos, inject->known_build_ids) {
> +               const char *space;
> +
> +               pos->s = skip_spaces(pos->s);
> +               space = strstr(pos->s, " ");
> +               if (space == NULL ||
> +                   !strcmp(dso->long_name, skip_spaces(space)))
> +                       continue;
> +               bid_len = space - pos->s;
> +               if (bid_len == 0 || bid_len / 2 > BUILD_ID_SIZE)

I'm not sure how bid_len can be 0.  And you can compare to
SBUILD_ID_SIZE (without dividing by 2) - but it'd be ">=".
Also it could be worthwhile to check if bid_len is even.


> +                       return false;
> +               for (int ix = 0; 2 * ix + 1 < bid_len; ++ix) {
> +                       if (!isxdigit(pos->s[2 * ix]) ||
> +                           !isxdigit(pos->s[2 * ix + 1]))
> +                               return false;
> +
> +                       dso->bid.data[ix] = (hex(pos->s[2 * ix]) << 4 |
> +                                            hex(pos->s[2 * ix + 1]));
> +               }
> +               dso->bid.size = bid_len / 2;
> +               dso->has_build_id = 1;
> +               return true;
> +       }
> +       return false;
> +}
> +
>  static int dso__inject_build_id(struct dso *dso, struct perf_tool *tool,
>                                 struct machine *machine, u8 cpumode, u32 flags)
>  {
> +       struct perf_inject *inject = container_of(tool, struct perf_inject,
> +                                                 tool);
>         int err;
>
>         if (is_anon_memory(dso->long_name) || flags & MAP_HUGETLB)
> @@ -580,6 +618,10 @@ static int dso__inject_build_id(struct dso *dso, struct perf_tool *tool,
>         if (is_no_dso_memory(dso->long_name))
>                 return 0;
>
> +       if (inject->known_build_ids != NULL &&
> +           perf_inject__lookup_known_build_id(inject, dso))
> +               return 1;
> +
>         if (dso__read_build_id(dso) < 0) {
>                 pr_debug("no build_id found for %s\n", dso->long_name);
>                 return -1;
> @@ -1082,6 +1124,9 @@ int cmd_inject(int argc, const char **argv)
>                             "Inject build-ids into the output stream"),
>                 OPT_BOOLEAN(0, "buildid-all", &inject.build_id_all,
>                             "Inject build-ids of all DSOs into the output stream"),
> +               OPT_STRING(0, "known-build-ids", &inject.known_build_ids_source,
> +                          "buildid path [buildid path...]",
> +                          "build-ids to use for specific files"),
>                 OPT_STRING('i', "input", &inject.input_name, "file",
>                            "input file name"),
>                 OPT_STRING('o', "output", &inject.output.path, "file",
> @@ -1215,6 +1260,18 @@ int cmd_inject(int argc, const char **argv)
>                  */
>                 inject.tool.ordered_events = true;
>                 inject.tool.ordering_requires_timestamps = true;
> +               if (inject.known_build_ids_source != NULL) {
> +                       struct strlist *known_build_ids;
> +
> +                       known_build_ids = strlist__new(
> +                           inject.known_build_ids_source, NULL);
> +
> +                       if (known_build_ids == NULL) {
> +                               pr_err("Couldn't parse known build ids.\n");
> +                               goto out_delete;
> +                       }
> +                       inject.known_build_ids = known_build_ids;

You need to release it after use.  Note that strlist__delete()
accepts a NULL pointer.

Thanks,
Namhyung


> +               }
>         }
>
>         if (inject.sched_stat) {
> --
> 2.37.0.rc0.161.g10f37bed90-goog
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ