clayborg added inline comments.
================
Comment at: lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp:34
+/// Checks whether the data buffer starts with a valid Wasm module header.
+static bool ValidateModuleHeader(const DataBufferSP &data_sp) {
+ if (!data_sp || data_sp->GetByteSize() < kWasmHeaderSize)
----------------
I typically would put "data_sp" into a DataExtractor and extract a uint32_t and
then check the decoded value with something like:
```
static bool ValidateModuleHeader(DataExtractor &data, uint64_t *offset_ptr) {
auto magic = data.GetU32(offset_ptr);
if (magic == WASM_MAGIC)
return true;
if (magic == WASM_CIGAM) {
// Set byte order in DataExtractor
data.SetByteOrder(data.GetByteOrder() == eByteOrderBig ? eByteOrderLittle :
eByteOrderBig);
return true;
}
return false;
}
```
This function expects a DataExtractor to be passed in that has "data_sp" inside
of it with the host endian set as the byte order. It will set the byte order
correctly. It also expects to have two uint32_t macros defined: WASM_MAGIC and
WASM_CIGAM. These contain the non byte swapped and the byte swapped magic
values. Easy to replace those with real definitions from else where
(llvm::file_magic::wasm_object? Not sure of the type of this though, seemed
like a StringRef).
================
Comment at: lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp:44
+
+ uint32_t version = llvm::support::endian::read32le(Ptr);
+ return version == llvm::wasm::WasmVersion;
----------------
use DataExtractor::GetU32()? Or is the byte order always little endian for wasm
object files?
================
Comment at: lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp:187-195
+ const uint8_t *name_bytes = section_header_data.PeekData(offset,
*name_len);
+ // If a custom section has a name longer than the allocated buffer or
longer
+ // than the data left in the image, ignore this section.
+ if (!name_bytes)
+ return false;
+
+ llvm::StringRef sect_name(reinterpret_cast<const char *>(name_bytes),
----------------
All these lines can use the GetCStr with a length:
```
ConstString sect_name(data.GetCStr(&offset, *name_len));
if (!sect_name)
return false;
```
================
Comment at: lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp:199
+ m_sect_infos.push_back(section_info{*offset_ptr + offset, section_length,
+ *section_id, ConstString(sect_name)});
+ offset += section_length;
----------------
remove ConstString constructor here if we switch code above as suggested.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D71575/new/
https://reviews.llvm.org/D71575
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
