Messages in this thread Patch in this message | | | Date | Mon, 3 Apr 2023 17:11:47 -0300 | From | Arnaldo Carvalho de Melo <> | Subject | Re: [PATCH v1] perf jit: Fix a few memory leaks |
| |
Em Mon, Apr 03, 2023 at 11:50:57AM -0700, Ian Rogers escreveu: > As reported by leak sanitizer. > > Signed-off-by: Ian Rogers <irogers@google.com> > --- > tools/perf/util/genelf_debug.c | 48 +++++++++++++++++++++++++--------- > tools/perf/util/jitdump.c | 7 ++++- > 2 files changed, 41 insertions(+), 14 deletions(-) > > diff --git a/tools/perf/util/genelf_debug.c b/tools/perf/util/genelf_debug.c > index dd40683bd4c0..bdcd37c34b64 100644 > --- a/tools/perf/util/genelf_debug.c > +++ b/tools/perf/util/genelf_debug.c > @@ -87,6 +87,12 @@ buffer_ext_init(struct buffer_ext *be) > be->max_sz = 0; > } > > +static void > +buffer_ext_exit(struct buffer_ext *be) > +{ > + free(be->data); > +} > + > static inline size_t > buffer_ext_size(struct buffer_ext *be) > { > @@ -494,21 +500,26 @@ jit_add_debug_info(Elf *e, uint64_t code_addr, void *debug, int nr_debug_entries > buffer_ext_init(&da); > > ret = jit_process_debug_info(code_addr, debug, nr_debug_entries, &dl, &da, &di);
Can we instead have:
int ret = -1; if (jit_process_debug_info(code_addr, debug, nr_debug_entries, &dl, &da, &di)) goto out;
And then remove all these ret = -1;
And...
> - if (ret) > - return -1; > + if (ret) { > + ret = -1; > + goto out; > + } > + > /* > * setup .debug_line section > */ > scn = elf_newscn(e); > if (!scn) { > warnx("cannot create section"); > - return -1; > + ret = -1; > + goto out; > } > > d = elf_newdata(scn); > if (!d) { > warnx("cannot get new data"); > - return -1; > + ret = -1; > + goto out; > } > > d->d_align = 1; > @@ -521,7 +532,8 @@ jit_add_debug_info(Elf *e, uint64_t code_addr, void *debug, int nr_debug_entries > shdr = elf_getshdr(scn); > if (!shdr) { > warnx("cannot get section header"); > - return -1; > + ret = -1; > + goto out; > } > > shdr->sh_name = 52; /* .debug_line */ > @@ -536,13 +548,15 @@ jit_add_debug_info(Elf *e, uint64_t code_addr, void *debug, int nr_debug_entries > scn = elf_newscn(e); > if (!scn) { > warnx("cannot create section"); > - return -1; > + ret = -1; > + goto out; > } > > d = elf_newdata(scn); > if (!d) { > warnx("cannot get new data"); > - return -1; > + ret = -1; > + goto out; > } > > d->d_align = 1; > @@ -555,7 +569,8 @@ jit_add_debug_info(Elf *e, uint64_t code_addr, void *debug, int nr_debug_entries > shdr = elf_getshdr(scn); > if (!shdr) { > warnx("cannot get section header"); > - return -1; > + ret = -1; > + goto out; > } > > shdr->sh_name = 64; /* .debug_info */ > @@ -570,13 +585,15 @@ jit_add_debug_info(Elf *e, uint64_t code_addr, void *debug, int nr_debug_entries > scn = elf_newscn(e); > if (!scn) { > warnx("cannot create section"); > - return -1; > + ret = -1; > + goto out; > } > > d = elf_newdata(scn); > if (!d) { > warnx("cannot get new data"); > - return -1; > + ret = -1; > + goto out; > } > > d->d_align = 1; > @@ -589,7 +606,8 @@ jit_add_debug_info(Elf *e, uint64_t code_addr, void *debug, int nr_debug_entries > shdr = elf_getshdr(scn); > if (!shdr) { > warnx("cannot get section header"); > - return -1; > + ret = -1; > + goto out; > } > > shdr->sh_name = 76; /* .debug_info */ > @@ -603,7 +621,11 @@ jit_add_debug_info(Elf *e, uint64_t code_addr, void *debug, int nr_debug_entries > */ > if (elf_update(e, ELF_C_WRITE) < 0) { > warnx("elf_update debug failed"); > - return -1; > + ret = -1; > } > - return 0;
.. just add: ret = 0;
here?
> +out: > + buffer_ext_exit(&dl); > + buffer_ext_exit(&di); > + buffer_ext_exit(&da); > + return ret; > } > diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c > index 0e033278fa12..28e49502db5e 100644 > --- a/tools/perf/util/jitdump.c > +++ b/tools/perf/util/jitdump.c > @@ -235,9 +235,11 @@ jit_open(struct jit_buf_desc *jd, const char *name) > */ > strcpy(jd->dir, name); > dirname(jd->dir); > + free(buf); > > return 0; > error: > + free(buf); > funlockfile(jd->in); > fclose(jd->in); > return retval; > @@ -523,7 +525,7 @@ static int jit_repipe_code_load(struct jit_buf_desc *jd, union jr_entry *jr) > > ret = perf_event__process_mmap2(tool, event, &sample, jd->machine); > if (ret) > - return ret; > + goto out; > > ret = jit_inject_event(jd, event); > /* > @@ -532,6 +534,8 @@ static int jit_repipe_code_load(struct jit_buf_desc *jd, union jr_entry *jr) > if (!ret) > build_id__mark_dso_hit(tool, event, &sample, NULL, jd->machine); > > +out: > + free(event); > return ret; > } > > @@ -874,6 +878,7 @@ jit_process(struct perf_session *session, > } > > nsinfo__put(jd.nsi); > + free(jd.buf); > > return ret; > } > -- > 2.40.0.348.gf938b09366-goog >
--
- Arnaldo
| |