[Lldb-commits] [PATCH] D89925: [lldb] Fix a regression introduced by D75730

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

In D89925#2346290 <https://reviews.llvm.org/D89925#2346290>, @jasonmolenda wrote:

> So in this patch you're setting the limit to be the size of the range (for someone who did 'disass -s 0x100 -e 0x200' they would be requesting 100 instructions) but the disassembly returned would be limited by the byte range requested, so this works OK.

Not exactly. The patch uses the range of the current symbol when the number of lines to disassemble == 0. You can think of it as just `disass` (even though that goes through a different code path and continued to work).

> The asm(nop) is to guarantee that sum() has at least one instruction?

The `nop` is just there so we can check that the output actually contains the instructions from the current frame. I think a `nop` should work everywhere our test suite runs.

> There's no documentation for the Disassembler ctors where we might mention that the use case is either (1) an AddressRange, or (2) a start address and instruction count, but maybe there should be?

Once converted to a `Range` object it has to be either so I think it's implicit in the API. This API doesn't take a range but a `uint32_t num_instructions` which is expected to give you all the instructions for the current frame/symbol when 0.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89925/new/

https://reviews.llvm.org/D89925