[Lldb-commits] [PATCH] D92063: [LLDB] RegisterInfoPOSIX_arm64 remove unused bytes from g/G packet

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid added a comment.

In D92063#2417815 <https://reviews.llvm.org/D92063#2417815>, @mgorny wrote:

> In D92063#2416162 <https://reviews.llvm.org/D92063#2416162>, @labath wrote:
>
>> + at mgorny, as he's been navigating these waters lately...
>>
>> So... I presume we can't just slap `__attribute__((packed))` on the structure, because the kernel actually expects that the data structure will have the extra space for the padding. Is that so?
>>
>> Even if we can't, I'm wondering if it wouldn't be cleaner to use two structures for this. Something like:
>>
>>   LLVM_PACKED_START
>>   struct GPR {
>>     // as before...
>>   };
>>   /// Big comment explaining the purpose of padding
>>   struct GPRBuffer: GPR {
>>     uint32_t pad;
>>   };
>>   LLVM_PACKED_END
>>
>> and then using GPR or GPRBuffer accordingly. What do you think?
>
> That would imply adding additional offset field to the register lists, wouldn't it? Not that I'm opposed — it might be reasonable to have the option to override the offset for system structs, coredumps...

Register infos should contain g/G packet offset and Ideally offset calculation should look something like this: reg[index].byte_offset = reg[index - 1].byte_offset + reg[index - 1].byte_size.

In case of AArch64 we are using GPR struct to calculate offsets which I think is inspired from the thinking that offset == ptrace offset rather than the g/G packet offset. Coincidentally ptrace offsets do no interfere with g/G packet offset the way we are calculating them right now except for this pad bytes added at the end.  And I have fixed that anyway in my latest update.


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

https://reviews.llvm.org/D92063