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

[Simon Farre <simon.farre.cx@gmail.com>]

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.