On 10/7/19 11:28 AM, Alex Bennée wrote:
> +static uint32_t get_e_machine(void)
> +{
> + uint32_t e_machine = EM_NONE;
> + Elf64_Ehdr elf_header;
Not ideal, as this appears to not work on 32-bit hosts, but the two structures
do match up within the first 24 bytes, in which this is located.
That said, this value is present within tcg/host/tcg-target.inc.c as
ELF_HOST_MACHINE. So we really don't have to play /proc/self/exec games.
> +void start_jitdump_file(void)
> +{
> + g_autofree gchar *dumpfile_name = g_strdup_printf("./jit-%d.dump",
> getpid());
> + dumpfile = fopen(dumpfile_name, "w+");
> +
> + /* 'Perf record' saves mmaped files during the execution of a program and
> + * 'perf inject' iterate over them to reconstruct all used/executed
> binary.
> + * So, we create a mmap with the path of our jitdump that is processed
> + * and used by 'perf inject' to reconstruct jitted binaries.
> + */
> + perf_marker = mmap(NULL, sysconf(_SC_PAGESIZE),
> + PROT_READ | PROT_EXEC,
> + MAP_PRIVATE,
> + fileno(dumpfile), 0);
(1) sysconf(_SC_PAGESIZE) is qemu_real_host_page_size.
(2) This is a page-sized mapping of a new, zero-sized file?
I assume this mapping event gets logged, and that it its
only purpose?
(3) I really need to read the kernel docs...
> +void append_load_in_jitdump_file(TranslationBlock *tb)
> +{
> + gchar *func_name = g_strdup_printf("TB virt:0x"TARGET_FMT_lx, tb->pc);
> +
> + /* Serialise the writing of the dump file */
> + qemu_mutex_lock(&dumpfile_lock);
> +
> + struct jr_code_load load_event;
> + load_event.p.id = JIT_CODE_LOAD;
> + load_event.p.total_size =
> + sizeof(struct jr_code_load) + func_name->len + 1 + tb->tc.size;
How does a "gchar *func_name" have ->len? Did this used to be GString, but a
last-minute change means it no longer compiles?
> + fflush(dumpfile);
Why fflushing all of the time? Surely the file contents doesn't matter until
after the final close.
> + qemu_mutex_unlock(&dumpfile_lock);
Why a separate qemu locking instead of using stdio's own locking (flockfile).
> diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
> index 871d91d559..3fafb656e7 100644
> --- a/accel/tcg/translate-all.c
> +++ b/accel/tcg/translate-all.c
> @@ -58,6 +58,10 @@
> #include "sysemu/cpus.h"
> #include "sysemu/tcg.h"
>
> +#ifdef __linux__
> +#include "perf/jitdump.h"
> +#endif
Why the ifdefs? We're not dependent on other headers are we?
Not that there's a "perf" on other hosts, but AFACT it should
at least compile...
r~
