Sure. Thanks a lot for suggesting this. Thanks & Regards Abhishek Aggarwal
From: Zachary Turner [mailto:ztur...@google.com] Sent: Tuesday, October 6, 2015 6:01 PM To: Aggarwal, Abhishek A; lldb-commits@lists.llvm.org Subject: Re: [Lldb-commits] [lldb] r249379 - Bug 25050: X87 FPU Special Purpose Registers I see. In the future in this kind of situation could you mention in the commit message what test this fixes. For example, "This fixes TestRegisters.py on FreeBSD". On Tue, Oct 6, 2015 at 7:59 AM Aggarwal, Abhishek A <abhishek.a.aggar...@intel.com<mailto:abhishek.a.aggar...@intel.com>> wrote: I had submitted a patch for the Bug 24457 which is similar kind of bug but on Linux x86_64 Platform. There, I had submitted a new test to reproduce this issue. The test is already there in public repository. Since, the same test goes for FreeBSD also therefore I didn’t need to write a new one. -Thanks From: Zachary Turner [mailto:ztur...@google.com<mailto:ztur...@google.com>] Sent: Tuesday, October 6, 2015 4:53 PM To: Aggarwal, Abhishek A; lldb-commits@lists.llvm.org<mailto:lldb-commits@lists.llvm.org> Subject: Re: [Lldb-commits] [lldb] r249379 - Bug 25050: X87 FPU Special Purpose Registers Please don't submit CLs like this without a test in the future. This should be testable by writing some assembly to move a value into an FPU register, setting a breakpoint, then querying the register values. Can you submit this as a followup? On Tue, Oct 6, 2015 at 12:05 AM Abhishek Aggarwal via lldb-commits <lldb-commits@lists.llvm.org<mailto:lldb-commits@lists.llvm.org>> wrote: Author: abhishek Date: Tue Oct 6 02:04:03 2015 New Revision: 249379 URL: http://llvm.org/viewvc/llvm-project?rev=249379&view=rev Log: Bug 25050: X87 FPU Special Purpose Registers Summary: - For x86_64-FreeBSD Platform: -- LLDB now provides correct values of X87 FPU Special Purpose Registers like fstat, ftag, fctrl etc.. Signed-off-by: Abhishek Aggarwal <abhishek.a.aggar...@intel.com<mailto:abhishek.a.aggar...@intel.com>> Reviewers: emaste, mikesart, clayborg Subscribers: emaste Differential Revision: http://reviews.llvm.org/D13434 Modified: lldb/trunk/source/Plugins/Process/FreeBSD/RegisterContextPOSIXProcessMonitor_x86.cpp lldb/trunk/source/Plugins/Process/FreeBSD/RegisterContextPOSIXProcessMonitor_x86.h Modified: lldb/trunk/source/Plugins/Process/FreeBSD/RegisterContextPOSIXProcessMonitor_x86.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/FreeBSD/RegisterContextPOSIXProcessMonitor_x86.cpp?rev=249379&r1=249378&r2=249379&view=diff ============================================================================== --- lldb/trunk/source/Plugins/Process/FreeBSD/RegisterContextPOSIXProcessMonitor_x86.cpp (original) +++ lldb/trunk/source/Plugins/Process/FreeBSD/RegisterContextPOSIXProcessMonitor_x86.cpp Tue Oct 6 02:04:03 2015 @@ -58,6 +58,9 @@ RegisterContextPOSIXProcessMonitor_x86_6 lldb_private::RegisterInfoInterface *register_info) : RegisterContextPOSIX_x86(thread, concrete_frame_idx, register_info) { + // Store byte offset of fctrl (i.e. first register of FPR) wrt 'UserArea' + const RegisterInfo *reg_info_fctrl = GetRegisterInfoByName("fctrl"); + m_fctrl_offset_in_userarea = reg_info_fctrl->byte_offset; } ProcessMonitor & @@ -254,8 +257,15 @@ RegisterContextPOSIXProcessMonitor_x86_6 } // Get pointer to m_fpr.xstate.fxsave variable and set the data from it. - assert (reg_info->byte_offset < sizeof(m_fpr)); - uint8_t *src = (uint8_t *)&m_fpr + reg_info->byte_offset; + // Byte offsets of all registers are calculated wrt 'UserArea' structure. + // However, ReadFPR() reads fpu registers {using ptrace(PT_GETFPREGS,..)} + // and stores them in 'm_fpr' (of type FPR structure). To extract values of fpu + // registers, m_fpr should be read at byte offsets calculated wrt to FPR structure. + + // Since, FPR structure is also one of the member of UserArea structure. + // byte_offset(fpu wrt FPR) = byte_offset(fpu wrt UserArea) - byte_offset(fctrl wrt UserArea) + assert ( (reg_info->byte_offset - m_fctrl_offset_in_userarea) < sizeof(m_fpr)); + uint8_t *src = (uint8_t *)&m_fpr + reg_info->byte_offset - m_fctrl_offset_in_userarea; switch (reg_info->byte_size) { case 2: @@ -308,8 +318,15 @@ RegisterContextPOSIXProcessMonitor_x86_6 else { // Get pointer to m_fpr.xstate.fxsave variable and set the data to it. - assert (reg_info->byte_offset < sizeof(m_fpr)); - uint8_t *dst = (uint8_t *)&m_fpr + reg_info->byte_offset; + // Byte offsets of all registers are calculated wrt 'UserArea' structure. + // However, WriteFPR() takes m_fpr (of type FPR structure) and writes only fpu + // registers using ptrace(PT_SETFPREGS,..) API. Hence fpu registers should + // be written in m_fpr at byte offsets calculated wrt FPR structure. + + // Since, FPR structure is also one of the member of UserArea structure. + // byte_offset(fpu wrt FPR) = byte_offset(fpu wrt UserArea) - byte_offset(fctrl wrt UserArea) + assert ( (reg_info->byte_offset - m_fctrl_offset_in_userarea) < sizeof(m_fpr)); + uint8_t *dst = (uint8_t *)&m_fpr + reg_info->byte_offset - m_fctrl_offset_in_userarea; switch (reg_info->byte_size) { case 2: Modified: lldb/trunk/source/Plugins/Process/FreeBSD/RegisterContextPOSIXProcessMonitor_x86.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/FreeBSD/RegisterContextPOSIXProcessMonitor_x86.h?rev=249379&r1=249378&r2=249379&view=diff ============================================================================== --- lldb/trunk/source/Plugins/Process/FreeBSD/RegisterContextPOSIXProcessMonitor_x86.h (original) +++ lldb/trunk/source/Plugins/Process/FreeBSD/RegisterContextPOSIXProcessMonitor_x86.h Tue Oct 6 02:04:03 2015 @@ -91,6 +91,7 @@ protected: private: ProcessMonitor & GetMonitor(); + uint32_t m_fctrl_offset_in_userarea; // Offset of 'fctrl' in 'UserArea' Structure }; #endif _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org<mailto:lldb-commits@lists.llvm.org> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits Intel Deutschland GmbH Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany Tel: +49 89 99 8853-0, www.intel.de<http://www.intel.de> Managing Directors: Christin Eisenschmid, Prof. Dr. Hermann Eul Chairperson of the Supervisory Board: Tiffany Doon Silva Registered Office: Munich Commercial Register: Amtsgericht Muenchen HRB 186928 Intel Deutschland GmbH Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany Tel: +49 89 99 8853-0, www.intel.de Managing Directors: Christin Eisenschmid, Prof. Dr. Hermann Eul Chairperson of the Supervisory Board: Tiffany Doon Silva Registered Office: Munich Commercial Register: Amtsgericht Muenchen HRB 186928
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
