[lldb] Fixed incorrect endianness when evaluating certain expressions

[cameron314]

Summary

The EntityVariable materializer was, under certain conditions, taking the bytes of a DataExtractor that were potentially in host order (e.g. little endian) and putting them in the IRMemoryMap (which assumes all values are in target order, e.g. big endian). This caused certain values to have the wrong endianness during expression evaluation.

On our platform, this manifested as certain expressions yielding incorrect results when the variables were in registers (e.g. argc + 1 would give 0x01000001).

[clayborg]

Sean Callanan needs to OK this as well.

[cameron314]

@spyffe, do you have a minute to look this over?

[cameron314]

Sorry to bug you again, but @spyffe, could you have a look when you have a sec?

[cameron314]

@spyffe, do you have a minute today?

[spyffe]

This looks fine. How tricky would it be to make a test case for a reproduction scenario?

[cameron314]

Hmm, I don't think this is easily testable at all. We happened to see it on our (out-of-tree) big-endian architecture when debugging remotely on Windows, but only for variables backed by registers. I was thus able to trace the problem back to this code, fix it, and verify it locally. But I can't reproduce this for other architectures because they likely don't follow the same code path (plus I don't have the hardware/simulator setup required). And trying to unit test this would be... quite a challenge, I feel, considering all the layers involved.

include/lldb/Expression/IRMemoryMap.h

lldb::addr_t Malloc (size_t size,
                          uint32_t permissions,
                          AllocationPolicy policy,
                          bool zero_memory,
                          Error &error);
     void Leak (lldb::addr_t process_address, Error &error);
     void Free (lldb::addr_t process_address, Error &error);
     
     void WriteMemory (lldb::addr_t process_address, const uint8_t *bytes, size_t size, Error &error);
+    void WriteMemory (lldb::addr_t process_address, const uint8_t *bytes, size_t size, lldb::ByteOrder order, Error &error);
     void WriteScalarToMemory (lldb::addr_t process_address, Scalar &scalar, size_t size, Error &error);
     void WritePointerToMemory (lldb::addr_t process_address, lldb::addr_t address, Error &error);
     void ReadMemory (uint8_t *bytes, lldb::addr_t process_address, size_t size, Error &error);
     void ReadScalarFromMemory (Scalar &scalar, lldb::addr_t process_address, size_t size, Error &error);
     void ReadPointerFromMemory (lldb::addr_t *address, lldb::addr_t process_address, Error &error);
     bool GetAllocSize(lldb::addr_t address, size_t &size);
     void GetMemoryData (DataExtractor &extractor, lldb::addr_t process_address, size_t size, Error &error);
     

source/Expression/IRMemoryMap.cpp

if (address > al.m_process_start)
         return true;
     }
 
     size = al.m_size;
     return true;
 }
 
 void
+IRMemoryMap::WriteMemory(lldb::addr_t process_address, const uint8_t *bytes, size_t size, lldb::ByteOrder byteOrder, Error &error)
+{
+    std::vector<uint8_t> temp;
+    if (byteOrder != GetByteOrder())
+    {
+        // Need to byte-swap the bytes before putting them in the map, otherwise they'll
+        // be interpreted with the wrong endianness when they're read back out.
+        temp.reserve(size);
+        typedef std::reverse_iterator<const uint8_t *> revit;
+        temp.assign(revit(bytes + size), revit(bytes));
+        bytes = temp.data();
+    }
+    WriteMemory(process_address, bytes, size, error);
+}
+
+void
 IRMemoryMap::WriteMemory (lldb::addr_t process_address, const uint8_t *bytes, size_t size, Error &error)
 {
     error.Clear();
 
     AllocationMap::iterator iter = FindAllocation(process_address, size);
 
     if (iter == m_allocations.end())
     {

source/Expression/Materializer.cpp

void Materialize(lldb::StackFrameSP &frame_sp,
                 if (!alloc_error.Success())
                 {
                     err.SetErrorStringWithFormat("couldn't allocate a temporary region for %s: %s", m_variable_sp->GetName().AsCString(), alloc_error.AsCString());
                     return;
                 }
 
                 Error write_error;
 
-                map.WriteMemory(m_temporary_allocation, data.GetDataStart(), data.GetByteSize(), write_error);
+                map.WriteMemory(m_temporary_allocation, data.GetDataStart(), data.GetByteSize(), data.GetByteOrder(), write_error);
 
                 if (!write_error.Success())
                 {
                     err.SetErrorStringWithFormat("couldn't write to the temporary region for %s: %s", m_variable_sp->GetName().AsCString(), write_error.AsCString());
                     return;
                 }
 
                 Error pointer_write_error;