* [PATCH] perf test: Fix msan uninitialized use.
@ 2020-09-23 21:06 Ian Rogers
2020-09-23 23:37 ` Nick Desaulniers
0 siblings, 1 reply; 4+ messages in thread
From: Ian Rogers @ 2020-09-23 21:06 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
linux-kernel, clang-built-linux
Cc: Stephane Eranian, Ian Rogers
Ensure 'st' is initialized before an error branch is taken.
Fixes test "67: Parse and process metrics" with LLVM msan:
==6757==WARNING: MemorySanitizer: use-of-uninitialized-value
#0 0x5570edae947d in rblist__exit tools/perf/util/rblist.c:114:2
#1 0x5570edb1c6e8 in runtime_stat__exit tools/perf/util/stat-shadow.c:141:2
#2 0x5570ed92cfae in __compute_metric tools/perf/tests/parse-metric.c:187:2
#3 0x5570ed92cb74 in compute_metric tools/perf/tests/parse-metric.c:196:9
#4 0x5570ed92c6d8 in test_recursion_fail tools/perf/tests/parse-metric.c:318:2
#5 0x5570ed92b8c8 in test__parse_metric tools/perf/tests/parse-metric.c:356:2
#6 0x5570ed8de8c1 in run_test tools/perf/tests/builtin-test.c:410:9
#7 0x5570ed8ddadf in test_and_print tools/perf/tests/builtin-test.c:440:9
#8 0x5570ed8dca04 in __cmd_test tools/perf/tests/builtin-test.c:661:4
#9 0x5570ed8dbc07 in cmd_test tools/perf/tests/builtin-test.c:807:9
#10 0x5570ed7326cc in run_builtin tools/perf/perf.c:313:11
#11 0x5570ed731639 in handle_internal_command tools/perf/perf.c:365:8
#12 0x5570ed7323cd in run_argv tools/perf/perf.c:409:2
#13 0x5570ed731076 in main tools/perf/perf.c:539:3
Fixes: commit f5a56570a3f2 ("perf test: Fix memory leaks in parse-metric test")
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/tests/parse-metric.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/perf/tests/parse-metric.c b/tools/perf/tests/parse-metric.c
index aea4f970fccc..7c1bde01cb50 100644
--- a/tools/perf/tests/parse-metric.c
+++ b/tools/perf/tests/parse-metric.c
@@ -157,6 +157,7 @@ static int __compute_metric(const char *name, struct value *vals,
}
perf_evlist__set_maps(&evlist->core, cpus, NULL);
+ runtime_stat__init(&st);
/* Parse the metric into metric_events list. */
err = metricgroup__parse_groups_test(evlist, &map, name,
@@ -170,7 +171,6 @@ static int __compute_metric(const char *name, struct value *vals,
goto out;
/* Load the runtime stats with given numbers for events. */
- runtime_stat__init(&st);
load_runtime_stat(&st, evlist, vals);
/* And execute the metric */
--
2.28.0.681.g6f77f65b4e-goog
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] perf test: Fix msan uninitialized use.
2020-09-23 21:06 [PATCH] perf test: Fix msan uninitialized use Ian Rogers
@ 2020-09-23 23:37 ` Nick Desaulniers
2020-09-24 23:12 ` Ian Rogers
2020-09-28 12:24 ` Arnaldo Carvalho de Melo
0 siblings, 2 replies; 4+ messages in thread
From: Nick Desaulniers @ 2020-09-23 23:37 UTC (permalink / raw)
To: Ian Rogers
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, LKML,
clang-built-linux, Stephane Eranian
On Wed, Sep 23, 2020 at 2:07 PM 'Ian Rogers' via Clang Built Linux
<clang-built-linux@googlegroups.com> wrote:
>
> Ensure 'st' is initialized before an error branch is taken.
> Fixes test "67: Parse and process metrics" with LLVM msan:
> ==6757==WARNING: MemorySanitizer: use-of-uninitialized-value
> #0 0x5570edae947d in rblist__exit tools/perf/util/rblist.c:114:2
> #1 0x5570edb1c6e8 in runtime_stat__exit tools/perf/util/stat-shadow.c:141:2
> #2 0x5570ed92cfae in __compute_metric tools/perf/tests/parse-metric.c:187:2
> #3 0x5570ed92cb74 in compute_metric tools/perf/tests/parse-metric.c:196:9
> #4 0x5570ed92c6d8 in test_recursion_fail tools/perf/tests/parse-metric.c:318:2
> #5 0x5570ed92b8c8 in test__parse_metric tools/perf/tests/parse-metric.c:356:2
> #6 0x5570ed8de8c1 in run_test tools/perf/tests/builtin-test.c:410:9
> #7 0x5570ed8ddadf in test_and_print tools/perf/tests/builtin-test.c:440:9
> #8 0x5570ed8dca04 in __cmd_test tools/perf/tests/builtin-test.c:661:4
> #9 0x5570ed8dbc07 in cmd_test tools/perf/tests/builtin-test.c:807:9
> #10 0x5570ed7326cc in run_builtin tools/perf/perf.c:313:11
> #11 0x5570ed731639 in handle_internal_command tools/perf/perf.c:365:8
> #12 0x5570ed7323cd in run_argv tools/perf/perf.c:409:2
> #13 0x5570ed731076 in main tools/perf/perf.c:539:3
>
> Fixes: commit f5a56570a3f2 ("perf test: Fix memory leaks in parse-metric test")
> Signed-off-by: Ian Rogers <irogers@google.com>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Orthogonal:
The case where metricgroup__parse_groups_test() can fail in
__compute_metric() also looks curious. Should &metric_events be passed
to metricgroup__rblist_exit() in that case?
> ---
> tools/perf/tests/parse-metric.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/perf/tests/parse-metric.c b/tools/perf/tests/parse-metric.c
> index aea4f970fccc..7c1bde01cb50 100644
> --- a/tools/perf/tests/parse-metric.c
> +++ b/tools/perf/tests/parse-metric.c
> @@ -157,6 +157,7 @@ static int __compute_metric(const char *name, struct value *vals,
> }
>
> perf_evlist__set_maps(&evlist->core, cpus, NULL);
> + runtime_stat__init(&st);
>
> /* Parse the metric into metric_events list. */
> err = metricgroup__parse_groups_test(evlist, &map, name,
> @@ -170,7 +171,6 @@ static int __compute_metric(const char *name, struct value *vals,
> goto out;
>
> /* Load the runtime stats with given numbers for events. */
> - runtime_stat__init(&st);
> load_runtime_stat(&st, evlist, vals);
>
> /* And execute the metric */
> --
> 2.28.0.681.g6f77f65b4e-goog
>
> --
> You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20200923210655.4143682-1-irogers%40google.com.
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] perf test: Fix msan uninitialized use.
2020-09-23 23:37 ` Nick Desaulniers
@ 2020-09-24 23:12 ` Ian Rogers
2020-09-28 12:24 ` Arnaldo Carvalho de Melo
1 sibling, 0 replies; 4+ messages in thread
From: Ian Rogers @ 2020-09-24 23:12 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, LKML,
clang-built-linux, Stephane Eranian
On Wed, Sep 23, 2020 at 4:37 PM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Wed, Sep 23, 2020 at 2:07 PM 'Ian Rogers' via Clang Built Linux
> <clang-built-linux@googlegroups.com> wrote:
> >
> > Ensure 'st' is initialized before an error branch is taken.
> > Fixes test "67: Parse and process metrics" with LLVM msan:
> > ==6757==WARNING: MemorySanitizer: use-of-uninitialized-value
> > #0 0x5570edae947d in rblist__exit tools/perf/util/rblist.c:114:2
> > #1 0x5570edb1c6e8 in runtime_stat__exit tools/perf/util/stat-shadow.c:141:2
> > #2 0x5570ed92cfae in __compute_metric tools/perf/tests/parse-metric.c:187:2
> > #3 0x5570ed92cb74 in compute_metric tools/perf/tests/parse-metric.c:196:9
> > #4 0x5570ed92c6d8 in test_recursion_fail tools/perf/tests/parse-metric.c:318:2
> > #5 0x5570ed92b8c8 in test__parse_metric tools/perf/tests/parse-metric.c:356:2
> > #6 0x5570ed8de8c1 in run_test tools/perf/tests/builtin-test.c:410:9
> > #7 0x5570ed8ddadf in test_and_print tools/perf/tests/builtin-test.c:440:9
> > #8 0x5570ed8dca04 in __cmd_test tools/perf/tests/builtin-test.c:661:4
> > #9 0x5570ed8dbc07 in cmd_test tools/perf/tests/builtin-test.c:807:9
> > #10 0x5570ed7326cc in run_builtin tools/perf/perf.c:313:11
> > #11 0x5570ed731639 in handle_internal_command tools/perf/perf.c:365:8
> > #12 0x5570ed7323cd in run_argv tools/perf/perf.c:409:2
> > #13 0x5570ed731076 in main tools/perf/perf.c:539:3
> >
> > Fixes: commit f5a56570a3f2 ("perf test: Fix memory leaks in parse-metric test")
> > Signed-off-by: Ian Rogers <irogers@google.com>
>
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
>
> Orthogonal:
> The case where metricgroup__parse_groups_test() can fail in
> __compute_metric() also looks curious. Should &metric_events be passed
> to metricgroup__rblist_exit() in that case?
Thanks Nick! Checking metricgroup.c, metricgroup__rblist_init does
occur even if an error is returned. So there isn't an error, but that
doesn't mean the code couldn't be cleaner :-)
Ian
> > ---
> > tools/perf/tests/parse-metric.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tools/perf/tests/parse-metric.c b/tools/perf/tests/parse-metric.c
> > index aea4f970fccc..7c1bde01cb50 100644
> > --- a/tools/perf/tests/parse-metric.c
> > +++ b/tools/perf/tests/parse-metric.c
> > @@ -157,6 +157,7 @@ static int __compute_metric(const char *name, struct value *vals,
> > }
> >
> > perf_evlist__set_maps(&evlist->core, cpus, NULL);
> > + runtime_stat__init(&st);
> >
> > /* Parse the metric into metric_events list. */
> > err = metricgroup__parse_groups_test(evlist, &map, name,
> > @@ -170,7 +171,6 @@ static int __compute_metric(const char *name, struct value *vals,
> > goto out;
> >
> > /* Load the runtime stats with given numbers for events. */
> > - runtime_stat__init(&st);
> > load_runtime_stat(&st, evlist, vals);
> >
> > /* And execute the metric */
> > --
> > 2.28.0.681.g6f77f65b4e-goog
> >
> > --
> > You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
> > To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com.
> > To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20200923210655.4143682-1-irogers%40google.com.
>
>
>
> --
> Thanks,
> ~Nick Desaulniers
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] perf test: Fix msan uninitialized use.
2020-09-23 23:37 ` Nick Desaulniers
2020-09-24 23:12 ` Ian Rogers
@ 2020-09-28 12:24 ` Arnaldo Carvalho de Melo
1 sibling, 0 replies; 4+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-09-28 12:24 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Ian Rogers, Peter Zijlstra, Ingo Molnar, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Namhyung Kim, LKML,
clang-built-linux, Stephane Eranian
Em Wed, Sep 23, 2020 at 04:37:08PM -0700, Nick Desaulniers escreveu:
> On Wed, Sep 23, 2020 at 2:07 PM 'Ian Rogers' via Clang Built Linux
> <clang-built-linux@googlegroups.com> wrote:
> >
> > Ensure 'st' is initialized before an error branch is taken.
> > Fixes test "67: Parse and process metrics" with LLVM msan:
> > ==6757==WARNING: MemorySanitizer: use-of-uninitialized-value
> > #0 0x5570edae947d in rblist__exit tools/perf/util/rblist.c:114:2
> > #1 0x5570edb1c6e8 in runtime_stat__exit tools/perf/util/stat-shadow.c:141:2
> > #2 0x5570ed92cfae in __compute_metric tools/perf/tests/parse-metric.c:187:2
> > #3 0x5570ed92cb74 in compute_metric tools/perf/tests/parse-metric.c:196:9
> > #4 0x5570ed92c6d8 in test_recursion_fail tools/perf/tests/parse-metric.c:318:2
> > #5 0x5570ed92b8c8 in test__parse_metric tools/perf/tests/parse-metric.c:356:2
> > #6 0x5570ed8de8c1 in run_test tools/perf/tests/builtin-test.c:410:9
> > #7 0x5570ed8ddadf in test_and_print tools/perf/tests/builtin-test.c:440:9
> > #8 0x5570ed8dca04 in __cmd_test tools/perf/tests/builtin-test.c:661:4
> > #9 0x5570ed8dbc07 in cmd_test tools/perf/tests/builtin-test.c:807:9
> > #10 0x5570ed7326cc in run_builtin tools/perf/perf.c:313:11
> > #11 0x5570ed731639 in handle_internal_command tools/perf/perf.c:365:8
> > #12 0x5570ed7323cd in run_argv tools/perf/perf.c:409:2
> > #13 0x5570ed731076 in main tools/perf/perf.c:539:3
> >
> > Fixes: commit f5a56570a3f2 ("perf test: Fix memory leaks in parse-metric test")
> > Signed-off-by: Ian Rogers <irogers@google.com>
>
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Thanks, applied.
- Arnaldo
> Orthogonal:
> The case where metricgroup__parse_groups_test() can fail in
> __compute_metric() also looks curious. Should &metric_events be passed
> to metricgroup__rblist_exit() in that case?
>
> > ---
> > tools/perf/tests/parse-metric.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tools/perf/tests/parse-metric.c b/tools/perf/tests/parse-metric.c
> > index aea4f970fccc..7c1bde01cb50 100644
> > --- a/tools/perf/tests/parse-metric.c
> > +++ b/tools/perf/tests/parse-metric.c
> > @@ -157,6 +157,7 @@ static int __compute_metric(const char *name, struct value *vals,
> > }
> >
> > perf_evlist__set_maps(&evlist->core, cpus, NULL);
> > + runtime_stat__init(&st);
> >
> > /* Parse the metric into metric_events list. */
> > err = metricgroup__parse_groups_test(evlist, &map, name,
> > @@ -170,7 +171,6 @@ static int __compute_metric(const char *name, struct value *vals,
> > goto out;
> >
> > /* Load the runtime stats with given numbers for events. */
> > - runtime_stat__init(&st);
> > load_runtime_stat(&st, evlist, vals);
> >
> > /* And execute the metric */
> > --
> > 2.28.0.681.g6f77f65b4e-goog
> >
> > --
> > You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
> > To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com.
> > To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20200923210655.4143682-1-irogers%40google.com.
>
>
>
> --
> Thanks,
> ~Nick Desaulniers
--
- Arnaldo
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-09-28 12:24 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-23 21:06 [PATCH] perf test: Fix msan uninitialized use Ian Rogers
2020-09-23 23:37 ` Nick Desaulniers
2020-09-24 23:12 ` Ian Rogers
2020-09-28 12:24 ` Arnaldo Carvalho de Melo
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.