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



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to