Re: [PATCH v1] gdb/DAP Fix disassemble bug

Simon Farre via Gdb-patches
2023-06-26T22:00:43Z

On 6/26/23 20:34, Tom Tromey wrote:
>>>>>> "Simon" == Simon Farre via Gdb-patches <gdb-patches@sourceware.org> writes:
> Simon> Fixes disassembleRequest
> Simon> The field instructionOffset can be negative. Previous patch made it so
> Simon> that sometimes the request got calculated to 0 instructions, when it
> Simon> meant to retrieve disasm for -50 to 0 (current position).
>
> I don't think this will work correctly, because this isn't counting by
> instruction but rather by byte.
>
> instructionOffset is defined in terms of instructions:
>
> Offset (in instructions) to be applied after the byte offset (if any)
> before disassembling. Can be negative.
>
> I must have missed the "negative" note, or maybe I just ignored it
> without documenting that -- since I wonder how it can possibly work. it
> seems to me that on architectures with variable length instructions, you
> can't really disassemble in "reverse" like that.
>
> I guess one idea would be to back up to the previous symbol and start
> disassembling from there. I feel like the TUI did this, though, and ran
> into all kinds of weird corner cases.
>
> Simon> - for elt in arch.disassemble(pc, count=total_count)[skip_insns:]:
>
> I notice now that the current code also neglects this part of the spec:
>
> * An adapter must return exactly this number of instructions - any
> * unavailable instructions should be replaced with an implementation-defined
> * 'invalid instruction' value.
>
> Tom

Then perhaps we should just ignore any and all offsets and *just* care
about count and produce current addr + N instructions and resolve
"invalid value" for the rest.