[Lldb-commits] [PATCH] D24764: Refactor NativeRegisterContextLinux_x86_64 code.

[Valentina Giusti via lldb-commits]

valentinagiusti added inline comments.

================
Comment at: source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp:805
@@ -827,2 +804,3 @@
+  if (m_xstate_type == XStateType::Invalid) {
     if (const_cast<NativeRegisterContextLinux_x86_64 *>(this)->ReadFPR().Fail())
       return false;
----------------
valentinagiusti wrote:
> valentinagiusti wrote:
> > labath wrote:
> > > Then I think we should make that non-const as well. I mean, what's the point of making it const if it does actually modify the state of the object. Either that, or implement it in a way that does not modify the state.
> > This involves changing the interface of NativeRegisterContextLinux just because NativeRegisterContextLinux_x86_64 has some specific needs that make GetRegisterCount use a non-const function. Also, I don't have all the hardware to test such a cross-platform change.
> Anyway I guess I can submit a follow-up patch with this, I will do it today ;)
So I have looked into this and actually in "source/Host/common/NativeRegisterContextRegisterInfo.cpp" the method GetRegisterCount() is implemented as const (and the pure virtual method is defined in "include/lldb/Host/common/NativeRegisterContext.h"). Unfortunately I don't see an easy way 1. to make the method non-const or 2. to avoid that GetRegisterSetCount() in NativeRegisterContextLinux_x86_64 needs to call a non const function: in order to get the register set count, it must know if the XState type is XSAVE, and in order to do that it must call ReadFPR(), or directly PtraceWrapper(), which are both non-const.
Could you please tell me what you think is best?
Imho, it would be better to first merge this patch and then clean up the NativeRegisterContext code, but I am open to suggestions.


https://reviews.llvm.org/D24764