omjavaid added a comment. In D96458#2614101 <https://reviews.llvm.org/D96458#2614101>, @labath wrote:
> In D96458#2614076 <https://reviews.llvm.org/D96458#2614076>, @omjavaid wrote: > >> 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 > > That would make the registers of a RegisterInfoPOSIX_arm64 instance immutable > (which is good), but that function would allow one to mutate the > RegisterContextPOSIX_arm64 class in the middle of its lifetime by swapping > the register info backing it (which is exactly what I'm trying to avoid). I'm > trying to create an apis that is impossible/hard to use incorrectly. I'd like > to avoid this kind of "configuration" functions unless they are really > necessary. I could be wrong, but right now I don't see a reason why they'd be > necessary. We cannot calculated register set configuration untill we create register context. Both RegisterContextPOSIX_arm64 and NativeRegisterContextRegisterInfo are base class objects for their corresponding register contexts and its not trivial to make them immutable without serious refactoring. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96458/new/ https://reviews.llvm.org/D96458 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
