[PATCH] gdb: improve error reporting from the disassembler

[Simon Marchi]

On 2021-10-11 9:10 a.m., Andrew Burgess wrote:
> If the libopcodes disassembler returns a negative value then this
> indicates that the disassembly failed for some reason.  In disas.c, in
> the function gdb_disassembler::print_insn we can see how this is
> handled; when we get a negative value back, we call the memory_error
> function, which throws an exception.
>
> The problem here is that the address used in the memory_error call is
> gdb_disassembler::m_err_memaddr, which is set in
> gdb_disassembler::dis_asm_memory_error, which is called from within
> the libopcodes disassembler through the
> disassembler_info::memory_error_func callback.
>
> However, for this to work correctly, every time the libopcodes
> disassembler returns a negative value, the libopcodes disassembler
> must have first called the memory_error_func callback.
>
> My first plan was to make m_err_memaddr a gdb::optional, and assert
> that it always had a value prior to calling memory_error, however, a
> quick look in opcodes/*-dis.c shows that there _are_ cases where a
> negative value is returned without first calling the memory_error_func
> callback, for example in arc-dis.c and cris-dis.c.
>
> Now, I think that a good argument can be made that these disassemblers
> must therefore be broken, except for the case where we can't read
> memory, we should always be able to disassemble the memory contents to
> _something_, even if it's just '.word 0x....'.  However, I certainly
> don't plan to go and fix all of the disassemblers.
>
> What I do propose to do then, is make m_err_memaddr a gdb::optional,
> but now, instead of always calling memory_error, I add a new path
> which just calls error complaining about an unknown error.  This new
> path is only used if m_err_memaddr doesn't have a value (indicating
> that the memory_error_func callback was not called).
>
> To test this I just augmented one of the disassemblers to always
> return -1, before this patch I see this:
>
>   Dump of assembler code for function main:
>      0x000101aa <+0>:	Cannot access memory at address 0x0
>
> And after this commit I now see:
>
>   Dump of assembler code for function main:
>      0x000101aa <+0>:	unknown disassembler error (error = -1)
>
> This doesn't really help much, but that's because there's no way to
> report non memory errors out of the disasembler, because, it was not
> expected that the disassembler would ever report non memory errors.

The patch LGTM since it makes GDB more resilient against the bugs in
opcodes.  Given infinite time, we would fix opcodes, but this looks like
a good compromise.

I'm wondering how you stumbled on that, did you actually get bitten by
opcodes returning -1 but not calling memory_error_func?

Simon