Re: [PATCH 1/3] perf script brstackinsn: Fix recovery from LBR/binary mismatch
Em Fri, Sep 27, 2019 at 04:35:44PM -0700, Andi Kleen escreveu:
> From: Andi Kleen
>
> When the LBR data and the instructions in a binary do not match the
> loop printing instructions could get confused and print a long
> stream of bogus instructions.
>
> The problem was that if the instruction decoder cannot decode an
> instruction it ilen wasn't initialized, so the loop going through
> the basic block would continue with the previous value.
>
> Harden the code to avoid such problems:
> - Make sure ilen is always freshly initialized and is 0 for bad
> instructions.
> - Do not overrun the code buffer while printing instructions
> - Print a warning message if the final jump is not on an
> instruction boundary.
Thanks, applied.
- Arnaldo
> Signed-off-by: Andi Kleen
> ---
> tools/perf/builtin-script.c | 6 +-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> index e079b34201f2..32b17d51c982 100644
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -1061,7 +1061,7 @@ static int perf_sample__fprintf_brstackinsn(struct
> perf_sample *sample,
> continue;
>
> insn = 0;
> - for (off = 0;; off += ilen) {
> + for (off = 0; off < (unsigned)len; off += ilen) {
> uint64_t ip = start + off;
>
> printed += ip__fprintf_sym(ip, thread, x.cpumode,
> x.cpu, &lastsym, attr, fp);
> @@ -1072,6 +1072,7 @@ static int perf_sample__fprintf_brstackinsn(struct
> perf_sample *sample,
> printed += print_srccode(thread,
> x.cpumode, ip);
> break;
> } else {
> + ilen = 0;
> printed += fprintf(fp, "\t%016" PRIx64
> "\t%s\n", ip,
> dump_insn(&x, ip, buffer +
> off, len - off, &ilen));
> if (ilen == 0)
> @@ -1081,6 +1082,8 @@ static int perf_sample__fprintf_brstackinsn(struct
> perf_sample *sample,
> insn++;
> }
> }
> + if (off != (unsigned)len)
> + printed += fprintf(fp, "\tmismatch of LBR data and
> executable\n");
> }
>
> /*
> @@ -1121,6 +1124,7 @@ static int perf_sample__fprintf_brstackinsn(struct
> perf_sample *sample,
> goto out;
> }
> for (off = 0; off <= end - start; off += ilen) {
> + ilen = 0;
> printed += fprintf(fp, "\t%016" PRIx64 "\t%s\n", start + off,
> dump_insn(&x, start + off, buffer + off, len
> - off, &ilen));
> if (ilen == 0)
> --
> 2.21.0
--
- Arnaldo
[PATCH 1/3] perf script brstackinsn: Fix recovery from LBR/binary mismatch
From: Andi Kleen
When the LBR data and the instructions in a binary do not match the
loop printing instructions could get confused and print a long
stream of bogus instructions.
The problem was that if the instruction decoder cannot decode an
instruction it ilen wasn't initialized, so the loop going through
the basic block would continue with the previous value.
Harden the code to avoid such problems:
- Make sure ilen is always freshly initialized and is 0 for bad
instructions.
- Do not overrun the code buffer while printing instructions
- Print a warning message if the final jump is not on an
instruction boundary.
Signed-off-by: Andi Kleen
---
tools/perf/builtin-script.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index e079b34201f2..32b17d51c982 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -1061,7 +1061,7 @@ static int perf_sample__fprintf_brstackinsn(struct
perf_sample *sample,
continue;
insn = 0;
- for (off = 0;; off += ilen) {
+ for (off = 0; off < (unsigned)len; off += ilen) {
uint64_t ip = start + off;
printed += ip__fprintf_sym(ip, thread, x.cpumode,
x.cpu, &lastsym, attr, fp);
@@ -1072,6 +1072,7 @@ static int perf_sample__fprintf_brstackinsn(struct
perf_sample *sample,
printed += print_srccode(thread,
x.cpumode, ip);
break;
} else {
+ ilen = 0;
printed += fprintf(fp, "\t%016" PRIx64
"\t%s\n", ip,
dump_insn(&x, ip, buffer +
off, len - off, &ilen));
if (ilen == 0)
@@ -1081,6 +1082,8 @@ static int perf_sample__fprintf_brstackinsn(struct
perf_sample *sample,
insn++;
}
}
+ if (off != (unsigned)len)
+ printed += fprintf(fp, "\tmismatch of LBR data and
executable\n");
}
/*
@@ -1121,6 +1124,7 @@ static int perf_sample__fprintf_brstackinsn(struct
perf_sample *sample,
goto out;
}
for (off = 0; off <= end - start; off += ilen) {
+ ilen = 0;
printed += fprintf(fp, "\t%016" PRIx64 "\t%s\n", start + off,
dump_insn(&x, start + off, buffer + off, len
- off, &ilen));
if (ilen == 0)
--
2.21.0
