Skip to content

Commit bb013a4

Browse files
Reland "Revert "[lldb] Fix OP_deref evaluation for large integer resu… (#159482)
…lts (#159460)"" The original had an issue on "AArch-less" bots. Fixed it with some ifdefs around the presence of the AArch ABI plugin. This reverts commit 1a4685d.
1 parent 658bb98 commit bb013a4

File tree

3 files changed

+131
-2
lines changed

3 files changed

+131
-2
lines changed

lldb/source/Expression/DWARFExpression.cpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -909,8 +909,6 @@ static llvm::Error Evaluate_DW_OP_deref(DWARFExpression::Stack &stack,
909909
" for DW_OP_deref",
910910
pointer_addr),
911911
error.takeError());
912-
if (ABISP abi_sp = process->GetABI())
913-
pointer_value = abi_sp->FixCodeAddress(pointer_value);
914912
stack.back().GetScalar() = pointer_value;
915913
stack.back().ClearContext();
916914
} break;

lldb/unittests/Expression/CMakeLists.txt

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,8 @@
1+
if ("AArch64" IN_LIST LLVM_TARGETS_TO_BUILD)
2+
set(LINK_COMPONENTS_AARCH AArch64)
3+
set(LINK_LIBS_AARCH lldbPluginABIAArch64)
4+
endif()
5+
16
add_lldb_unittest(ExpressionTests
27
ClangParserTest.cpp
38
ClangExpressionDeclMapTest.cpp
@@ -6,6 +11,9 @@ add_lldb_unittest(ExpressionTests
611
CppModuleConfigurationTest.cpp
712
ExpressionTest.cpp
813

14+
LINK_COMPONENTS
15+
Support
16+
${LINK_COMPONENTS_AARCH}
917
LINK_LIBS
1018
lldbCore
1119
lldbPluginObjectFileELF
@@ -18,4 +26,9 @@ add_lldb_unittest(ExpressionTests
1826
lldbUtilityHelpers
1927
lldbSymbolHelpers
2028
LLVMTestingSupport
29+
${LINK_LIBS_AARCH}
2130
)
31+
32+
if ("AArch64" IN_LIST LLVM_TARGETS_TO_BUILD)
33+
target_compile_definitions(ExpressionTests PRIVATE ARCH_AARCH64)
34+
endif()

lldb/unittests/Expression/DWARFExpressionTest.cpp

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@
77
//===----------------------------------------------------------------------===//
88

99
#include "lldb/Expression/DWARFExpression.h"
10+
#ifdef ARCH_AARCH64
11+
#include "Plugins/ABI/AArch64/ABISysV_arm64.h"
12+
#endif
1013
#include "Plugins/ObjectFile/wasm/ObjectFileWasm.h"
1114
#include "Plugins/Platform/Linux/PlatformLinux.h"
1215
#include "Plugins/SymbolFile/DWARF/DWARFDebugInfo.h"
@@ -21,10 +24,12 @@
2124
#include "lldb/Core/dwarf.h"
2225
#include "lldb/Host/HostInfo.h"
2326
#include "lldb/Symbol/ObjectFile.h"
27+
#include "lldb/Target/ABI.h"
2428
#include "lldb/Target/RegisterContext.h"
2529
#include "lldb/Utility/RegisterValue.h"
2630
#include "lldb/Utility/StreamString.h"
2731
#include "llvm/ADT/StringExtras.h"
32+
#include "llvm/Support/TargetSelect.h"
2833
#include "llvm/Testing/Support/Error.h"
2934
#include "gtest/gtest.h"
3035

@@ -199,6 +204,26 @@ class DWARFExpressionMockProcessTest : public ::testing::Test {
199204
}
200205
};
201206

207+
struct PlatformTargetDebugger {
208+
lldb::PlatformSP platform_sp;
209+
lldb::TargetSP target_sp;
210+
lldb::DebuggerSP debugger_sp;
211+
};
212+
213+
/// A helper function to create <Platform, Target, Debugger> objects with the
214+
/// "aarch64-pc-linux" ArchSpec.
215+
static PlatformTargetDebugger CreateTarget() {
216+
ArchSpec arch("aarch64-pc-linux");
217+
Platform::SetHostPlatform(
218+
platform_linux::PlatformLinux::CreateInstance(true, &arch));
219+
lldb::PlatformSP platform_sp;
220+
lldb::TargetSP target_sp;
221+
lldb::DebuggerSP debugger_sp = Debugger::CreateInstance();
222+
debugger_sp->GetTargetList().CreateTarget(
223+
*debugger_sp, "", arch, eLoadDependentsNo, platform_sp, target_sp);
224+
return PlatformTargetDebugger{platform_sp, target_sp, debugger_sp};
225+
}
226+
202227
// NB: This class doesn't use the override keyword to avoid
203228
// -Winconsistent-missing-override warnings from the compiler. The
204229
// inconsistency comes from the overriding definitions in the MOCK_*** macros.
@@ -1135,3 +1160,96 @@ TEST_F(DWARFExpressionMockProcessTest, DW_OP_piece_file_addr) {
11351160
ASSERT_EQ(result->GetValueType(), Value::ValueType::HostAddress);
11361161
ASSERT_THAT(result->GetBuffer().GetData(), ElementsAre(0x11, 0x22));
11371162
}
1163+
1164+
/// A Process whose `ReadMemory` override queries a DenseMap.
1165+
struct MockProcessWithMemRead : Process {
1166+
using addr_t = lldb::addr_t;
1167+
1168+
llvm::DenseMap<addr_t, addr_t> memory_map;
1169+
1170+
MockProcessWithMemRead(lldb::TargetSP target_sp, lldb::ListenerSP listener_sp,
1171+
llvm::DenseMap<addr_t, addr_t> &&memory_map)
1172+
: Process(target_sp, listener_sp), memory_map(memory_map) {}
1173+
size_t DoReadMemory(addr_t vm_addr, void *buf, size_t size,
1174+
Status &error) override {
1175+
assert(memory_map.contains(vm_addr));
1176+
assert(size == sizeof(addr_t));
1177+
*reinterpret_cast<addr_t *>(buf) = memory_map[vm_addr];
1178+
return sizeof(addr_t);
1179+
}
1180+
size_t ReadMemory(addr_t addr, void *buf, size_t size,
1181+
Status &status) override {
1182+
return DoReadMemory(addr, buf, size, status);
1183+
}
1184+
bool CanDebug(lldb::TargetSP, bool) override { return true; }
1185+
Status DoDestroy() override { return Status(); }
1186+
llvm::StringRef GetPluginName() override { return ""; }
1187+
void RefreshStateAfterStop() override {}
1188+
bool DoUpdateThreadList(ThreadList &, ThreadList &) override { return false; }
1189+
};
1190+
1191+
class DWARFExpressionMockProcessTestWithAArch
1192+
: public DWARFExpressionMockProcessTest {
1193+
public:
1194+
void SetUp() override {
1195+
DWARFExpressionMockProcessTest::SetUp();
1196+
#ifdef ARCH_AARCH64
1197+
LLVMInitializeAArch64TargetInfo();
1198+
LLVMInitializeAArch64TargetMC();
1199+
ABISysV_arm64::Initialize();
1200+
#endif
1201+
}
1202+
void TearDown() override {
1203+
DWARFExpressionMockProcessTest::TearDown();
1204+
#ifdef ARCH_AARCH64
1205+
ABISysV_arm64::Terminate();
1206+
#endif
1207+
}
1208+
};
1209+
1210+
/// Sets the value of register x22 to "42".
1211+
/// Creates a process whose memory address 42 contains the value
1212+
/// memory[42] = ((0xffULL) << 56) | 0xabcdef;
1213+
/// The expression DW_OP_breg22, 0, DW_OP_deref should produce that same value,
1214+
/// without clearing the top byte 0xff.
1215+
TEST_F(DWARFExpressionMockProcessTestWithAArch, DW_op_deref_no_ptr_fixing) {
1216+
llvm::DenseMap<lldb::addr_t, lldb::addr_t> memory;
1217+
constexpr lldb::addr_t expected_value = ((0xffULL) << 56) | 0xabcdefULL;
1218+
constexpr lldb::addr_t addr = 42;
1219+
memory[addr] = expected_value;
1220+
1221+
PlatformTargetDebugger test_setup = CreateTarget();
1222+
lldb::ProcessSP process_sp = std::make_shared<MockProcessWithMemRead>(
1223+
test_setup.target_sp, Listener::MakeListener("dummy"), std::move(memory));
1224+
auto thread = std::make_shared<MockThread>(*process_sp);
1225+
lldb::RegisterContextSP reg_ctx_sp =
1226+
std::make_shared<MockRegisterContext>(*thread, RegisterValue(addr));
1227+
thread->SetRegisterContext(reg_ctx_sp);
1228+
process_sp->GetThreadList().AddThread(thread);
1229+
1230+
auto evaluate_expr = [&](auto &expr_data) {
1231+
DataExtractor extractor(expr_data, sizeof(expr_data),
1232+
lldb::eByteOrderLittle,
1233+
/*addr_size*/ 8);
1234+
DWARFExpression expr(extractor);
1235+
1236+
ExecutionContext exe_ctx(process_sp);
1237+
llvm::Expected<Value> result = DWARFExpression::Evaluate(
1238+
&exe_ctx, reg_ctx_sp.get(), /*module_sp*/ nullptr, extractor,
1239+
/*unit*/ nullptr, lldb::eRegisterKindLLDB,
1240+
/*initial_value_ptr=*/nullptr,
1241+
/*object_address_ptr=*/nullptr);
1242+
return result;
1243+
};
1244+
1245+
uint8_t expr_reg[] = {DW_OP_breg22, 0};
1246+
llvm::Expected<Value> result_reg = evaluate_expr(expr_reg);
1247+
ASSERT_THAT_EXPECTED(result_reg, llvm::Succeeded());
1248+
ASSERT_EQ(result_reg->GetValueType(), Value::ValueType::LoadAddress);
1249+
ASSERT_EQ(result_reg->GetScalar().ULongLong(), addr);
1250+
1251+
uint8_t expr_deref[] = {DW_OP_breg22, 0, DW_OP_deref};
1252+
llvm::Expected<Value> result_deref = evaluate_expr(expr_deref);
1253+
ASSERT_THAT_EXPECTED(result_deref, llvm::Succeeded());
1254+
ASSERT_EQ(result_deref->GetScalar().ULongLong(), expected_value);
1255+
}

0 commit comments

Comments
 (0)