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

[Tom Tromey <tom@tromey.com>]

>>>>> "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