Skip to content

Commit dc29048

Browse files
JDevlieghereJDevlieghere
committed
Address review feedback
1 parent fe6624e commit dc29048

File tree

4 files changed

+119
-70
lines changed

4 files changed

+119
-70
lines changed

‎lldb/source/Expression/DWARFExpression.cpp

Lines changed: 0 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -2301,36 +2301,6 @@ llvm::Expected<Value> DWARFExpression::Evaluate(
23012301
break;
23022302
}
23032303

2304-
case DW_OP_WASM_location: {
2305-
uint8_t wasm_op = opcodes.GetU8(&offset);
2306-
uint32_t index;
2307-
2308-
/* LLDB doesn't have an address space to represents WebAssembly locals,
2309-
* globals and operand stacks.
2310-
* We encode these elements into virtual registers:
2311-
* | tag: 2 bits | index: 30 bits |
2312-
* where tag is:
2313-
* 0: Not a WebAssembly ___location
2314-
* 1: Local
2315-
* 2: Global
2316-
* 3: Operand stack value
2317-
*/
2318-
if (wasm_op == 3) {
2319-
index = opcodes.GetU32(&offset);
2320-
wasm_op = 2; // Global
2321-
} else {
2322-
index = opcodes.GetULEB128(&offset);
2323-
}
2324-
2325-
reg_num = (((wasm_op + 1) & 0x03) << 30) | (index & 0x3fffffff);
2326-
2327-
if (llvm::Error error =
2328-
ReadRegisterValueAsScalar(reg_ctx, reg_kind, reg_num, tmp))
2329-
return std::move(error);
2330-
stack.push_back(tmp);
2331-
break;
2332-
}
2333-
23342304
default:
23352305
if (dwarf_cu) {
23362306
if (dwarf_cu->ParseVendorDWARFOpcode(op, opcodes, offset, reg_ctx,

‎lldb/source/Plugins/SymbolFile/DWARF/SymbolFileWasm.cpp

Lines changed: 39 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,14 @@
77
//===----------------------------------------------------------------------===//
88

99
#include "SymbolFileWasm.h"
10+
#include "Plugins/SymbolFile/DWARF/LogChannelDWARF.h"
11+
#include "Utility/WasmVirtualRegisters.h"
12+
#include "lldb/Utility/LLDBLog.h"
1013

1114
using namespace lldb;
1215
using namespace lldb_private;
1316
using namespace lldb_private::plugin::dwarf;
17+
1418
SymbolFileWasm::SymbolFileWasm(ObjectFileSP objfile_sp,
1519
SectionList *dwo_section_list)
1620
: SymbolFileDWARF(objfile_sp, dwo_section_list) {}
@@ -21,17 +25,15 @@ lldb::offset_t
2125
SymbolFileWasm::GetVendorDWARFOpcodeSize(const DataExtractor &data,
2226
const lldb::offset_t data_offset,
2327
const uint8_t op) const {
24-
if (op != llvm::dwarf::DW_OP_WASM_location) {
28+
if (op != llvm::dwarf::DW_OP_WASM_location)
2529
return LLDB_INVALID_OFFSET;
26-
}
2730

2831
lldb::offset_t offset = data_offset;
29-
uint8_t wasm_op = data.GetU8(&offset);
30-
if (wasm_op == 3) {
32+
const uint8_t wasm_op = data.GetU8(&offset);
33+
if (wasm_op == eWasmTagOperandStack)
3134
data.GetU32(&offset);
32-
} else {
35+
else
3336
data.GetULEB128(&offset);
34-
}
3537
return offset - data_offset;
3638
}
3739

@@ -41,37 +43,48 @@ bool SymbolFileWasm::ParseVendorDWARFOpcode(uint8_t op,
4143
RegisterContext *reg_ctx,
4244
lldb::RegisterKind reg_kind,
4345
std::vector<Value> &stack) const {
44-
if (op != llvm::dwarf::DW_OP_WASM_location) {
46+
if (op != llvm::dwarf::DW_OP_WASM_location)
4547
return false;
46-
}
4748

48-
uint8_t wasm_op = opcodes.GetU8(&offset);
49+
uint32_t index = 0;
50+
uint8_t tag = eWasmTagNotAWasmLocation;
4951

50-
/* LLDB doesn't have an address space to represents WebAssembly locals,
51-
* globals and operand stacks.
52-
* We encode these elements into virtual registers:
53-
* | tag: 2 bits | index: 30 bits |
54-
* where tag is:
55-
* 0: Not a WebAssembly ___location
56-
* 1: Local
57-
* 2: Global
58-
* 3: Operand stack value
59-
*/
60-
uint32_t index;
61-
if (wasm_op == 3) {
62-
index = opcodes.GetU32(&offset);
63-
wasm_op = 2; // Global
64-
} else {
52+
/// |DWARF Location Index | WebAssembly Construct |
53+
/// |---------------------|-----------------------|
54+
/// |0 | Local |
55+
/// |1 or 3 | Global |
56+
/// |2 | Operand Stack |
57+
const uint8_t wasm_op = opcodes.GetU8(&offset);
58+
switch (wasm_op) {
59+
case 0: // LOCAL
6560
index = opcodes.GetULEB128(&offset);
61+
tag = eWasmTagLocal;
62+
break;
63+
case 1: // GLOBAL_FIXED
64+
index = opcodes.GetULEB128(&offset);
65+
tag = eWasmTagGlobal;
66+
break;
67+
case 2: // OPERAND_STACK
68+
index = opcodes.GetULEB128(&offset);
69+
tag = eWasmTagOperandStack;
70+
break;
71+
case 3: // GLOBAL_RELOC
72+
index = opcodes.GetU32(&offset);
73+
tag = eWasmTagGlobal;
74+
break;
75+
default:
76+
return false;
6677
}
6778

68-
uint32_t reg_num = (((wasm_op + 1) & 0x03) << 30) | (index & 0x3fffffff);
79+
const uint32_t reg_num = GetWasmRegister(tag, index);
6980

7081
Value tmp;
7182
llvm::Error error = DWARFExpression::ReadRegisterValueAsScalar(
7283
reg_ctx, reg_kind, reg_num, tmp);
73-
if (error)
84+
if (error) {
85+
LLDB_LOG_ERROR(GetLog(DWARFLog::DebugInfo), std::move(error), "{0}");
7486
return false;
87+
}
7588

7689
stack.push_back(tmp);
7790
return true;

‎lldb/source/Utility/WasmVirtualRegisters.h

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
//===----------------------------------------------------------------------===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#ifndef LLDB_SOURCE_UTILITY_WASM_VIRTUAL_REGISTERS_H
10+
#define LLDB_SOURCE_UTILITY_WASM_VIRTUAL_REGISTERS_H
11+
12+
#include "lldb/lldb-private.h"
13+
14+
namespace lldb_private {
15+
16+
// LLDB doesn't have an address space to represents WebAssembly locals,
17+
// globals and operand stacks. We encode these elements into virtual
18+
// registers:
19+
//
20+
// | tag: 2 bits | index: 30 bits |
21+
//
22+
// Where tag is:
23+
// 0: Not a Wasm ___location
24+
// 1: Local
25+
// 2: Global
26+
// 3: Operand stack value
27+
enum WasmVirtualRegisterKinds {
28+
eWasmTagNotAWasmLocation = 0,
29+
eWasmTagLocal = 1,
30+
eWasmTagGlobal = 2,
31+
eWasmTagOperandStack = 3,
32+
};
33+
34+
static const uint32_t kWasmVirtualRegisterTagMask = 0x03;
35+
static const uint32_t kWasmVirtualRegisterIndexMask = 0x3fffffff;
36+
static const uint32_t kWasmVirtualRegisterTagShift = 30;
37+
38+
inline uint32_t GetWasmVirtualRegisterTag(size_t reg) {
39+
return (reg >> kWasmVirtualRegisterTagShift) & kWasmVirtualRegisterTagMask;
40+
}
41+
42+
inline uint32_t GetWasmVirtualRegisterIndex(size_t reg) {
43+
return reg & kWasmVirtualRegisterIndexMask;
44+
}
45+
46+
inline uint32_t GetWasmRegister(uint8_t tag, uint32_t index) {
47+
return ((tag & kWasmVirtualRegisterTagMask) << kWasmVirtualRegisterTagShift) |
48+
(index & kWasmVirtualRegisterIndexMask);
49+
}
50+
51+
} // namespace lldb_private
52+
53+
#endif

‎lldb/unittests/Expression/DWARFExpressionTest.cpp

Lines changed: 27 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -39,15 +39,20 @@ struct MockProcess : Process {
3939
: Process(target_sp, listener_sp) {}
4040

4141
llvm::StringRef GetPluginName() override { return "mock process"; }
42+
4243
bool CanDebug(lldb::TargetSP target, bool plugin_specified_by_name) override {
4344
return false;
4445
};
46+
4547
Status DoDestroy() override { return {}; }
48+
4649
void RefreshStateAfterStop() override {}
50+
4751
bool DoUpdateThreadList(ThreadList &old_thread_list,
4852
ThreadList &new_thread_list) override {
4953
return false;
5054
};
55+
5156
size_t DoReadMemory(lldb::addr_t vm_addr, void *buf, size_t size,
5257
Status &error) override {
5358
for (size_t i = 0; i < size; ++i)
@@ -59,15 +64,18 @@ struct MockProcess : Process {
5964

6065
class MockThread : public Thread {
6166
public:
62-
MockThread(Process &process) : Thread(process, 1 /* tid */), m_reg_ctx_sp() {}
67+
MockThread(Process &process) : Thread(process, /*tid=*/1), m_reg_ctx_sp() {}
6368
~MockThread() override { DestroyThread(); }
6469

6570
void RefreshStateAfterStop() override {}
71+
6672
lldb::RegisterContextSP GetRegisterContext() override { return m_reg_ctx_sp; }
73+
6774
lldb::RegisterContextSP
6875
CreateRegisterContextForFrame(StackFrame *frame) override {
6976
return m_reg_ctx_sp;
7077
}
78+
7179
bool CalculateStopInfo() override { return false; }
7280

7381
void SetRegisterContext(lldb::RegisterContextSP reg_ctx_sp) {
@@ -85,27 +93,32 @@ class MockRegisterContext : public RegisterContext {
8593
m_reg_value(reg_value) {}
8694

8795
void InvalidateAllRegisters() override {}
96+
8897
size_t GetRegisterCount() override { return 0; }
98+
8999
const RegisterInfo *GetRegisterInfoAtIndex(size_t reg) override {
90-
if (reg == 0x4000002a) {
91-
return &m_reg_info;
92-
}
93-
return &m_reg_info; /* nullptr; */
100+
return &m_reg_info;
94101
}
102+
95103
size_t GetRegisterSetCount() override { return 0; }
104+
96105
const RegisterSet *GetRegisterSet(size_t reg_set) override { return nullptr; }
106+
97107
lldb::ByteOrder GetByteOrder() override {
98108
return lldb::ByteOrder::eByteOrderLittle;
99109
}
110+
100111
bool ReadRegister(const RegisterInfo *reg_info,
101112
RegisterValue &reg_value) override {
102113
reg_value = m_reg_value;
103114
return true;
104115
}
116+
105117
bool WriteRegister(const RegisterInfo *reg_info,
106118
const RegisterValue &reg_value) override {
107119
return false;
108120
}
121+
109122
uint32_t ConvertRegisterKindToRegisterNumber(lldb::RegisterKind kind,
110123
uint32_t num) override {
111124
return num;
@@ -127,8 +140,8 @@ static llvm::Expected<Scalar> Evaluate(llvm::ArrayRef<uint8_t> expr,
127140

128141
llvm::Expected<Value> result = DWARFExpression::Evaluate(
129142
exe_ctx, reg_ctx, module_sp, extractor, unit, lldb::eRegisterKindLLDB,
130-
/*initial_value_ptr*/ nullptr,
131-
/*object_address_ptr*/ nullptr);
143+
/*initial_value_ptr=*/nullptr,
144+
/*object_address_ptr=*/nullptr);
132145
if (!result)
133146
return result.takeError();
134147

@@ -823,7 +836,7 @@ TEST(DWARFExpression, Extensions) {
823836
SubsystemRAII<FileSystem, HostInfo, ObjectFileWasm, SymbolVendorWasm>
824837
subsystems;
825838

826-
// Set up a wasm target
839+
// Set up a wasm target.
827840
ArchSpec arch("wasm32-unknown-unknown-wasm");
828841
lldb::PlatformSP host_platform_sp =
829842
platform_linux::PlatformLinux::CreateInstance(true, &arch);
@@ -1022,7 +1035,7 @@ TEST(DWARFExpression, ExtensionsSplitSymbols) {
10221035
SubsystemRAII<FileSystem, HostInfo, ObjectFileWasm, SymbolVendorWasm>
10231036
subsystems;
10241037

1025-
// Set up a wasm target
1038+
// Set up a wasm target.
10261039
ArchSpec arch("wasm32-unknown-unknown-wasm");
10271040
lldb::PlatformSP host_platform_sp =
10281041
platform_linux::PlatformLinux::CreateInstance(true, &arch);
@@ -1090,12 +1103,12 @@ TEST_F(DWARFExpressionMockProcessTest, DW_OP_piece_file_addr) {
10901103
uint8_t expr[] = {DW_OP_addr, 0x40, 0x0, 0x0, 0x0, DW_OP_piece, 1,
10911104
DW_OP_addr, 0x50, 0x0, 0x0, 0x0, DW_OP_piece, 1};
10921105
DataExtractor extractor(expr, sizeof(expr), lldb::eByteOrderLittle,
1093-
/*addr_size*/ 4);
1106+
/*addr_size=*/4);
10941107
llvm::Expected<Value> result = DWARFExpression::Evaluate(
1095-
&exe_ctx, /*reg_ctx*/ nullptr, /*module_sp*/ {}, extractor,
1096-
/*unit*/ nullptr, lldb::eRegisterKindLLDB,
1097-
/*initial_value_ptr*/ nullptr,
1098-
/*object_address_ptr*/ nullptr);
1108+
&exe_ctx, /*reg_ctx=*/nullptr, /*module_sp=*/{}, extractor,
1109+
/*unit=*/nullptr, lldb::eRegisterKindLLDB,
1110+
/*initial_value_ptr=*/nullptr,
1111+
/*object_address_ptr=*/nullptr);
10991112

11001113
ASSERT_THAT_EXPECTED(result, llvm::Succeeded());
11011114
ASSERT_EQ(result->GetValueType(), Value::ValueType::HostAddress);

0 commit comments

Comments
 (0)