clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.
There are a few places where you are reading memory and then wanting to decode
data from it. Right now, you first read memory into a local buffer and then we
create an SBData and set the data to the bytes. It would be nice to have the
ability to read memory and get an SBData back:
class SBProcess
{
SBData ReadData(lldb::addr_t addr, size_t size, SBError &error);
};
Since the process can fill in the byte order and address byte size it can
return an SBData that is ready to use. If you are interested in adding this in
this patch, let me know.
================
Comment at: tools/intel-mpx/IntelMPXTablePlugin.cpp:35-36
+
+ std::string ptr_str(cptr);
+ ptr_str.insert(0, "&");
+ lldb::SBValue ptr_addr = frame.GetValueForVariablePath(ptr_str.c_str());
----------------
```
std:string ptr_str("&");
ptr_str += cptr;
```
================
Comment at: tools/intel-mpx/IntelMPXTablePlugin.cpp:47
+
+static inline uint32_t toU32(uint8_t b0, uint8_t b1, uint8_t b2, uint8_t b3) {
+ return ((uint32_t)b0 << 24) | ((uint32_t)b1 << 16) | ((uint32_t)b2 << 8) |
----------------
Remove this and use SBData::GetUnsignedInt32(...) or SBData::GetAddress(...).
================
Comment at: tools/intel-mpx/IntelMPXTablePlugin.cpp:52
+
+static inline uint64_t toU64(uint8_t b0, uint8_t b1, uint8_t b2, uint8_t b3,
+ uint8_t b4, uint8_t b5, uint8_t b6, uint8_t b7) {
----------------
Remove this and use SBData::GetUnsignedInt64(...) or SBData::GetAddress(...).
================
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]);
----------------
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);
```
================
Comment at: tools/intel-mpx/IntelMPXTablePlugin.cpp:199-218
+ if (arch == llvm::Triple::ArchType::x86_64) {
+ lbound = toU64(bt_entry_v[7], bt_entry_v[6], bt_entry_v[5], bt_entry_v[4],
+ bt_entry_v[3], bt_entry_v[2], bt_entry_v[1], bt_entry_v[0]);
+ ubound =
+ ~toU64(bt_entry_v[15], bt_entry_v[14], bt_entry_v[13], bt_entry_v[12],
+ bt_entry_v[11], bt_entry_v[10], bt_entry_v[9], bt_entry_v[8]);
+ value =
----------------
Use SBData and you don't need the "if (arch == ...)" since SBData knows the
address byte size:
```
SBError error;
SBData data;
uint32_t addr_size = target.GetAddressByteSize();
data.SetData(error, bt_entry_v.data(), bt_entry_v.size(),
target.GetByteOrder(), addr_size);
lldb::addr_t lbound = data.GetAddress(error, 0 * addr_size);
lldb::addr_t ubound = data.GetAddress(error, 1 * addr_size);
uint64_t value = data.GetAddress(error, 2 * addr_size);
uint64_t meta = data.GetAddress(error, 3 * addr_size);
```
================
Comment at: tools/intel-mpx/IntelMPXTablePlugin.cpp:313-314
+
+ lldb::SBData bndcfgu_data = bndcfgu_val.GetData();
+ bndcfgu = bndcfgu_data.GetUnsignedInt64(error, 0);
+ if (!error.Success()) {
----------------
No need to use SBData here, the SBValue can provide what you need. You use
SBValue::GetValueAsUnsigned() and specify the invalid value to return as the
argument (zero for nullptr):
```
bndcfgu = bndcfgu_val.GetValueAsUnsigned(0);
```
================
Comment at: tools/intel-mpx/IntelMPXTablePlugin.cpp:354
+ llvm::Triple::ArchType arch;
+ lldb::SBFrame frame;
+ lldb::SBError error;
----------------
No need for the "frame" variable as it is only used within GetInitInfo. Remove
this?
================
Comment at: tools/intel-mpx/IntelMPXTablePlugin.cpp:359
+
+ if (!GetInitInfo(debugger, target, arch, frame, bndcfgu, arg, ptr,
result,
+ error))
----------------
I would either extract the target here before passing to any other functions
like this one and only pass the target. There is no need to pass both the
debugger and the target since you can do "target.GetDebugger()" anytime you
need the debugger from the target. You can also remove the "frame" variable
from this since it is only used in GetInitInfo().
https://reviews.llvm.org/D29078
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
