| 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: <CAP-5=fU8CJzOttgVDSxqHQoRg_eZ1+sToywOiek+8vw4j2GykA@mail.gmail.com>
Date: Tue, 26 May 2020 22:03:45 -0700
From: Ian Rogers <irogers@...gle.com>
To: Nick Gasson <nick.gasson@....com>
Cc: Arnaldo Carvalho de Melo <acme@...nel.org>,
Jiri Olsa <jolsa@...hat.com>,
Peter Zijlstra <peterz@...radead.org>,
Stephane Eranian <eranian@...gle.com>,
LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] perf jvmti: remove redundant jitdump line table entries
On Thu, May 21, 2020 at 11:54 PM Nick Gasson <nick.gasson@....com> wrote:
>
> For each PC/BCI pair in the JVMTI compiler inlining record table, the
> jitdump plugin emits debug line table entries for every source line in
> the method preceding that BCI. Instead only emit one source line per
> PC/BCI pair. Reported by Ian Rogers. This reduces the .dump size for
> SPECjbb from ~230MB to ~40MB.
Great result, thanks! I note there is a lack of symbolization when
benchmarking a few Java applications. I'll try to see if there's a
sensible resolution for those.
> Also fix an error in the DWARF line table state machine where addresses
> are incorrectly offset by -0x40 (GEN_ELF_TEXT_OFFSET). This can be seen
> with `objdump -S` on the ELF files after perf inject.
It'd be better to make this into two patches. Also on acme's perf/core
branch if possible:
https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/log/?h=perf/core
> Signed-off-by: Nick Gasson <nick.gasson@....com>
> ---
> tools/perf/jvmti/libjvmti.c | 73 +++++++++++++---------------------
> tools/perf/util/genelf_debug.c | 4 +-
> 2 files changed, 30 insertions(+), 47 deletions(-)
>
> diff --git a/tools/perf/jvmti/libjvmti.c b/tools/perf/jvmti/libjvmti.c
> index a9a056d68416..398e4ba6498d 100644
> --- a/tools/perf/jvmti/libjvmti.c
> +++ b/tools/perf/jvmti/libjvmti.c
> @@ -32,38 +32,41 @@ static void print_error(jvmtiEnv *jvmti, const char *msg, jvmtiError ret)
>
> #ifdef HAVE_JVMTI_CMLR
> static jvmtiError
> -do_get_line_numbers(jvmtiEnv *jvmti, void *pc, jmethodID m, jint bci,
> - jvmti_line_info_t *tab, jint *nr)
> +do_get_line_number(jvmtiEnv *jvmti, void *pc, jmethodID m, jint bci,
> + jvmti_line_info_t *tab)
> {
> - jint i, lines = 0;
> - jint nr_lines = 0;
> + jint i, nr_lines = 0;
> jvmtiLineNumberEntry *loc_tab = NULL;
> jvmtiError ret;
> + jint src_line = -1;
>
> ret = (*jvmti)->GetLineNumberTable(jvmti, m, &nr_lines, &loc_tab);
> if (ret == JVMTI_ERROR_ABSENT_INFORMATION || ret == JVMTI_ERROR_NATIVE_METHOD) {
> /* No debug information for this method */
> - *nr = 0;
> - return JVMTI_ERROR_NONE;
> + return ret;
> } else if (ret != JVMTI_ERROR_NONE) {
> print_error(jvmti, "GetLineNumberTable", ret);
> return ret;
> }
>
> - for (i = 0; i < nr_lines; i++) {
> - if (loc_tab[i].start_location < bci) {
> - tab[lines].pc = (unsigned long)pc;
> - tab[lines].line_number = loc_tab[i].line_number;
> - tab[lines].discrim = 0; /* not yet used */
> - tab[lines].methodID = m;
> - lines++;
> - } else {
> - break;
> - }
> + for (i = 0; i < nr_lines && loc_tab[i].start_location <= bci; i++) {
> + src_line = i;
> + }
> +
> + if (src_line != -1) {
> + tab->pc = (unsigned long)pc;
> + tab->line_number = loc_tab[src_line].line_number;
> + tab->discrim = 0; /* not yet used */
> + tab->methodID = m;
> +
> + ret = JVMTI_ERROR_NONE;
> + } else {
> + ret = JVMTI_ERROR_ABSENT_INFORMATION;
> }
> +
> (*jvmti)->Deallocate(jvmti, (unsigned char *)loc_tab);
> - *nr = lines;
> - return JVMTI_ERROR_NONE;
> +
> + return ret;
> }
>
> static jvmtiError
> @@ -71,9 +74,8 @@ get_line_numbers(jvmtiEnv *jvmti, const void *compile_info, jvmti_line_info_t **
> {
> const jvmtiCompiledMethodLoadRecordHeader *hdr;
> jvmtiCompiledMethodLoadInlineRecord *rec;
> - jvmtiLineNumberEntry *lne = NULL;
> PCStackInfo *c;
> - jint nr, ret;
> + jint ret;
> int nr_total = 0;
> int i, lines_total = 0;
>
> @@ -86,24 +88,7 @@ get_line_numbers(jvmtiEnv *jvmti, const void *compile_info, jvmti_line_info_t **
> for (hdr = compile_info; hdr != NULL; hdr = hdr->next) {
> if (hdr->kind == JVMTI_CMLR_INLINE_INFO) {
> rec = (jvmtiCompiledMethodLoadInlineRecord *)hdr;
> - for (i = 0; i < rec->numpcs; i++) {
> - c = rec->pcinfo + i;
> - nr = 0;
> - /*
> - * unfortunately, need a tab to get the number of lines!
> - */
> - ret = (*jvmti)->GetLineNumberTable(jvmti, c->methods[0], &nr, &lne);
> - if (ret == JVMTI_ERROR_NONE) {
> - /* free what was allocated for nothing */
> - (*jvmti)->Deallocate(jvmti, (unsigned char *)lne);
> - nr_total += (int)nr;
> - } else if (ret == JVMTI_ERROR_ABSENT_INFORMATION
> - || ret == JVMTI_ERROR_NATIVE_METHOD) {
> - /* No debug information for this method */
> - } else {
> - print_error(jvmti, "GetLineNumberTable", ret);
> - }
> - }
> + nr_total += rec->numpcs;
> }
> }
>
> @@ -122,14 +107,12 @@ get_line_numbers(jvmtiEnv *jvmti, const void *compile_info, jvmti_line_info_t **
> rec = (jvmtiCompiledMethodLoadInlineRecord *)hdr;
> for (i = 0; i < rec->numpcs; i++) {
> c = rec->pcinfo + i;
> - nr = 0;
> - ret = do_get_line_numbers(jvmti, c->pc,
> - c->methods[0],
> - c->bcis[0],
> - *tab + lines_total,
> - &nr);
> + ret = do_get_line_number(jvmti, c->pc,
> + c->methods[0],
> + c->bcis[0],
> + *tab + lines_total);
Nit: It'd be nice to comment here that [0] is the leaf method/bci. I
don't believe it is supported but c->numstackframes could be used to
give full inlining information.
Thanks,
Ian
> if (ret == JVMTI_ERROR_NONE)
> - lines_total += nr;
> + lines_total++;
> }
> }
> }
> diff --git a/tools/perf/util/genelf_debug.c b/tools/perf/util/genelf_debug.c
> index 30e9f618f6cd..dd40683bd4c0 100644
> --- a/tools/perf/util/genelf_debug.c
> +++ b/tools/perf/util/genelf_debug.c
> @@ -342,7 +342,7 @@ static void emit_lineno_info(struct buffer_ext *be,
> */
>
> /* start state of the state machine we take care of */
> - unsigned long last_vma = code_addr;
> + unsigned long last_vma = 0;
> char const *cur_filename = NULL;
> unsigned long cur_file_idx = 0;
> int last_line = 1;
> @@ -473,7 +473,7 @@ jit_process_debug_info(uint64_t code_addr,
> ent = debug_entry_next(ent);
> }
> add_compilation_unit(di, buffer_ext_size(dl));
> - add_debug_line(dl, debug, nr_debug_entries, 0);
> + add_debug_line(dl, debug, nr_debug_entries, GEN_ELF_TEXT_OFFSET);
> add_debug_abbrev(da);
> if (0) buffer_ext_dump(da, "abbrev");
>
> --
> 2.26.2
>
Powered by blists - more mailing lists