[Lldb-commits] [PATCH] D74398: [lldb-server] jThreadsInfo returns stack memory

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.



> Does this approach sound reasonable?

I think this is ok, in principle. It might be nice to include some numbers, like how much this increases the jThreadsInfo packet size vs. how much time or packets does this save us.

> How do we test this?

I am imagining a two-pronged test strategy. One test, where we do a high-level check on the syntax of the packet, and cross-check the consistency of the data against regular memory reads. And another test, which creates "interesting" stack(s) via inline asm, and where we can make stronger assertions about the data returned.

> Is it fine if we do not handle the big-endian and 32-bit word cases? (There we will be basically never generate the frame list.)

We should definitely do 32-bit. It shouldn't be that hard -- just use `process->GetArchitecture().GetAddressByteSize()` instead of `sizeof(addr_t)`. I have a feeling this code might actually work on big-endian (if we assume the lldb-server and inferior have the same endianness), but it may be better to read the data via `DataExtractor::GetAddress`, as that will also handle the 32-bit case for you.

Another case where this won't work is architectures with upward-growing stacks -- but I don't think we need to handle that, as lldb does not even have a way to describe this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74398