On Fri, 2023-06-02 at 18:21 +0100, Peter Maydell wrote:
> On Mon, 16 Jan 2023 at 22:36, Richard Henderson
> <richard.hender...@linaro.org> wrote:
> >
> > From: Ilya Leoshkevich <i...@linux.ibm.com>
> >
> > Add ability to dump /tmp/perf-<pid>.map and jit-<pid>.dump.
> > The first one allows the perf tool to map samples to each
> > individual
> > translation block. The second one adds the ability to resolve
> > symbol
> > names, line numbers and inspect JITed code.
> >
> > Example of use:
> >
> > perf record qemu-x86_64 -perfmap ./a.out
> > perf report
> >
> > or
> >
> > perf record -k 1 qemu-x86_64 -jitdump ./a.out
> > DEBUGINFOD_URLS= perf inject -j -i perf.data -o
> > perf.data.jitted
> > perf report -i perf.data.jitted
> >
> > Co-developed-by: Vanderson M. do Rosario <vanderson...@gmail.com>
> > Co-developed-by: Alex Bennée <alex.ben...@linaro.org>
> > Signed-off-by: Ilya Leoshkevich <i...@linux.ibm.com>
> > Message-Id: <20230112152013.125680-4-...@linux.ibm.com>
> > Signed-off-by: Richard Henderson <richard.hender...@linaro.org>
>
> Hi; Coverity thinks (CID 1507521) that there's a memory leak
> in this code:
>
> > +void perf_enable_jitdump(void)
> > +{
> > + struct jitheader header;
> > + char jitdump_file[32];
> > + void *perf_marker;
> > +
> > + if (!use_rt_clock) {
> > + warn_report("CLOCK_MONOTONIC is not available, proceeding
> > without jitdump");
> > + return;
> > + }
> > +
> > + snprintf(jitdump_file, sizeof(jitdump_file), "jit-%d.dump",
> > getpid());
> > + jitdump = safe_fopen_w(jitdump_file);
> > + if (jitdump == NULL) {
> > + warn_report("Could not open %s: %s, proceeding without
> > jitdump",
> > + jitdump_file, strerror(errno));
> > + return;
> > + }
> > +
> > + /*
> > + * `perf inject` will see that the mapped file name in the
> > corresponding
> > + * PERF_RECORD_MMAP or PERF_RECORD_MMAP2 event is of the form
> > jit-%d.dump
> > + * and will process it as a jitdump file.
> > + */
> > + perf_marker = mmap(NULL, qemu_real_host_page_size(), PROT_READ
> > | PROT_EXEC,
> > + MAP_PRIVATE, fileno(jitdump), 0);
>
> Here we mmap() something...
>
> > + if (perf_marker == MAP_FAILED) {
> > + warn_report("Could not map %s: %s, proceeding without
> > jitdump",
> > + jitdump_file, strerror(errno));
> > + fclose(jitdump);
> > + jitdump = NULL;
> > + return;
> > + }
> > +
> > + header.magic = JITHEADER_MAGIC;
> > + header.version = JITHEADER_VERSION;
> > + header.total_size = sizeof(header);
> > + header.elf_mach = get_e_machine();
> > + header.pad1 = 0;
> > + header.pid = getpid();
> > + header.timestamp = get_clock();
> > + header.flags = 0;
> > + fwrite(&header, sizeof(header), 1, jitdump);
>
> ...but we never do anything with that pointer, so the memory
> we just mmap()ed is never going to be freed.
>
> Is this doing something particularly magical, or should that
> pointer be kept track of somewhere ?
>
> thanks
> -- PMM
It's magic that points perf to the location of the jitdump file,
but it won't hurt munmap()ping it in perf_exit(). I'll send a patch.
