[Lldb-commits] [lldb] r249379 - Bug 25050: X87 FPU Special Purpose Registers

Aggarwal, Abhishek A via lldb-commits lldb-commits at lists.llvm.org
Wed Oct 7 00:44:52 PDT 2015


Sure. Thanks a lot for suggesting this.

Thanks & Regards
Abhishek Aggarwal

From: Zachary Turner [mailto:zturner at google.com]
Sent: Tuesday, October 6, 2015 6:01 PM
To: Aggarwal, Abhishek A; lldb-commits at 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.aggarwal at intel.com<mailto:abhishek.a.aggarwal at 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:zturner at google.com<mailto:zturner at google.com>]
Sent: Tuesday, October 6, 2015 4:53 PM
To: Aggarwal, Abhishek A; lldb-commits at lists.llvm.org<mailto:lldb-commits at 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 at lists.llvm.org<mailto:lldb-commits at 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.aggarwal at intel.com<mailto:abhishek.a.aggarwal at 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 at lists.llvm.org<mailto:lldb-commits at 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20151007/093f0979/attachment-0001.html>


More information about the lldb-commits mailing list