[Lldb-commits] [PATCH] D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging

paolosev updated this revision to Diff 234471. paolosev retitled this revision from "[LLDB] Add initial support for WebAssembly debugging" to "[LLDB] Add ObjectFileWasm plugin for WebAssembly debugging". paolosev edited the summary of this revision. CHANGES SINCE LAST ACTION https://reviews.ll

[Lldb-commits] [PATCH] D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging

paolosev updated this revision to Diff 234474. paolosev set the repository for this revision to rG LLVM Github Monorepo. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71575/new/ https://reviews.llvm.org/D71575 Files: lldb/include/lldb/Utility/Arch

[Lldb-commits] [PATCH] D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging

aprantl added inline comments. Comment at: lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp:45 + +/// Reads a LEB128 variable-length unsigned integer, limited to 7 bits. +llvm::Optional GetVaruint7(DataExtractor §ion_header_data, The LLVM coding style reque

[Lldb-commits] [PATCH] D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging

paolosev updated this revision to Diff 234920. paolosev marked 7 inline comments as done. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71575/new/ https://reviews.llvm.org/D71575 Files: lldb/include/lldb/Utility/ArchSpec.h lldb/source/API/System

[Lldb-commits] [PATCH] D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging

paolosev added inline comments. Comment at: lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp:45 + +/// Reads a LEB128 variable-length unsigned integer, limited to 7 bits. +llvm::Optional GetVaruint7(DataExtractor §ion_header_data, aprantl wrote: > The LLVM

[Lldb-commits] [PATCH] D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging

aprantl added inline comments. Comment at: lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp:45 + +/// Reads a LEB128 variable-length unsigned integer, limited to 7 bits. +llvm::Optional GetVaruint7(DataExtractor §ion_header_data, paolosev wrote: > aprantl w

[Lldb-commits] [PATCH] D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging

labath added inline comments. Comment at: lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp:40 + + const uint32_t *version = reinterpret_cast( + data_sp->GetBytes() + sizeof(llvm::wasm::WasmMagic)); This looks like it will cause problems on big endian

[Lldb-commits] [PATCH] D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging

labath added a comment. A bunch more comments from me. :) A higher level question I have is whether there's something suitable within llvm for parsing wasm object files that could be reused. I know this can be tricky with files loaded from memory etc., so it's fine if it isn't possible to do t

[Lldb-commits] [PATCH] D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging

paolosev marked 10 inline comments as done. paolosev added a comment. In D71575#1793791 , @labath wrote: > A bunch more comments from me. :) > > A higher level question I have is whether there's something suitable within > llvm for parsing wasm object fil

[Lldb-commits] [PATCH] D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging

paolosev added inline comments. Comment at: lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp:46-63 +llvm::Optional GetVaruint7(DataExtractor §ion_header_data, +lldb::offset_t *offset_ptr) { + lldb::offset_t initial_offset = *offset_ptr;

[Lldb-commits] [PATCH] D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging

paolosev updated this revision to Diff 236237. paolosev marked 8 inline comments as done. paolosev added a comment. Addressed more review comments: - removed code to manage "external_debug_info" sections; logic for this will be implemented in the symbol vendor code. - modified test code, from un

[Lldb-commits] [PATCH] D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging

clayborg added a comment. I would suggest removing GetVaruint7 and GetVaruint32 and adding "llvm::Optional DataExtractor::GetULEB128(uint64_t *offset_ptr, uint64_t max_value);" as mentioned in inlined comments. Comment at: lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cp

[Lldb-commits] [PATCH] D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging

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->GetByteSi

[Lldb-commits] [PATCH] D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging

labath added a comment. In D71575#1803321 , @paolosev wrote: > In D71575#1793791 , @labath wrote: > > > A bunch more comments from me. :) > > > > A higher level question I have is whether there's something suitable

[Lldb-commits] [PATCH] D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging

paolosev added inline comments. Comment at: lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp:49 + lldb::offset_t initial_offset = *offset_ptr; + uint64_t value = section_header_data.GetULEB128(offset_ptr); + if (*offset_ptr == initial_offset || value > 127) -

[Lldb-commits] [PATCH] D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging

paolosev updated this revision to Diff 236868. paolosev marked 20 inline comments as done. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71575/new/ https://reviews.llvm.org/D71575 Files: lldb/include/lldb/Utility/ArchSpec.h lldb/include/lldb/Uti

[Lldb-commits] [PATCH] D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging

labath added inline comments. Comment at: lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp:342-343 +section_type, // Section type. +sect_info.offset & 0x, // VM address. +sect_i

[Lldb-commits] [PATCH] D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging

paolosev marked 2 inline comments as done. paolosev added inline comments. Comment at: lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp:342-343 +section_type, // Section type. +sect_info.offset & 0x, /

[Lldb-commits] [PATCH] D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging

paolosev marked an inline comment as done. paolosev added a comment. I apologize for the noob question, but how do I schedule a build for this diff with Harbormaster? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71575/new/ https://reviews.llvm.or

[Lldb-commits] [PATCH] D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging

labath added a comment. Sorry for the delay. I was trying to figure out whether I want to get into the whole DataExtractor discussion or not -- I eventually did... :/ Besides that bit, I think this is looking good.. Comment at: lldb/source/Plugins/ObjectFile/wasm/ObjectFileWa

[Lldb-commits] [PATCH] D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging

labath added a subscriber: merge_guards_bot. labath added a comment. In D71575#1814658 , @paolosev wrote: > I apologize for the noob question, but how do I schedule a build for this > diff with Harbormaster? Harbormaster is a red herring. There's no aut

[Lldb-commits] [PATCH] D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging

paolosev updated this revision to Diff 238168. paolosev marked 2 inline comments as done. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71575/new/ https://reviews.llvm.org/D71575 Files: lldb/include/lldb/Utility/ArchSpec.h lldb/source/API/System

[Lldb-commits] [PATCH] D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging

paolosev added inline comments. Comment at: lldb/source/Utility/DataExtractor.cpp:914 +/// Extract an unsigned LEB128 number with a specified max value. If the +/// extracted value exceeds "max_value" the offset will be left unchanged and +/// llvm::None will be returned. ---

[Lldb-commits] [PATCH] D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Thanks. I think this is looking very good now. Excited to have this ready. Do you have commit access? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.o

[Lldb-commits] [PATCH] D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging

paolosev added a comment. In D71575#1821312 , @labath wrote: > Thanks. I think this is looking very good now. Excited to have this ready. > > Do you have commit access? No, I certainly don't have commit access, this would be my first accepted patch. :)

[Lldb-commits] [PATCH] D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging

This revision was automatically updated to reflect the committed changes. Closed by commit rG4bafceced6a7: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging (authored by paolosev, committed by dschuff). Changed prior to commit: https://reviews.llvm.org/D71575?vs=238168&id=238391#toc R

[Lldb-commits] [PATCH] D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging

jingham added a comment. BTW, I had to fix this patch (cd9e5c32302cd3b34b796683eedb072c6a1cfdc1 ) to build on macOS. uint64_t and size_t are differently spelled (though I think otherwise equivalent.) One is "long long unsign

[Lldb-commits] [PATCH] D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging

paolosev added a comment. In D71575#1823252 , @jingham wrote: > BTW, I had to fix this patch (cd9e5c32302cd3b34b796683eedb072c6a1cfdc1 > ) to > build on macOS. uint64_t and size_t are

[Lldb-commits] [PATCH] D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging

labath added a comment. In D71575#1822343 , @paolosev wrote: > In D71575#1821312 , @labath wrote: > > > Do you have commit access? > > > No, I certainly don't have commit access, this would be my first accepted > p