Re: [Lldb-commits] [PATCH] D21328: [lldb] Fixed incorrect endianness when evaluating certain expressions

[Cameron via lldb-commits]

cameron314 added a comment.

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.


https://reviews.llvm.org/D21328



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D21328: [lldb] Fixed incorrect endianness when evaluating certain expressions

[Sean Callanan via lldb-commits]

spyffe requested changes to this revision.
spyffe added a comment.
This revision now requires changes to proceed.

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


https://reviews.llvm.org/D21328



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D21328: [lldb] Fixed incorrect endianness when evaluating certain expressions

[Cameron via lldb-commits]

cameron314 added a comment.

@spyffe, do you have a minute today?


https://reviews.llvm.org/D21328



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D21328: [lldb] Fixed incorrect endianness when evaluating certain expressions

[Cameron via lldb-commits]

cameron314 added a comment.

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


http://reviews.llvm.org/D21328



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D21328: [lldb] Fixed incorrect endianness when evaluating certain expressions

[Cameron via lldb-commits]

cameron314 added a comment.

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


http://reviews.llvm.org/D21328



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D21328: [lldb] Fixed incorrect endianness when evaluating certain expressions

[Greg Clayton via lldb-commits]

clayborg accepted this revision.
clayborg added a comment.
This revision is now accepted and ready to land.

Sean Callanan needs to OK this as well.


http://reviews.llvm.org/D21328



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D21328: [lldb] Fixed incorrect endianness when evaluating certain expressions

[Cameron via lldb-commits]

cameron314 created this revision.
cameron314 added reviewers: spyffe, zturner, clayborg.
cameron314 added a subscriber: lldb-commits.

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 
`0x0101`).

http://reviews.llvm.org/D21328

Files:
  include/lldb/Expression/IRMemoryMap.h
  source/Expression/IRMemoryMap.cpp
  source/Expression/Materializer.cpp

Index: source/Expression/Materializer.cpp
===
--- source/Expression/Materializer.cpp
+++ source/Expression/Materializer.cpp
@@ -575,7 +575,7 @@
 
 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())
 {
Index: source/Expression/IRMemoryMap.cpp
===
--- source/Expression/IRMemoryMap.cpp
+++ source/Expression/IRMemoryMap.cpp
@@ -564,6 +564,22 @@
 }
 
 void
+IRMemoryMap::WriteMemory(lldb::addr_t process_address, const uint8_t *bytes, 
size_t size, lldb::ByteOrder byteOrder, Error &error)
+{
+std::vector 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 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();
Index: include/lldb/Expression/IRMemoryMap.h
===
--- include/lldb/Expression/IRMemoryMap.h
+++ include/lldb/Expression/IRMemoryMap.h
@@ -60,6 +60,7 @@
 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);


Index: source/Expression/Materializer.cpp
===
--- source/Expression/Materializer.cpp
+++ source/Expression/Materializer.cpp
@@ -575,7 +575,7 @@
 
 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())
 {
Index: source/Expression/IRMemoryMap.cpp
===
--- source/Expression/IRMemoryMap.cpp
+++ source/Expression/IRMemoryMap.cpp
@@ -564,6 +564,22 @@
 }
 
 void
+IRMemoryMap::WriteMemory(lldb::addr_t process_address, const uint8_t *bytes, size_t size, lldb::ByteOrder byteOrder, Error &error)
+{
+std::vector 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 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();
Index: include/lldb/Expression/IRMemoryMap.h
===
--- include/lldb/Expression/IRMemoryMap.h
+++ include/lldb/Expression/IRMemoryMap.h
@@ -60,6 +60,7 @@
 void Free (lldb::addr_t process_address, Error &error);
 
 void WriteMemory (lldb::addr_t process_address, const