[Lldb-commits] [PATCH] D91241: [LLDB] Make offset field optional in RegisterInfo packet for Arm64
Pavel Labath via Phabricator via lldb-commits
lldb-commits at lists.llvm.org
Wed Nov 18 05:23:15 PST 2020
labath added a comment.
Yes, this is pretty much what I hoped for. Thanks.
Besides relaxing existing tests, it would be nice to have a positive test that checks for the behavior that we want. Maybe a gdb-client test which checks that the register values are extracted from the appropriate place from within a g packet (send custom target.xml, and a `g` response, and ensure the register values come out what we expect)?
In D91241#2398995 <https://reviews.llvm.org/D91241#2398995>, @omjavaid wrote:
> I have made some updates which are sufficient for current AArch64 scenario where offset field is not present. For ordering registers based on register numbers I think we may skip doing that for now though i intend to come back to it after getting SVE through.
That seems fair.
================
Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:480
0, // byte size
reg_offset, // offset
eEncodingUint, // encoding
----------------
How about we initialize to UINT32_MAX here, and then just don't update the field if the offset is not set.
================
Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:573
+ else
+ reg_info.byte_offset = LLDB_INVALID_INDEX32;
+ } else {
----------------
The use of LLDB_INVALID_INDEX32 is not really correct (this is not an index) and the rest of the code (e.g. line 507) already uses UINT32_MAX.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D91241/new/
https://reviews.llvm.org/D91241
More information about the lldb-commits
mailing list