omjavaid added a comment.

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

> In D91241#2391156 <https://reviews.llvm.org/D91241#2391156>, @omjavaid wrote:
>
>> I guess GDB standard does enforce ascending order, here is what it says 
>> about regnum:
>>
>> "The register’s number. If omitted, a register’s number is one greater than 
>> that of the previous register (either in the current feature or in a 
>> preceding feature); the first register in the target description defaults to 
>> zero. This register number is used to read or write the register; e.g. it is 
>> used in the remote p and P packets, and registers appear in the g and G 
>> packets in order of increasing register number."
>>
>> https://sourceware.org/gdb/current/onlinedocs/gdb/Target-Description-Format.html
>
> I've just read that paragraph before writing that, but that's not how I would 
> interpret it. What I think that says is:
>
> - in the `g` packet, registers appear in the increasing register number 
> order. (I think we agree on that part)
> - **if the register number is omitted**, the registers are assigned 
> increasing register numbers. Here, I think the bold part is very important, 
> as it means the rest of the sentence describes default/fallback behavior. If 
> the stub specifies the register number explicitly, then I'd say it's free to 
> send order the register descriptions any way it likes. It can even combine 
> things and set explicit numbers for some  register, but not others...

I gave thought and looked at GDB handling of g/G packet offsets. 
Important points to note:

1. GDB assigns register numbers if they are not provided, with scheme last 
regnum + 1.
2. GDB maintains registers list in order of their placement in XML regardless 
of any specific register numbers.
3. GDB sends g/G packet based on register numbers in order of increasing 
register numbers. For this gdb creates a sorted list based of register numbers 
and then assigns offsets accordingly.

In LLDB we are trying to support legacy offset field which means offset 
calculation has to be done in location where we parse an XML register entry 
(ParseRegisters) or qRegisterInfo packet 
(ProcessGDBRemote::BuildDynamicRegisterInfo).

Also both of above functions are just inserting into register's information 
list maintained by DynamicRegisterInfo class so there is not enough leverage 
available to make offset calculation in one single location without giving up 
offset field altogether. We may write a helper function to do offset post 
processing for pseudo registers but that will still be a part of 
DynamicRegisterInfo class and may be called from ParseRegisters and 
ProcessGDBRemote::BuildDynamicRegisterInfo.

IMO, lets keep this current patch as it is unless we decide on giving up offset 
field for all architectures.


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