We can't assume inlined symbols with the same name are equal, because
their address range may be different. This will cause the symbols with
different addresses be shadowed when adding to the hist entry, and lead
to ERANGE error when checking the symbol address during sample parse, the
addr should be within the range of [sym.start, sym.end].
The error message is like: "0x36aea60 [0x8]: failed to process type: 68".
The second parameter of symbol__new() is the length of the fake symbol for
the inline frame, which is the subtraction of the end and start address of
base_sym.
Signed-off-by: He Kuang <[email protected]>
---
tools/perf/util/sort.c | 10 ++++++++--
tools/perf/util/srcline.c | 2 +-
2 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 6c1a83768eb0..d0334c33da54 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -230,8 +230,14 @@ static int64_t _sort__sym_cmp(struct symbol *sym_l, struct symbol *sym_r)
if (sym_l == sym_r)
return 0;
- if (sym_l->inlined || sym_r->inlined)
- return strcmp(sym_l->name, sym_r->name);
+ if (sym_l->inlined || sym_r->inlined) {
+ int ret = strcmp(sym_l->name, sym_r->name);
+
+ if (ret)
+ return ret;
+ if ((sym_l->start <= sym_r->end) && (sym_l->end >= sym_r->start))
+ return 0;
+ }
if (sym_l->start != sym_r->start)
return (int64_t)(sym_r->start - sym_l->start);
diff --git a/tools/perf/util/srcline.c b/tools/perf/util/srcline.c
index dc86597d0cc4..ccf42c4e83f0 100644
--- a/tools/perf/util/srcline.c
+++ b/tools/perf/util/srcline.c
@@ -104,7 +104,7 @@ static struct symbol *new_inline_sym(struct dso *dso,
} else {
/* create a fake symbol for the inline frame */
inline_sym = symbol__new(base_sym ? base_sym->start : 0,
- base_sym ? base_sym->end : 0,
+ base_sym ? (base_sym->end - base_sym->start) : 0,
base_sym ? base_sym->binding : 0,
base_sym ? base_sym->type : 0,
funcname);
--
2.20.1
On Tue, Feb 19, 2019 at 09:05:31PM +0800, He Kuang wrote:
> We can't assume inlined symbols with the same name are equal, because
> their address range may be different. This will cause the symbols with
> different addresses be shadowed when adding to the hist entry, and lead
> to ERANGE error when checking the symbol address during sample parse, the
> addr should be within the range of [sym.start, sym.end].
>
> The error message is like: "0x36aea60 [0x8]: failed to process type: 68".
>
> The second parameter of symbol__new() is the length of the fake symbol for
> the inline frame, which is the subtraction of the end and start address of
> base_sym.
>
> Signed-off-by: He Kuang <[email protected]>
Acked-by: Jiri Olsa <[email protected]>
thanks,
jirka
> ---
> tools/perf/util/sort.c | 10 ++++++++--
> tools/perf/util/srcline.c | 2 +-
> 2 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
> index 6c1a83768eb0..d0334c33da54 100644
> --- a/tools/perf/util/sort.c
> +++ b/tools/perf/util/sort.c
> @@ -230,8 +230,14 @@ static int64_t _sort__sym_cmp(struct symbol *sym_l, struct symbol *sym_r)
> if (sym_l == sym_r)
> return 0;
>
> - if (sym_l->inlined || sym_r->inlined)
> - return strcmp(sym_l->name, sym_r->name);
> + if (sym_l->inlined || sym_r->inlined) {
> + int ret = strcmp(sym_l->name, sym_r->name);
> +
> + if (ret)
> + return ret;
> + if ((sym_l->start <= sym_r->end) && (sym_l->end >= sym_r->start))
> + return 0;
> + }
>
> if (sym_l->start != sym_r->start)
> return (int64_t)(sym_r->start - sym_l->start);
> diff --git a/tools/perf/util/srcline.c b/tools/perf/util/srcline.c
> index dc86597d0cc4..ccf42c4e83f0 100644
> --- a/tools/perf/util/srcline.c
> +++ b/tools/perf/util/srcline.c
> @@ -104,7 +104,7 @@ static struct symbol *new_inline_sym(struct dso *dso,
> } else {
> /* create a fake symbol for the inline frame */
> inline_sym = symbol__new(base_sym ? base_sym->start : 0,
> - base_sym ? base_sym->end : 0,
> + base_sym ? (base_sym->end - base_sym->start) : 0,
> base_sym ? base_sym->binding : 0,
> base_sym ? base_sym->type : 0,
> funcname);
> --
> 2.20.1
>
Em Tue, Feb 19, 2019 at 03:02:13PM +0100, Jiri Olsa escreveu:
> On Tue, Feb 19, 2019 at 09:05:31PM +0800, He Kuang wrote:
> > We can't assume inlined symbols with the same name are equal, because
> > their address range may be different. This will cause the symbols with
> > different addresses be shadowed when adding to the hist entry, and lead
> > to ERANGE error when checking the symbol address during sample parse, the
> > addr should be within the range of [sym.start, sym.end].
> > The error message is like: "0x36aea60 [0x8]: failed to process type: 68".
> > The second parameter of symbol__new() is the length of the fake symbol for
> > the inline frame, which is the subtraction of the end and start address of
> > base_sym.
> > Signed-off-by: He Kuang <[email protected]>
> Acked-by: Jiri Olsa <[email protected]>
Thanks, applying and adding the missing Fixes line:
Fixes: aa441895f7b4 ("perf report: Compare symbol name for inlined frames when sorting")
Also added Millian, the author of that patch to the Cc list, so that he
can check this as well if he has the time for doing so.
Please double check that this indeed is when this problem was
introduced,
- Arnaldo
Commit-ID: 7346195e8643482968f547483e0d823ec1982fab
Gitweb: https://git.kernel.org/tip/7346195e8643482968f547483e0d823ec1982fab
Author: He Kuang <[email protected]>
AuthorDate: Tue, 19 Feb 2019 21:05:31 +0800
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 19 Feb 2019 12:30:12 -0300
perf report: Don't shadow inlined symbol with different addr range
We can't assume inlined symbols with the same name are equal, because
their address range may be different. This will cause the symbols with
different addresses be shadowed when adding to the hist entry, and lead
to ERANGE error when checking the symbol address during sample parse,
the addr should be within the range of [sym.start, sym.end].
The error message is like: "0x36aea60 [0x8]: failed to process type: 68".
The second parameter of symbol__new() is the length of the fake symbol
for the inline frame, which is the subtraction of the end and start
address of base_sym.
Signed-off-by: He Kuang <[email protected]>
Acked-by: Jiri Olsa <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: Milian Wolff <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Fixes: aa441895f7b4 ("perf report: Compare symbol name for inlined frames when sorting")
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/sort.c | 10 ++++++++--
tools/perf/util/srcline.c | 2 +-
2 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 2b6c1ccb878c..d2299e912e59 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -231,8 +231,14 @@ static int64_t _sort__sym_cmp(struct symbol *sym_l, struct symbol *sym_r)
if (sym_l == sym_r)
return 0;
- if (sym_l->inlined || sym_r->inlined)
- return strcmp(sym_l->name, sym_r->name);
+ if (sym_l->inlined || sym_r->inlined) {
+ int ret = strcmp(sym_l->name, sym_r->name);
+
+ if (ret)
+ return ret;
+ if ((sym_l->start <= sym_r->end) && (sym_l->end >= sym_r->start))
+ return 0;
+ }
if (sym_l->start != sym_r->start)
return (int64_t)(sym_r->start - sym_l->start);
diff --git a/tools/perf/util/srcline.c b/tools/perf/util/srcline.c
index 00f215580b5a..10ca1533937e 100644
--- a/tools/perf/util/srcline.c
+++ b/tools/perf/util/srcline.c
@@ -104,7 +104,7 @@ static struct symbol *new_inline_sym(struct dso *dso,
} else {
/* create a fake symbol for the inline frame */
inline_sym = symbol__new(base_sym ? base_sym->start : 0,
- base_sym ? base_sym->end : 0,
+ base_sym ? (base_sym->end - base_sym->start) : 0,
base_sym ? base_sym->binding : 0,
base_sym ? base_sym->type : 0,
funcname);