Re: [PULL 06/35] Hexagon (disas) disassembler

[Peter Maydell]

On Wed, 17 Feb 2021 at 23:40, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> From: Taylor Simpson <tsimpson@quicinc.com>
>
> Add hexagon to disas/meson.build
> Add disas/hexagon.c
> Add hexagon to include/disas/dis-asm.h
>
> Signed-off-by: Taylor Simpson <tsimpson@quicinc.com>
> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Message-Id: <1612763186-18161-6-git-send-email-tsimpson@quicinc.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Coverity reports a memory leak in this code (CID 1460121):



> +int print_insn_hexagon(bfd_vma memaddr, struct disassemble_info *info)
> +{
> +    uint32_t words[PACKET_WORDS_MAX];
> +    bool found_end = false;
> +    GString *buf = g_string_sized_new(PACKET_BUFFER_LEN);

We allocate buf here...

> +    int i, len;
> +
> +    for (i = 0; i < PACKET_WORDS_MAX && !found_end; i++) {
> +        int status = (*info->read_memory_func)(memaddr + i * 
> sizeof(uint32_t),
> +                                               (bfd_byte *)&words[i],
> +                                               sizeof(uint32_t), info);
> +        if (status) {
> +            if (i > 0) {
> +                break;
> +            }
> +            (*info->memory_error_func)(status, memaddr, info);
> +            return status;

...but in the early error return cases here...

> +        }
> +        if (is_packet_end(words[i])) {
> +            found_end = true;
> +        }
> +    }
> +
> +    if (!found_end) {
> +        (*info->fprintf_func)(info->stream, "<invalid>");
> +        return PACKET_WORDS_MAX * sizeof(uint32_t);

...and here we do not free it.

> +    }
> +

Easiest fix is to move the allocation
   buf = g_string_sized_new(PACKET_BUFFER_LEN);
down to here, just above the point where we're going to use it.

> +    len = disassemble_hexagon(words, i, memaddr, buf);
> +    (*info->fprintf_func)(info->stream, "%s", buf->str);
> +    g_string_free(buf, true);
> +
> +    return len;
> +}

thanks
-- PMM