labath added a comment.
Thanks. Could you also add the other kind of test (the one inline asm) I
mentioned. In an ideal world we'd have a test case for every boundary
condition, but we're pretty far from that right now. Even so, one test case
like that would be nice.
I am imagining the inferior doing something like:
movabsq $0xdead, %rax
pushq %rax ; fake pc
leaq 0x1000(%rsp), %rbp ; larger than kMaxFrameSize
pushq %rbp
movq %rsp, %rbp
pushq $1 ; fake frame contents
pushq $2
pushq $3
incq %rax
push %rax; second fake pc
pushq %rbp
movq %rsp, %rbp
pushq $4 ; fake frame contents
pushq $5
pushq $6
int3
and then the test would assert that the result contains the entirety of the
first fake frame, the bp+pc of the second fake frame, and then would stop due
to hitting the kMaxFrameSize boundary.
================
Comment at:
lldb/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteThreadsInStopReply.py:373
+ # Read memory chunks from jThreadsInfo.
+ memory_chunks = self.gather_threads_info_memory()
+ # Check the chunks are correct.
----------------
Also assert that you have at least one chunk here?
================
Comment at:
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:501-504
+static void AddMemoryChunk(json::Array &stack_memory_chunks, addr_t address,
+ std::vector<int8_t> &bytes) {
+ if (bytes.empty())
+ return;
----------------
I think it would be cleaner if this returned the json::Object as a return value
(much like `GetStackMemoryAsJSON` returns a json::Array).
================
Comment at:
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:534
+ } else {
+ return stack_memory_chunks;
+ }
----------------
You will need to "handle" the error inside the Expected object here. The object
asserts (at runtime) that you do that at the right time.
(http://llvm.org/docs/ProgrammersManual.html#recoverable-errors).
That said, if you're not going to do anything with the error, then maybe the
GetRegisterValue doesn't really need to return an Expected (maybe
Optional<RegisterValue>)
================
Comment at:
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:548
+ static_cast<size_t>(fp_value - sp_value));
+ std::vector<int8_t> buf(byte_count, 0);
+
----------------
we usually use arrays of *u*int8_t to represent uninterpreted data.
================
Comment at:
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:578
+ lldb_private::DataExtractor extractor(fp_ra_buf.data(), fp_and_ra_size,
+ endian::InlHostByteOrder(),
+ address_size);
----------------
It is probably going to be the same, but you might as well use
`process.GetArchitecture().GetByteOrder()`, since you have it handy.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D74398/new/
https://reviews.llvm.org/D74398
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
