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

Tom Tromey
2023-06-26T18:34:21Z

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