Bug 24457 - X87 FPU Special Purpose Registers

[abhishek.aggarwal]

Summary

• For 'register read --all' command on x86_64-Linux Platform:
  • Provide correct values of X87 FPU Special Purpose Registers
  • Both 32-bit & 64-bit inferiors give correct values on this Platform
• Added a Test Vector:
  • To verify the expected behaviour of the command

Signed-off-by: Abhishek Aggarwal <[e-mail]>

[tfiala]

Overall LGTM. I'd put the offset calculation into an inline function.

It sounds like you gave it a test on a 32-bit process running against a 64-bit OS? (I'm not sure if we're still using the x86_64 register contexts on 32-bit processes running on a 64-bit host these days, but it would be a good check).

source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp

640–642 ↗

(On Diff #33920)

It might be worth putting the offset calculation into either a macro or an inlined function since it looks like we're using it in two places.

[clayborg]

I would suggest making a new member variable: "uint32_t m_fpr_offset;" and filling it in once and using as mentioned in the inline comments.

source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp

563–572 ↗

(On Diff #33920)

Seems like the FPR offset could be stored as a member variable and computed once? Then this would become:

uint8_t *src = (uint8_t *)&m_fpr + reg_info->byte_offset - m_fpr_offset;

640–642 ↗

(On Diff #33920)

Seems like the FPR offset could be stored as a member variable and computed once? Then this would become:

uint8_t *dst = (uint8_t *)&m_fpr + reg_info->byte_offset - m_fpr_offset;

[abhishek.aggarwal]

Created a data member in the class to store byte offset of fctrl

Changed the implementation accordingly

[abhishek.aggarwal]

In D12592#239288, @tfiala wrote:

Overall LGTM. I'd put the offset calculation into an inline function.

It sounds like you gave it a test on a 32-bit process running against a 64-bit OS? (I'm not sure if we're still using the x86_64 register contexts on 32-bit processes running on a 64-bit host these days, but it would be a good check).

Yes. I tested 64-bit inferiors as well as 32-bit inferiors on 64-bit Linux OS. For 32-bit inferior case, we initialize the register offsets with i386 register offsets calculations but we overwrite them with x86_64 register offsets as soon as we figure out that inferiors will be running on x86_64 target system. Hence, this works fine there also.

[tfiala]

LGTM. Thanks for tracking that down!

[clayborg]

Looks good.

lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h

private:
 
         // Private member variables.
         mutable FPRType m_fpr_type;
         FPR m_fpr;
         IOVEC m_iovec;
         YMM m_ymm_set;
         RegInfo m_reg_info;
         uint64_t m_gpr_x86_64[k_num_gpr_registers_x86_64];
+        uint32_t m_fctrl_offset_in_userarea;
 
         // Private member methods.
         bool IsRegisterSetAvailable (uint32_t set_index) const;
 
         bool
         IsGPR(uint32_t reg_index) const;
 
         FPRType

lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp

NativeRegisterContextLinux_x86_64::NativeRegisterContextLinux_x86_64 (const ArchSpec& target_arch,
     // Initialize m_iovec to point to the buffer and buffer size
     // using the conventions of Berkeley style UIO structures, as required
     // by PTRACE extensions.
     m_iovec.iov_base = &m_fpr.xstate.xsave;
     m_iovec.iov_len = sizeof(m_fpr.xstate.xsave);
 
     // Clear out the FPR state.
     ::memset(&m_fpr, 0, sizeof(FPR));
+
+    // Store byte offset of fctrl (i.e. first register of FPR)
+    const RegisterInfo *reg_info_fctrl = GetRegisterInfoByName("fctrl");
+    m_fctrl_offset_in_userarea = reg_info_fctrl->byte_offset;
 }
 
 // CONSIDER after local and llgs debugging are merged, register set support can
 // be moved into a base x86-64 class with IsRegisterSetAvailable made virtual.
 uint32_t
 NativeRegisterContextLinux_x86_64::GetRegisterSetCount () const
 {
     uint32_t sets = 0;
if (reg_info->encoding == lldb::eEncodingVector)
             return error;
         }
 
         error.SetErrorString ("byte order is invalid");
         return error;
     }
 
     // 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(PTRACE_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:
             reg_value.SetUInt16(*(uint16_t *)src);
             break;
         case 4:
             reg_value.SetUInt32(*(uint32_t *)src);
             break;
if (IsFPR(reg_index, GetFPRType()))
                 ::memcpy (m_ymm_set.ymm[reg_index - m_reg_info.first_ymm].bytes, reg_value.GetBytes(), reg_value.GetByteSize());
                 if (!CopyYMMtoXSTATE(reg_index, GetByteOrder()))
                     return Error ("CopyYMMtoXSTATE() failed");
             }
         }
         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(PTRACE_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:
                     *(uint16_t *)dst = reg_value.GetAsUInt16();
                     break;
                 case 4:
                     *(uint32_t *)dst = reg_value.GetAsUInt32();
                     break;

lldb/trunk/test/functionalities/register/Makefile

 LEVEL = ../../make
 
-CXX_SOURCES := main.cpp
+CXX_SOURCES := main.cpp a.cpp
 
 include $(LEVEL)/Makefile.rules

lldb/trunk/test/functionalities/register/TestRegisters.py

class RegisterCommandsTestCase(TestBase):
 
     def test_fp_register_write(self):
         """Test commands that write to registers, in particular floating-point registers."""
         if not self.getArchitecture() in ['amd64', 'i386', 'x86_64']:
             self.skipTest("This test requires x86 or x86_64 as the architecture for the inferior")
         self.buildDefault()
         self.fp_register_write()
 
+    def test_fp_special_purpose_register_read(self):
+        """Test commands that read fpu special purpose registers."""
+        if not self.getArchitecture() in ['amd64', 'i386', 'x86_64']:
+            self.skipTest("This test requires x86 or x86_64 as the architecture for the inferior")
+        self.buildDefault()
+        self.fp_special_purpose_register_read()
+
     def test_register_expressions(self):
         """Test expression evaluation with commands related to registers."""
         if not self.getArchitecture() in ['amd64', 'i386', 'x86_64']:
             self.skipTest("This test requires x86 or x86_64 as the architecture for the inferior")
         self.buildDefault()
         self.register_expressions()
 
     def test_convenience_registers(self):
def vector_write_and_read(self, frame, register, new_value, must_exist = True):
             self.assertTrue(value.IsValid(), "finding a value for register " + register)
         elif not value.IsValid():
             return # If register doesn't exist, skip this test
 
         self.runCmd("register write " + register + " \'" + new_value + "\'")
         self.expect("register read " + register,
             substrs = [register + ' = ', new_value])
 
+    def fp_special_purpose_register_read(self):
+        exe = os.path.join(os.getcwd(), "a.out")
+
+        # Create a target by the debugger.
+        target = self.dbg.CreateTarget(exe)
+        self.assertTrue(target, VALID_TARGET)
+
+        # Find the line number to break inside a.cpp.
+        self.line = line_number('a.cpp', '// Set break point at this line.')
+
+        # Set breakpoint
+        lldbutil.run_break_set_by_file_and_line (self, "a.cpp", self.line, num_expected_locations=1, loc_exact=True)
+
+        # Launch the process, and do not stop at the entry point.
+        self.runCmd ("run", RUN_SUCCEEDED)
+
+        process = target.GetProcess()
+        self.assertTrue(process.GetState() == lldb.eStateStopped,
+                        PROCESS_STOPPED)
+
+        thread = process.GetThreadAtIndex(0)
+        self.assertTrue(thread.IsValid(), "current thread is valid")
+
+        currentFrame = thread.GetFrameAtIndex(0)
+        self.assertTrue(currentFrame.IsValid(), "current frame is valid")
+
+        # Extract the value of fstat and ftag flag at the point just before
+        # we start pushing floating point values on st% register stack
+        value = currentFrame.FindValue("fstat", lldb.eValueTypeRegister)
+        error = lldb.SBError()
+        reg_value_fstat_initial = value.GetValueAsUnsigned(error, 0)
+
+        self.assertTrue(error.Success(), "reading a value for fstat")
+        value = currentFrame.FindValue("ftag", lldb.eValueTypeRegister)
+        error = lldb.SBError()
+        reg_value_ftag_initial = value.GetValueAsUnsigned(error, 0)
+
+        self.assertTrue(error.Success(), "reading a value for ftag")
+        fstat_top_pointer_initial = (reg_value_fstat_initial & 0x3800)>>11
+
+        # Execute 'si' aka 'thread step-inst' instruction 5 times and with
+        # every execution verify the value of fstat and ftag registers
+        for x in range(0,5):
+            # step into the next instruction to push a value on 'st' register stack
+            self.runCmd ("si", RUN_SUCCEEDED)
+
+            # Verify fstat and save it to be used for verification in next execution of 'si' command
+            if not (reg_value_fstat_initial & 0x3800):
+                self.expect("register read fstat",
+                    substrs = ['fstat' + ' = ', str("0x%0.4x" %((reg_value_fstat_initial & ~(0x3800))| 0x3800))])
+                reg_value_fstat_initial = ((reg_value_fstat_initial & ~(0x3800))| 0x3800)
+                fstat_top_pointer_initial = 7
+            else :
+                self.expect("register read fstat",
+                    substrs = ['fstat' + ' = ', str("0x%0.4x" % (reg_value_fstat_initial - 0x0800))])
+                reg_value_fstat_initial = (reg_value_fstat_initial - 0x0800)
+                fstat_top_pointer_initial -= 1
+
+            # Verify ftag and save it to be used for verification in next execution of 'si' command
+            self.expect("register read ftag",
+                substrs = ['ftag' + ' = ', str("0x%0.4x" % (reg_value_ftag_initial | (1<< fstat_top_pointer_initial)))])
+            reg_value_ftag_initial = reg_value_ftag_initial | (1<< fstat_top_pointer_initial)
+
     def fp_register_write(self):
         exe = os.path.join(os.getcwd(), "a.out")
 
         # Create a target by the debugger.
         target = self.dbg.CreateTarget(exe)
         self.assertTrue(target, VALID_TARGET)
 
         lldbutil.run_break_set_by_symbol (self, "main", num_expected_locations=-1)

lldb/trunk/test/functionalities/register/a.cpp

+//===-- a.cpp ------------------------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+#include <stdio.h>
+
+long double
+return_long_double (long double value)
+{
+  float a=2, b=4,c=8, d=16, e=32, f=64, k=128, l=256, add=0;
+__asm__ ( "fld %1 ;"
+  "fld %2 ;"
+  "fld %3 ;"
+  "fld %4 ;"
+  "fld %5 ;"
+  "fld %6 ;"
+  "fld %7 ;"
+  "fadd ;" : "=g" (add) : "g" (a), "g" (b), "g" (c), "g" (d), "g" (e), "g" (f), "g" (k), "g" (l) );  // Set break point at this line.
+  return value;
+}
+
+long double
+outer_return_long_double (long double value)
+{
+  long double val = return_long_double(value);
+  val *= 2 ;
+  return val;
+}
+
+long double
+outermost_return_long_double (long double value)
+{
+  long double val = outer_return_long_double(value);
+  val *= 2 ;
+  return val;
+}

lldb/trunk/test/functionalities/register/main.cpp

 
 #if defined(__linux__)
 #include <sys/prctl.h>
 #endif
 
 #include <chrono>
 #include <thread>
 
+long double outermost_return_long_double (long double my_long_double);
+
 int main (int argc, char const *argv[])
 {
 #if defined(__linux__)
     // Immediately enable any ptracer so that we can allow the stub attach
     // operation to succeed.  Some Linux kernels are locked down so that
     // only an ancestor process can be a ptracer of a process.  This disables that
     // restriction.  Without it, attach-related stub tests will fail.
 #if defined(PR_SET_PTRACER) && defined(PR_SET_PTRACER_ANY)
     // For now we execute on best effort basis.  If this fails for
     // some reason, so be it.
     const int prctl_result = prctl(PR_SET_PTRACER, PR_SET_PTRACER_ANY, 0, 0, 0);
     static_cast<void> (prctl_result);
 #endif
 #endif
 
     char my_string[] = {'a', 'b', 'c', 'd', 'e', 'f', 'g', 0};
     double my_double = 1234.5678;
+    long double my_long_double = 1234.5678;
 
     // For simplicity assume that any cmdline argument means wait for attach.
     if (argc > 1)
     {
         volatile int wait_for_attach=1;
         while (wait_for_attach)
             std::this_thread::sleep_for(std::chrono::microseconds(1));
     }
 
     printf("my_string=%s\n", my_string);
     printf("my_double=%g\n", my_double);
+    outermost_return_long_double (my_long_double);
     return 0;
 }