[Lldb-commits] [PATCH] D96458: [LLDB] Add support for Arm64/Linux dynamic register sets

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid added a comment.

In D96458#2613948 <https://reviews.llvm.org/D96458#2613948>, @labath wrote:

> In D96458#2613894 <https://reviews.llvm.org/D96458#2613894>, @omjavaid wrote:
>
>> ConfigureRegisterContext is called only once in the lifetime of a core from RegisterContextCorePOSIX_arm64 constructor. A process registers cannot change during execution which is what made basis of this change. for NativeRegisterContextLinux_arm64 I moved the ConfigureRegisterInfos logic out of configure register context because it will be called again and again during execution. But for RegisterContextCorePOSIX_arm64 we call it once in the constructor so its safe to keep the function.
>
> Ok, I understand what's happening now -- thanks for re-iterating. I have managed to miss the fact that this patch deals with both core file and live register contexts.
>
> In that case, I'd like to go one step further, and remove the "configuration" operation from the `RegisterInfoPOSIX_arm64` class as well (by making it a part of the construction). It should be possible to do that by fetching the information needed to determine the registers before the class is instantiated. For the live context, that could be done inside the `CreateHostNativeRegisterContextLinux` function. For the core file context, we could create a similar factory function as well. I.e. move the construction of `RegisterInfoPOSIX_arm64` from ThreadElfCore into some static function inside `RegisterContextCorePOSIX_arm64` -- this function could examine the core data to determine the registers and construct the appropriate info class.

hmm .. in that case we will have to defer creation of RegisterInfoPOSIX_arm64 as you said above. I will create a function that will allow us to update unique_ptr m_register_info_up in RegisterContextPOSIX_arm64 and also another function for doing the same for m_register_info_interface_up object in NativeRegisterContextRegisterInfo class. Rest will be same as you suggested above.


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

https://reviews.llvm.org/D96458