[Lldb-commits] [PATCH] D29078: This patch implements a command to access and manipulate the Intel(R) MPX Boundary Tables.

[Valentina Giusti via Phabricator via lldb-commits]

valentinagiusti marked 2 inline comments as done.
valentinagiusti added a comment.

Hi Greg, thanks a lot for your review. I have a question about the API that you proposed, please have a look at the inline comments.



================
Comment at: tools/intel-mpx/IntelMPXTablePlugin.cpp:143-150
+  if (arch == llvm::Triple::ArchType::x86_64) {
+    bd_entry =
+        toU64(bd_entry_v[7], bd_entry_v[6], bd_entry_v[5], bd_entry_v[4],
+              bd_entry_v[3], bd_entry_v[2], bd_entry_v[1], bd_entry_v[0]);
+  } else {
+    bd_entry =
+        toU32(bd_entry_v[3], bd_entry_v[2], bd_entry_v[1], bd_entry_v[0]);
----------------
clayborg wrote:
> No need to do manual byte order stuff here, we can use SBData and you don't need the "if (arch == ...)" since SBData knows the address byte size:
> 
> ```
> SBError error;
> SBData data;
> data.SetData(error, bd_entry_v.data(), bd_entry_v.size(), target.GetByteOrder(), target.GetAddressByteSize());
> lldb::addr_t bd_entry = data.GetAddress(error, 0);
> ```
Hi Greg, thanks for the tip, but the code that you proposed doesn't work for i386 for me. GetAddress() fails with the error "unable to read data". 
To me it doesn't look like this API is able to handle different arch byte sizes.
In fact, if I leave the "if (arch ==) check and I use GetUnsignedInt32() instead of GetAddress() it works. Is this a bug of GetAddress()?


https://reviews.llvm.org/D29078