[Lldb-commits] [PATCH] Fixed DataExtractor to correctly display Intel extended doubles.

[Richard Mitton]

Fixed DataExtractor to correctly display Intel extended doubles.

This means that "register read stmm0 --format f" actually works now.

This isn't the best fix, but doing anything better would be hard with the formatting framework LLDB currently has. There are two separate problems at work here:

Firstly, LLDB assumes the host C++ compiler uses IEEE floats for 'float' and 'double', which isn't standard but would probably happen to work most of the time. It also assumes that 'long double' is an IEEE standardized format, which it is not. There are also many points in the code where it assumes the host's float format is the same as the target's (e.g. search for "sizeof(long double)"), which is really bad. A long double can often be 16-bytes on Intel when padding is included, but there are other 16-byte float formats that aren't Intel's (e.g. PPC double-double, IEEE quad, etc).

Secondly, the 'Format' enum used to decide display formats is a little confused over how to decode data. Right now it has options such as 'decimal/octal/float/OSType/CString/etc', which confuses two different ideas; the type of the underlying data and the way to display it. DataExtractor then has to additionally use the byte_size of the item in conjunction with the format and current target in order to know how to actually print it.

It seems to me that this is wrong; I don't believe it's right that DataExtractor should in any way be guessing as to what it's data format is, I think the inputs should all be completely explicit.

I think the best plan would be to make it take as input: a Clang type to say what the data storage format is, a display type ('number/OSType/CString/etc' with no int/float mentioned), and a base ('bin/dec/oct/hex'). So then if somebody wanted to display a C99 hex float, they could just set the base to 'hex'. And if they wanted to e.g. force a float to be displayed as an int instead, they could just pass a different Clang type in.

Having a more flexible format here would also allow for options that are not available on the current codebase, such as printing a uint32_t[] vector as hexadecimal.

Feedback welcome!


http://llvm-reviews.chandlerc.com/D1386

Files:
  source/Core/DataExtractor.cpp
  test/functionalities/register/TestRegisters.py

Index: source/Core/DataExtractor.cpp
===================================================================
--- source/Core/DataExtractor.cpp
+++ source/Core/DataExtractor.cpp
@@ -777,24 +777,12 @@
 long double
 DataExtractor::GetLongDouble (offset_t *offset_ptr) const
 {
-    typedef long double float_type;
-    float_type val = 0.0;
-    const size_t src_size = sizeof(float_type);
-    const float_type *src = (const float_type *)GetData (offset_ptr, src_size);
-    if (src)
-    {
-        if (m_byte_order != lldb::endian::InlHostByteOrder())
-        {
-            const uint8_t *src_data = (const uint8_t *)src;
-            uint8_t *dst_data = (uint8_t *)&val;
-            for (size_t i=0; i<sizeof(float_type); ++i)
-                dst_data[sizeof(float_type) - 1 - i] = src_data[i];
-        }
-        else
-        {
-            val = *src;
-        }
-    }
+    long double val = 0.0;
+#if defined (__i386__) || defined (__amd64__) || defined (__x86_64__) || defined(_M_IX86) || defined(_M_IA64) || defined(_M_X64)
+    *offset_ptr += CopyByteOrderedData (*offset_ptr, 10, &val, sizeof(val), lldb::endian::InlHostByteOrder());
+#else
+    *offset_ptr += CopyByteOrderedData (*offset_ptr, sizeof(val), &val, sizeof(val), lldb::endian::InlHostByteOrder());
+#endif
     return val;
 }
 
@@ -1842,7 +1830,7 @@
                         ss.precision(std::numeric_limits<double>::digits10);
                         ss << GetDouble(&offset);
                     }
-                    else if (item_byte_size == sizeof(long double))
+                    else if (item_byte_size == sizeof(long double) || item_byte_size == 10)
                     {
                         ss.precision(std::numeric_limits<long double>::digits10);
                         ss << GetLongDouble(&offset);
Index: test/functionalities/register/TestRegisters.py
===================================================================
--- test/functionalities/register/TestRegisters.py
+++ test/functionalities/register/TestRegisters.py
@@ -180,6 +180,10 @@
         new_value = "{0x01 0x02 0x03 0x00 0x00 0x00 0x00 0x00 0x09 0x0a 0x2f 0x2f 0x2f 0x2f 0x0e 0x0f}"
         self.vector_write_and_read(currentFrame, "xmm15", new_value, False)
 
+        self.runCmd("register write stmm0 \"{0x00 0x00 0x00 0x00 0x00 0x00 0x40 0x9a 0x09 0x40}\"")
+        self.expect("register read stmm0 --format f",
+            substrs = ['stmm0 = 1234'])
+
         has_avx = False 
         registerSets = currentFrame.GetRegisters() # Returns an SBValueList.
         for registerSet in registerSets:
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D1386.1.patch
Type: text/x-patch
Size: 2574 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20130813/2bd4c867/attachment.bin>