omjavaid added a comment.

In D91241#2402491 <https://reviews.llvm.org/D91241#2402491>, @labath wrote:

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

Let me take a look into writing a test case for this change.

Moreover, I have tried what you suggested about initializing all register 
offset with LLDB_INVALID_INDEX32 unless an offset is explicitly specified. The 
problem is that once we make that change in ProcessGDBRemote.cpp more changes 
are required elsewhere and many tests start failing. I kept plugging those 
tests with adjustments untill I decided on current solution which required less 
no of changes.

DynamicRegisterInfo::AddRegister also calculated the maximum offset if we take 
maximum offset calculation then it impacts DynamicRegisterInfo::SetRegisterInfo 
which is using Finalize but calculating offsets itself. Also oid 
GDBRemoteDynamicRegisterInfo::HardcodeARMRegisters also use 
DynamicRegisterInfo::AddRegister but calculates offsets by itself.


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

https://reviews.llvm.org/D91241

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to