Skip to content

Commit 14a7c8a

Browse files
authored
[lldb] Use the correct Wasm register context (llvm#162942)
When implementing the WebAssembly Process Plugin, I initially added WasmGDBRemoteRegisterContext as a placeholder in the UnwindWasm TU. After implementing RegisterContextWasm (llvm#151056), I forgot to switch over to the new implementation. This meant we were using the wrong register context for all frames, except frame 0. The register context deals with the virtual registers, so this only becomes an issue when trying to evaluate a `DW_OP_WASM_location`, which is why this went unnoticed. This PR removes the WasmGDBRemoteRegisterContext placeholder and updates UnwindWasm to use RegisterContextWasm instead for all frames. In terms of testing, I considered updating TestWasm but that would require adding even more state to the already complicated GDB stub. This doesn't scale and my focus over the next weeks/months will be coming up with a comprehensive testing strategy that involves running (a subset of the) API tests under a Wasm runtime, which will cover this and much more. rdar://159297244
1 parent fc22b58 commit 14a7c8a

File tree

3 files changed

+7
-20
lines changed

3 files changed

+7
-20
lines changed

lldb/source/Plugins/Process/wasm/RegisterContextWasm.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ using namespace lldb_private::process_gdb_remote;
2222
using namespace lldb_private::wasm;
2323

2424
RegisterContextWasm::RegisterContextWasm(
25-
wasm::ThreadWasm &thread, uint32_t concrete_frame_idx,
25+
ThreadGDBRemote &thread, uint32_t concrete_frame_idx,
2626
GDBRemoteDynamicRegisterInfoSP reg_info_sp)
2727
: GDBRemoteRegisterContext(thread, concrete_frame_idx, reg_info_sp, false,
2828
false) {}

lldb/source/Plugins/Process/wasm/RegisterContextWasm.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#define LLDB_SOURCE_PLUGINS_PROCESS_WASM_REGISTERCONTEXTWASM_H
1111

1212
#include "Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h"
13+
#include "Plugins/Process/gdb-remote/ThreadGDBRemote.h"
1314
#include "ThreadWasm.h"
1415
#include "Utility/WasmVirtualRegisters.h"
1516
#include "lldb/lldb-private-types.h"
@@ -34,7 +35,7 @@ class RegisterContextWasm
3435
: public process_gdb_remote::GDBRemoteRegisterContext {
3536
public:
3637
RegisterContextWasm(
37-
wasm::ThreadWasm &thread, uint32_t concrete_frame_idx,
38+
process_gdb_remote::ThreadGDBRemote &thread, uint32_t concrete_frame_idx,
3839
process_gdb_remote::GDBRemoteDynamicRegisterInfoSP reg_info_sp);
3940

4041
~RegisterContextWasm() override;

lldb/source/Plugins/Process/wasm/UnwindWasm.cpp

Lines changed: 4 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include "UnwindWasm.h"
1010
#include "Plugins/Process/gdb-remote/ThreadGDBRemote.h"
1111
#include "ProcessWasm.h"
12+
#include "RegisterContextWasm.h"
1213
#include "ThreadWasm.h"
1314
#include "lldb/Utility/LLDBLog.h"
1415
#include "lldb/Utility/Log.h"
@@ -18,21 +19,6 @@ using namespace lldb_private;
1819
using namespace process_gdb_remote;
1920
using namespace wasm;
2021

21-
class WasmGDBRemoteRegisterContext : public GDBRemoteRegisterContext {
22-
public:
23-
WasmGDBRemoteRegisterContext(ThreadGDBRemote &thread,
24-
uint32_t concrete_frame_idx,
25-
GDBRemoteDynamicRegisterInfoSP &reg_info_sp,
26-
uint64_t pc)
27-
: GDBRemoteRegisterContext(thread, concrete_frame_idx, reg_info_sp, false,
28-
false) {
29-
// Wasm does not have a fixed set of registers but relies on a mechanism
30-
// named local and global variables to store information such as the stack
31-
// pointer. The only actual register is the PC.
32-
PrivateSetRegisterValue(0, pc);
33-
}
34-
};
35-
3622
lldb::RegisterContextSP
3723
UnwindWasm::DoCreateRegisterContextForFrame(lldb_private::StackFrame *frame) {
3824
if (m_frames.size() <= frame->GetFrameIndex())
@@ -43,9 +29,9 @@ UnwindWasm::DoCreateRegisterContextForFrame(lldb_private::StackFrame *frame) {
4329
ProcessWasm *wasm_process =
4430
static_cast<ProcessWasm *>(thread->GetProcess().get());
4531

46-
return std::make_shared<WasmGDBRemoteRegisterContext>(
47-
*gdb_thread, frame->GetConcreteFrameIndex(),
48-
wasm_process->GetRegisterInfo(), m_frames[frame->GetFrameIndex()]);
32+
return std::make_shared<RegisterContextWasm>(*gdb_thread,
33+
frame->GetConcreteFrameIndex(),
34+
wasm_process->GetRegisterInfo());
4935
}
5036

5137
uint32_t UnwindWasm::DoGetFrameCount() {

0 commit comments

Comments
 (0)