[lldb][RISCV] function calls support in lldb expressions #99336
Conversation
|
@llvm/pr-subscribers-lldb Author: None (dlav-sc) ChangesTo make function calls inside lldb expressions ABI support, JIT engine support are required. This patch augments corresponding functionality to RISCV ABI and implements RISCV relocation resolver in JIT, which allows to make function calls with integer and void function arguments and return value. Also it adds RISCV specific DirectToIndirectFunctionCallsReplacement IR pass, that allows to make assembly jumps at any 64bit address without RISCV large code model, which has not been implemented yet. This pass is needed, because jitted code contains more that +-2GB jumps, which are not available in RISCV without large code model now. Fixed tests: Patch is 35.55 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/99336.diff 15 Files Affected:
diff --git a/lldb/include/lldb/Core/Architecture.h b/lldb/include/lldb/Core/Architecture.h
index b6fc1a20e1e69..ca6a9207d8012 100644
--- a/lldb/include/lldb/Core/Architecture.h
+++ b/lldb/include/lldb/Core/Architecture.h
@@ -12,6 +12,7 @@
#include "lldb/Core/PluginInterface.h"
#include "lldb/Target/DynamicRegisterInfo.h"
#include "lldb/Target/MemoryTagManager.h"
+#include "llvm/IR/LegacyPassManager.h"
namespace lldb_private {
@@ -129,6 +130,17 @@ class Architecture : public PluginInterface {
RegisterContext ®_context) const {
return false;
}
+
+ // Takes a Pass Manager and adds passes for this Architecture that should be
+ // run before IRForTarget
+ virtual std::unique_ptr<llvm::legacy::PassManager>
+ GetArchitectureCustomPasses(const ExecutionContext &exe_ctx,
+ const llvm::StringRef expr) const {
+ return nullptr;
+ }
+
+ static constexpr llvm::StringLiteral s_target_incompatibility_marker =
+ "target_incompatibility_detected";
};
} // namespace lldb_private
diff --git a/lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp b/lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp
index 6395f5bb5bd9b..f3edee1dd6dc1 100644
--- a/lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp
+++ b/lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp
@@ -10,7 +10,9 @@
#include <array>
#include <limits>
+#include <sstream>
+#include "llvm/ADT/STLExtras.h"
#include "llvm/IR/DerivedTypes.h"
#include "lldb/Core/PluginManager.h"
@@ -19,6 +21,7 @@
#include "lldb/Target/RegisterContext.h"
#include "lldb/Target/StackFrame.h"
#include "lldb/Target/Thread.h"
+#include "lldb/Utility/LLDBLog.h"
#include "lldb/Utility/RegisterValue.h"
#define DEFINE_REG_NAME(reg_num) ConstString(#reg_num).GetCString()
@@ -163,11 +166,83 @@ TotalArgsSizeInWords(bool is_rv64,
return total_size;
}
+static bool UpdateRegister(RegisterContext *reg_ctx,
+ const lldb::RegisterKind reg_kind,
+ const uint32_t reg_num, const addr_t value) {
+ Log *log = GetLog(LLDBLog::Expressions);
+
+ const RegisterInfo *reg_info = reg_ctx->GetRegisterInfo(reg_kind, reg_num);
+
+ LLDB_LOG(log, "Writing %s: 0x%" PRIx64, reg_info->name,
+ static_cast<uint64_t>(value));
+ if (!reg_ctx->WriteRegisterFromUnsigned(reg_info, value)) {
+ LLDB_LOG(log, "Writing %s: failed", reg_info->name);
+ return false;
+ }
+ return true;
+}
+
+static void LogInitInfo(Log *log, const Thread &thread, addr_t sp,
+ addr_t func_addr, addr_t return_addr,
+ const llvm::ArrayRef<addr_t> args) {
+ assert(log);
+ std::stringstream ss;
+ ss << "ABISysV_riscv::PrepareTrivialCall"
+ << " (tid = 0x%" << std::hex << thread.GetID() << ", sp = 0x%" << sp
+ << ", func_addr = 0x%" << func_addr << ", return_addr = 0x%"
+ << return_addr;
+
+ for (auto &&[idx, arg] : enumerate(args))
+ ss << ", arg" << std::dec << idx << " = 0x%" << std::hex << arg;
+ ss << ")";
+ log->PutString(ss.str());
+}
+
bool ABISysV_riscv::PrepareTrivialCall(Thread &thread, addr_t sp,
addr_t func_addr, addr_t return_addr,
llvm::ArrayRef<addr_t> args) const {
- // TODO: Implement
- return false;
+ Log *log = GetLog(LLDBLog::Expressions);
+ if (log)
+ LogInitInfo(log, thread, sp, func_addr, return_addr, args);
+
+ const auto reg_ctx_sp = thread.GetRegisterContext();
+ if (!reg_ctx_sp) {
+ LLDB_LOG(log, "Failed to get RegisterContext");
+ return false;
+ }
+
+ if (args.size() > s_regs_for_args_count) {
+ LLDB_LOG(log, "Function has %lu arguments, but only %lu are allowed!",
+ args.size(), s_regs_for_args_count);
+ return false;
+ }
+
+ // Write arguments to registers
+ for (auto &&[idx, arg] : enumerate(args)) {
+ const RegisterInfo *reg_info = reg_ctx_sp->GetRegisterInfo(
+ eRegisterKindGeneric, LLDB_REGNUM_GENERIC_ARG1 + idx);
+ LLDB_LOG(log, "About to write arg%lu (0x%" PRIx64 ") into %s", idx, arg,
+ reg_info->name);
+
+ if (!reg_ctx_sp->WriteRegisterFromUnsigned(reg_info, arg)) {
+ LLDB_LOG(log, "Failed to write arg%lu (0x%" PRIx64 ") into %s", idx, arg,
+ reg_info->name);
+ return false;
+ }
+ }
+
+ if (!UpdateRegister(reg_ctx_sp.get(), eRegisterKindGeneric,
+ LLDB_REGNUM_GENERIC_PC, func_addr))
+ return false;
+ if (!UpdateRegister(reg_ctx_sp.get(), eRegisterKindGeneric,
+ LLDB_REGNUM_GENERIC_SP, sp))
+ return false;
+ if (!UpdateRegister(reg_ctx_sp.get(), eRegisterKindGeneric,
+ LLDB_REGNUM_GENERIC_RA, return_addr))
+ return false;
+
+ LLDB_LOG(log, "ABISysV_riscv::%s: success", __func__);
+ return true;
}
bool ABISysV_riscv::PrepareTrivialCall(
@@ -221,14 +296,14 @@ bool ABISysV_riscv::PrepareTrivialCall(
assert(prototype.getFunctionNumParams() == args.size());
const size_t num_args = args.size();
- const size_t regs_for_args_count = 8U;
const size_t num_args_in_regs =
- num_args > regs_for_args_count ? regs_for_args_count : num_args;
+ num_args > s_regs_for_args_count ? s_regs_for_args_count : num_args;
// Number of arguments passed on stack.
size_t args_size = TotalArgsSizeInWords(m_is_rv64, args);
- auto on_stack =
- args_size <= regs_for_args_count ? 0 : args_size - regs_for_args_count;
+ auto on_stack = args_size <= s_regs_for_args_count
+ ? 0
+ : args_size - s_regs_for_args_count;
auto offset = on_stack * word_size;
uint8_t reg_value[8];
@@ -259,7 +334,7 @@ bool ABISysV_riscv::PrepareTrivialCall(
++reg_index;
}
- if (reg_index < regs_for_args_count || size == 0)
+ if (reg_index < s_regs_for_args_count || size == 0)
continue;
// Remaining arguments are passed on the stack.
diff --git a/lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.h b/lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.h
index d8cf008dbb0bf..04ec018c8a718 100644
--- a/lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.h
+++ b/lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.h
@@ -124,6 +124,9 @@ class ABISysV_riscv : public lldb_private::RegInfoBasedABI {
using lldb_private::RegInfoBasedABI::RegInfoBasedABI; // Call CreateInstance
// instead.
bool m_is_rv64; // true if target is riscv64; false if target is riscv32
+ static constexpr size_t s_regs_for_args_count =
+ 8U; // number of argument registers (the base integer calling convention
+ // provides 8 argument registers, a0-a7)
};
#endif // liblldb_ABISysV_riscv_h_
diff --git a/lldb/source/Plugins/Architecture/CMakeLists.txt b/lldb/source/Plugins/Architecture/CMakeLists.txt
index 9ed8edf70af3f..78cdaa0bdf2d4 100644
--- a/lldb/source/Plugins/Architecture/CMakeLists.txt
+++ b/lldb/source/Plugins/Architecture/CMakeLists.txt
@@ -2,3 +2,4 @@ add_subdirectory(Arm)
add_subdirectory(Mips)
add_subdirectory(PPC64)
add_subdirectory(AArch64)
+add_subdirectory(RISCV)
diff --git a/lldb/source/Plugins/Architecture/RISCV/ArchitectureRISCV.cpp b/lldb/source/Plugins/Architecture/RISCV/ArchitectureRISCV.cpp
new file mode 100644
index 0000000000000..e4608b41bd787
--- /dev/null
+++ b/lldb/source/Plugins/Architecture/RISCV/ArchitectureRISCV.cpp
@@ -0,0 +1,55 @@
+//===-- ArchitectureRISCV.cpp----------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "Plugins/Architecture/RISCV/ArchitectureRISCV.h"
+#include "lldb/Core/PluginManager.h"
+#include "lldb/Target/RegisterContext.h"
+#include "lldb/Target/Thread.h"
+#include "lldb/Utility/ArchSpec.h"
+
+#include "llvm/IR/LegacyPassManager.h"
+
+#include "DirectToIndirectFCR.h"
+
+using namespace lldb_private;
+using namespace lldb;
+
+LLDB_PLUGIN_DEFINE(ArchitectureRISCV)
+
+void ArchitectureRISCV::Initialize() {
+ PluginManager::RegisterPlugin(GetPluginNameStatic(),
+ "RISCV-specific algorithms",
+ &ArchitectureRISCV::Create);
+}
+
+void ArchitectureRISCV::Terminate() {
+ PluginManager::UnregisterPlugin(&ArchitectureRISCV::Create);
+}
+
+std::unique_ptr<Architecture> ArchitectureRISCV::Create(const ArchSpec &arch) {
+ if (!arch.GetTriple().isRISCV())
+ return nullptr;
+ return std::unique_ptr<Architecture>(new ArchitectureRISCV());
+}
+
+void ArchitectureRISCV::OverrideStopInfo(Thread &thread) const {}
+
+std::unique_ptr<llvm::legacy::PassManager>
+ArchitectureRISCV::GetArchitectureCustomPasses(
+ const ExecutionContext &exe_ctx, const llvm::StringRef expr) const {
+ // LLDB generates additional support functions like
+ // '_$__lldb_valid_pointer_check', that do not require custom passes
+ if (expr != "$__lldb_expr")
+ return nullptr;
+
+ std::unique_ptr<llvm::legacy::PassManager> custom_passes =
+ std::make_unique<llvm::legacy::PassManager>();
+ auto *P = createDirectToIndirectFCR(exe_ctx);
+ custom_passes->add(P);
+ return custom_passes;
+}
diff --git a/lldb/source/Plugins/Architecture/RISCV/ArchitectureRISCV.h b/lldb/source/Plugins/Architecture/RISCV/ArchitectureRISCV.h
new file mode 100644
index 0000000000000..6ef6c62de5f27
--- /dev/null
+++ b/lldb/source/Plugins/Architecture/RISCV/ArchitectureRISCV.h
@@ -0,0 +1,34 @@
+//===-- ArchitectureRISCV.h -------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#pragma once
+
+#include "lldb/Core/Architecture.h"
+
+namespace lldb_private {
+
+class ArchitectureRISCV : public Architecture {
+public:
+ static llvm::StringRef GetPluginNameStatic() { return "riscv"; }
+ static void Initialize();
+ static void Terminate();
+
+ llvm::StringRef GetPluginName() override { return GetPluginNameStatic(); }
+
+ void OverrideStopInfo(Thread &thread) const override;
+
+ std::unique_ptr<llvm::legacy::PassManager>
+ GetArchitectureCustomPasses(const ExecutionContext &exe_ctx,
+ const llvm::StringRef expr) const override;
+
+private:
+ static std::unique_ptr<Architecture> Create(const ArchSpec &arch);
+ ArchitectureRISCV() = default;
+};
+
+} // namespace lldb_private
diff --git a/lldb/source/Plugins/Architecture/RISCV/CMakeLists.txt b/lldb/source/Plugins/Architecture/RISCV/CMakeLists.txt
new file mode 100644
index 0000000000000..f2545eb35b000
--- /dev/null
+++ b/lldb/source/Plugins/Architecture/RISCV/CMakeLists.txt
@@ -0,0 +1,12 @@
+add_lldb_library(lldbPluginArchitectureRISCV PLUGIN
+ ArchitectureRISCV.cpp
+ DirectToIndirectFCR.cpp
+
+ LINK_LIBS
+ lldbPluginProcessUtility
+ lldbCore
+ lldbTarget
+ lldbUtility
+ LINK_COMPONENTS
+ Support
+ )
diff --git a/lldb/source/Plugins/Architecture/RISCV/DirectToIndirectFCR.cpp b/lldb/source/Plugins/Architecture/RISCV/DirectToIndirectFCR.cpp
new file mode 100644
index 0000000000000..71d3001fc06e8
--- /dev/null
+++ b/lldb/source/Plugins/Architecture/RISCV/DirectToIndirectFCR.cpp
@@ -0,0 +1,194 @@
+//===--- DirectToIndirectFCR.cpp - RISC-V specific pass -------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/IR/Constants.h"
+#include "llvm/IR/Function.h"
+#include "llvm/IR/IRBuilder.h"
+#include "llvm/IR/InstIterator.h"
+#include "llvm/IR/Instructions.h"
+#include "llvm/IR/LLVMContext.h"
+#include "llvm/IR/Module.h"
+#include "llvm/IR/Value.h"
+#include "llvm/InitializePasses.h"
+#include "llvm/Support/Casting.h"
+#include "llvm/Transforms/Utils/BasicBlockUtils.h"
+
+#include "Plugins/Architecture/RISCV/DirectToIndirectFCR.h"
+
+#include "lldb/Core/Architecture.h"
+#include "lldb/Core/Module.h"
+#include "lldb/Symbol/Symtab.h"
+#include "lldb/Target/ExecutionContext.h"
+#include "lldb/Target/Process.h"
+#include "lldb/Target/Target.h"
+#include "lldb/Utility/ConstString.h"
+#include "lldb/Utility/LLDBLog.h"
+#include "lldb/Utility/Log.h"
+
+#include <optional>
+
+using namespace llvm;
+using namespace lldb_private;
+
+namespace {
+std::string GetValueTypeStr(const llvm::Type *value_ty) {
+ assert(value_ty);
+ std::string str_type;
+ llvm::raw_string_ostream rso(str_type);
+ value_ty->print(rso);
+ return rso.str();
+}
+
+template <typename... Args> void LogMessage(const char *msg, Args &&...args) {
+ Log *log = GetLog(LLDBLog::Expressions);
+ LLDB_LOG(log, msg, std::forward<Args>(args)...);
+}
+} // namespace
+
+bool DirectToIndirectFCR::canBeReplaced(const llvm::CallInst *ci) {
+ assert(ci);
+ auto *return_value_ty = ci->getType();
+ if (!(return_value_ty->isIntegerTy() || return_value_ty->isVoidTy())) {
+ LogMessage("DirectToIndirectFCR: function {0} has unsupported "
+ "return type ({1})\n",
+ ci->getCalledFunction()->getName(),
+ GetValueTypeStr(return_value_ty));
+ return false;
+ }
+
+ const auto *arg = llvm::find_if_not(ci->args(), [](const auto &arg) {
+ const auto *type = arg->getType();
+ return type->isIntegerTy();
+ });
+
+ if (arg != ci->arg_end()) {
+ LogMessage("DirectToIndirectFCR: argument {0} of {1} function "
+ "has unsupported type ({2})\n",
+ (*arg)->getName(), ci->getCalledFunction()->getName(),
+ GetValueTypeStr((*arg)->getType()));
+ return false;
+ }
+ return true;
+}
+
+std::vector<llvm::Value *>
+DirectToIndirectFCR::getFunctionArgsAsValues(const llvm::CallInst *ci) {
+ assert(ci);
+ std::vector<llvm::Value *> args{};
+ llvm::transform(ci->args(), std::back_inserter(args),
+ [](const auto &arg) { return arg.get(); });
+ return args;
+}
+
+std::optional<lldb::addr_t>
+DirectToIndirectFCR::getFunctionAddress(const llvm::CallInst *ci) const {
+ auto *target = m_exe_ctx.GetTargetPtr();
+ const auto &lldb_module_sp = target->GetExecutableModule();
+ const auto &symtab = lldb_module_sp->GetSymtab();
+ const llvm::StringRef name = ci->getCalledFunction()->getName();
+
+ // eSymbolTypeCode: we try to find function
+ // eDebugNo: not a debug symbol
+ // eVisibilityExtern: function from extern module
+ const auto *symbol = symtab->FindFirstSymbolWithNameAndType(
+ ConstString(name), lldb::SymbolType::eSymbolTypeCode,
+ Symtab::Debug::eDebugNo, Symtab::Visibility::eVisibilityExtern);
+ if (!symbol) {
+ LogMessage("DirectToIndirectFCR: can't find {0} in symtab\n", name);
+ return std::nullopt;
+ }
+
+ lldb::addr_t addr = symbol->GetLoadAddress(target);
+ LogMessage("DirectToIndirectFCR: found address ({0}) of symbol {1}\n", addr,
+ name);
+ return addr;
+}
+
+llvm::CallInst *DirectToIndirectFCR::getInstReplace(llvm::CallInst *ci) const {
+ assert(ci);
+
+ std::optional<lldb::addr_t> addr_or_null = getFunctionAddress(ci);
+ if (!addr_or_null.has_value())
+ return nullptr;
+
+ lldb::addr_t addr = addr_or_null.value();
+
+ llvm::IRBuilder<> builder(ci);
+
+ std::vector<llvm::Value *> args = getFunctionArgsAsValues(ci);
+ llvm::Constant *func_addr = builder.getInt64(addr);
+ llvm::PointerType *ptr_func_ty = builder.getPtrTy();
+ auto *cast = builder.CreateIntToPtr(func_addr, ptr_func_ty);
+ auto *new_inst =
+ builder.CreateCall(ci->getFunctionType(), cast, ArrayRef(args));
+ return new_inst;
+}
+
+DirectToIndirectFCR::DirectToIndirectFCR(const ExecutionContext &exe_ctx)
+ : FunctionPass(ID), m_exe_ctx{exe_ctx} {}
+
+DirectToIndirectFCR::~DirectToIndirectFCR() = default;
+
+bool DirectToIndirectFCR::runOnFunction(llvm::Function &func) {
+ bool has_irreplaceable =
+ llvm::any_of(instructions(func), [this](llvm::Instruction &inst) {
+ llvm::CallInst *ci = dyn_cast<llvm::CallInst>(&inst);
+ if (!ci || ci->getCalledFunction()->isIntrinsic() ||
+ (DirectToIndirectFCR::canBeReplaced(ci) &&
+ getFunctionAddress(ci).has_value()))
+ return false;
+ return true;
+ });
+
+ if (has_irreplaceable) {
+ func.getParent()->getOrInsertNamedMetadata(
+ Architecture::s_target_incompatibility_marker);
+ return false;
+ }
+
+ std::vector<std::reference_wrapper<llvm::Instruction>>
+ replaceable_function_calls{};
+ llvm::copy_if(instructions(func),
+ std::back_inserter(replaceable_function_calls),
+ [](llvm::Instruction &inst) {
+ llvm::CallInst *ci = dyn_cast<llvm::CallInst>(&inst);
+ if (ci && !ci->getCalledFunction()->isIntrinsic())
+ return true;
+ return false;
+ });
+
+ if (replaceable_function_calls.empty())
+ return false;
+
+ std::vector<std::pair<llvm::CallInst *, llvm::CallInst *>> replaces;
+ llvm::transform(replaceable_function_calls, std::back_inserter(replaces),
+ [this](std::reference_wrapper<llvm::Instruction> inst)
+ -> std::pair<llvm::CallInst *, llvm::CallInst *> {
+ llvm::CallInst *ci = cast<llvm::CallInst>(&(inst.get()));
+ llvm::CallInst *new_inst = getInstReplace(ci);
+ return {ci, new_inst};
+ });
+
+ for (auto &&[from, to] : replaces) {
+ from->replaceAllUsesWith(to);
+ from->eraseFromParent();
+ }
+
+ return true;
+}
+
+llvm::StringRef DirectToIndirectFCR::getPassName() const {
+ return "Transform function calls to calls by address";
+}
+
+char DirectToIndirectFCR::ID = 0;
+
+llvm::FunctionPass *
+lldb_private::createDirectToIndirectFCR(const ExecutionContext &exe_ctx) {
+ return new DirectToIndirectFCR(exe_ctx);
+}
diff --git a/lldb/source/Plugins/Architecture/RISCV/DirectToIndirectFCR.h b/lldb/source/Plugins/Architecture/RISCV/DirectToIndirectFCR.h
new file mode 100644
index 0000000000000..5ac5820bd95a0
--- /dev/null
+++ b/lldb/source/Plugins/Architecture/RISCV/DirectToIndirectFCR.h
@@ -0,0 +1,58 @@
+//===--- DirectToIndirectFCR.h - RISC-V specific pass ---------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#pragma once
+
+#include "lldb/lldb-types.h"
+
+#include "llvm/IR/Instructions.h"
+#include "llvm/Pass.h"
+
+namespace lldb_private {
+
+class ExecutionContext;
+
+// During the lldb expression execution lldb wraps a user expression, jittes
+// fabricated code and then puts it into the stack memory. Thus, if user tried
+// to make a function call there will be a jump from a stack address to a code
+// sections's address. RISC-V Architecture doesn't have a large code model yet
+// and can make only a +-2GiB jumps, but in 64-bit architecture a distance
+// between stack addresses and code sections's addresses is longer. Therefore,
+// relocations resolver obtains an invalid address. To avoid such problem, this
+// pass should be used. It replaces function calls with appropriate function's
+// addresses explicitly. By doing so it removes relocations related to function
+// calls. This pass should be cosidered as temprorary solution until a large
+// code model will be approved.
+class DirectToIndirectFCR : public llvm::FunctionPass {
+
+ static bool canBeReplaced(const llvm::CallInst *ci);
+
+ static std::vector<llvm::Value *>
+ getFunctionArgsAsValues(const llvm::CallInst *ci);
+
+ std::optional<lldb::addr_t>
+ getFunctionAddress(const llvm::CallInst *ci) const;
+
+ llvm::CallInst *getInstReplace(llvm::CallInst *ci) const;
+
+public:
+ static char ID;
+
+ DirectToIndirectFCR(const ExecutionContext &exe_ctx);
+ ~DirectT...
[truncated]
|
|
✅ With the latest revision this PR passed the Python code formatter. |
dlav-sc
force-pushed
the
dlav-sc/lldb-expr-func-calls
branch
from
3ad75f0
to
73eb9f8
Compare
|
Thanks for the patches (if we end up going down this route you'll probably want to split out the LLVM JIT changes into a separate PR). Not very familiar with RISCV. Could you elaborate on the exact failures you were seeing and why we need the extra IR pass?
I guess this summarizes it, but would be good to see actual example failures |
I know little about the JIT side but this is my first reaction too. I wonder if that has its own tests that need adding, and it'll get more expert eyes on it.
I presume the work to do this is much greater than the work shown here? |
| class ExecutionContext; | ||
|
|
||
| // During the lldb expression execution lldb wraps a user expression, jittes | ||
| // fabricated code and then puts it into the stack memory. Thus, if user tried |
There was a problem hiding this comment.
I don't think this is quite true. lldb asks the remote stub to allocate memory with read+write permissions for storing the arguments, and read+execute permissions for the jitted code, e.g.
(lldb) p (int)isdigit('1')
< 13> send packet: $_M1000,rx#83
< 13> read packet: $100008000#00
< 13> send packet: $_M1000,rw#82
< 13> read packet: $10000c000#00
It would be surprising (to me) if the stub returned memory regions on the stack, because lldb is allowed to reuse these regions throughout the debug session, and we can't have the inferior process overwriting them.
You are correct that the allocated regions may be too far from the function(s) they'll need to call, and the same problem you describe below could exist. I don't mean to focus on the "stack memory" part of this, but I think it will confuse people when they read it.
There was a problem hiding this comment.
I think your expression is simple enough to be interpreted by IRInterpreter, instead of be sent to JIT compiler, so in this case there are no stack allocations.
There was a problem hiding this comment.
The arguments to the jitted code probably go to the stack (just like they would if you called the code "normally"), but I'd be very surprised if the code itself ended up on the stack (present-day architectures generally don't use executable stacks)
There was a problem hiding this comment.
Well, lldb generates its own code that wraps expression, something like:
#line 1 "<lldb wrapper prefix>"
#ifndef offsetof
#define offsetof(t, d) __builtin_offsetof(t, d)
#endif
#ifndef NULL
#define NULL (__null)
#endif
#ifndef Nil
#define Nil (__null)
#endif
#ifndef nil
#define nil (__null)
#endif
#ifndef YES
#define YES ((BOOL)1)
#endif
#ifndef NO
#define NO ((BOOL)0)
#endif
typedef __INT8_TYPE__ int8_t;
typedef __UINT8_TYPE__ uint8_t;
typedef __INT16_TYPE__ int16_t;
typedef __UINT16_TYPE__ uint16_t;
typedef __INT32_TYPE__ int32_t;
typedef __UINT32_TYPE__ uint32_t;
typedef __INT64_TYPE__ int64_t;
typedef __UINT64_TYPE__ uint64_t;
typedef __INTPTR_TYPE__ intptr_t;
typedef __UINTPTR_TYPE__ uintptr_t;
typedef __SIZE_TYPE__ size_t;
typedef __PTRDIFF_TYPE__ ptrdiff_t;
typedef unsigned short unichar;
extern "C"
{
int printf(const char * __restrict, ...);
}
typedef signed char BOOL;
void
$__lldb_expr(void *$__lldb_arg)
{
;
#line 1 "<user expression 0>"
foo()
;
#line 1 "<lldb wrapper suffix>"
}
So lldb have to puts this code somewhere, resolves relocations and executes. I have no idea where lldb can place the code except of the stack.
I've checked that on x86 lldb does the same:
lldb IRMemoryMap::Malloc (79, 0x10, 0x6, eAllocationPolicyProcessOnly) -> 0x7ffff7fc7040
lldb IRMemoryMap::Malloc (1, 0x1, 0x2, eAllocationPolicyProcessOnly) -> 0x7ffff7fc8450
lldb IRMemoryMap::Malloc (102, 0x1, 0x2, eAllocationPolicyProcessOnly) -> 0x7ffff7fc8460
lldb IRMemoryMap::Malloc (7, 0x4, 0x6, eAllocationPolicyProcessOnly) -> 0x7ffff7fc7090
lldb IRMemoryMap::Malloc (31, 0x8, 0x2, eAllocationPolicyProcessOnly) -> 0x7ffff7fc84d0
lldb IRMemoryMap::Malloc (111, 0x1, 0x2, eAllocationPolicyProcessOnly) -> 0x7ffff7fc84f0
lldb IRMemoryMap::Malloc (1, 0x1, 0x2, eAllocationPolicyProcessOnly) -> 0x7ffff7fc8560
lldb IRMemoryMap::Malloc (103, 0x8, 0x2, eAllocationPolicyProcessOnly) -> 0x7ffff7fc8570
lldb Function disassembly:
0x7ffff7fc7040: 55 other pushq %rbp
0x7ffff7fc7041: 48 89 e5 other movq %rsp, %rbp
0x7ffff7fc7044: 53 other pushq %rbx
0x7ffff7fc7045: 50 other pushq %rax
0x7ffff7fc7046: 48 89 fb other movq %rdi, %rbx
0x7ffff7fc7049: 48 8d 7d f0 other leaq -0x10(%rbp), %rdi
0x7ffff7fc704d: 48 b9 00 70 fc f7 ff 7f 00 00 other movabsq $0x7ffff7fc7000, %rcx ; imm = 0x7FFFF7FC7000
0x7ffff7fc7057: b0 00 other movb $0x0, %al
0x7ffff7fc7059: ff d1 call callq *%rcx
0x7ffff7fc705b: 48 89 5d f0 other movq %rbx, -0x10(%rbp)
0x7ffff7fc705f: 48 b8 29 51 55 55 55 55 00 00 other movabsq $0x555555555129, %rax ; imm = 0x555555555129
0x7ffff7fc7069: ff d0 call callq *%rax
0x7ffff7fc706b: 48 83 c4 08 other addq $0x8, %rsp
0x7ffff7fc706f: 5b other popq %rbx
0x7ffff7fc7070: 5d other popq %rbp
0x7ffff7fc7071: c3 return retq
There was a problem hiding this comment.
LLDB allocates memory inside the target process, either by calling mmap (that's what the PrepareTrivialCall function you're updating in this patch does), or through a server-specific mechanism (the _M packet).
There was a problem hiding this comment.
We've been doing some internal work to enable JIT on Hexagon, and we found that there are certain simple expressions that the IR interpreter doesn't handle, so they fall back to JIT. Ternary operators, for example. An expression like "(a == 2) : 10 ? 20" will run in JIT, not the IR interpreter.
As for memory, lldb will use the _M packet first, and if that fails (usually because the remote stub doesn't handle it), it will call mmap(). Some of the memory that lldb requests will be used for a stack, and some for the JITted code.
| // addresses explicitly. By doing so it removes relocations related to function | ||
| // calls. This pass should be cosidered as temprorary solution until a large | ||
| // code model will be approved. | ||
| class DirectToIndirectFCR : public llvm::FunctionPass { |
There was a problem hiding this comment.
It's a minor detail, and I never work on the llvm side of the codebase so this is probably just my own ignorance, but: I have no idea what DirectToIndirectFCR means, beyond the explanation above. I'm sure to people more familiar with llvm/clang this is clear.
There was a problem hiding this comment.
Just Direct To Indirect Function Calls Replacement. Maybe it's not the best name, but I have a poor fantasy, so if you have any sugestions, feel free to leave them here.
There was a problem hiding this comment.
DirectCallReplacementPass ? Unless FCR is an established abbreviation in some areas of llvm, I think the name is too cryptic.
There was a problem hiding this comment.
addressed. Changed to DirectCallReplacementPass.
| // relocations resolver obtains an invalid address. To avoid such problem, this | ||
| // pass should be used. It replaces function calls with appropriate function's | ||
| // addresses explicitly. By doing so it removes relocations related to function | ||
| // calls. This pass should be cosidered as temprorary solution until a large |
There was a problem hiding this comment.
tiny nit: "cosidered", "temprorary". I do this all the time myself, just happened to see them.
There was a problem hiding this comment.
addressed. Tricky ones
|
A quick look at the RISCV ISA and it says that the JAL instruction is pc relative and can jump +/- 1MB. JALR gets the upper 20 bits from a general purpose register and it includes 12 low bits in its instruction encoding. I know almost nothing about rv32, but is this what you mean by a Large Memory Model, the use of the JALR instruction? I didn't find the part that talks about how JALR works in rv64, but it seems likely it can specify an arbitrary 64-bit address? |
|
As for getting to the jitted code, of course lldb sets the pc directly so that initial switch from user code to jitted code region is fine. But getting from the jitted wrapper function to the user functions we need to call would require JALR instructions? |
| LLDB_LOG(log, "Writing %s: 0x%" PRIx64, reg_info->name, | ||
| static_cast<uint64_t>(value)); | ||
| if (!reg_ctx->WriteRegisterFromUnsigned(reg_info, value)) { | ||
| LLDB_LOG(log, "Writing %s: failed", reg_info->name); |
There was a problem hiding this comment.
You'll want to use LLDB_LOGF for printf-style formatters. LLDB_LOG (sans F) uses llvm's formatv formatters.
There was a problem hiding this comment.
addressed. I've changed log messages.
| template <typename... Args> void LogMessage(const char *msg, Args &&...args) { | ||
| Log *log = GetLog(LLDBLog::Expressions); | ||
| LLDB_LOG(log, msg, std::forward<Args>(args)...); | ||
| } |
There was a problem hiding this comment.
I don't think this helper makes a lot of sense:
- The goal is to get the log once (which is "expensive") and then have the macro just do a pointer check.
- This also breaks the line attribution which is why that's a macro in the first place.
Please use the LOG macros directly.
| // eSymbolTypeCode: we try to find function | ||
| // eDebugNo: not a debug symbol | ||
| // eVisibilityExtern: function from extern module |
There was a problem hiding this comment.
Either make these named constant variables or use inline comment in the function call (e.g, /*argument_name=*/value)
There was a problem hiding this comment.
addressed. Changed this place
To be fair I'm not sure. We have this MR: riscv-non-isa/riscv-elf-psabi-doc#388 (comment) . This review is taking quite a while
Well, for now I assume that existing tests are enough. Meaning we have a list of tests that start passing |
yep. |
Consider a simple function call:
Before patch: Lldb generates a wrapper, obtains its IR and tries to interpret it with IRInterpreter, but fails on a function call: IR: Output: After patch: Lldb also generates a wrapper, obtains its IR but sends it to a JIT compiler instead of IRInterpreter. MCJIT generates an assembly, puts it into a process stack memory, resolves relocations and finally executes: IR the same. Assembly: Puts assembly into a stack memory: Result: |
| class ExecutionContext; | ||
|
|
||
| // During the lldb expression execution lldb wraps a user expression, jittes | ||
| // fabricated code and then puts it into the stack memory. Thus, if user tried |
There was a problem hiding this comment.
The arguments to the jitted code probably go to the stack (just like they would if you called the code "normally"), but I'd be very surprised if the code itself ended up on the stack (present-day architectures generally don't use executable stacks)
| // addresses explicitly. By doing so it removes relocations related to function | ||
| // calls. This pass should be cosidered as temprorary solution until a large | ||
| // code model will be approved. | ||
| class DirectToIndirectFCR : public llvm::FunctionPass { |
There was a problem hiding this comment.
DirectCallReplacementPass ? Unless FCR is an established abbreviation in some areas of llvm, I think the name is too cryptic.
|
|
||
| std::optional<lldb::addr_t> | ||
| DirectToIndirectFCR::getFunctionAddress(const llvm::CallInst *ci) const { | ||
| auto *target = m_exe_ctx.GetTargetPtr(); |
There was a problem hiding this comment.
Have you looked into the possibility of reusing the code which does symbol resolution/linking for "normal" expression evaluation (I'm not sure where that happens, but it must happen somewhere). Looking at this function, I see at least two problems:
- it only looks for symbols in the main executable
- it doesn't look for non-exported symbols (you probably want to be able to call static functions)
There was a problem hiding this comment.
Large code model has been merged, so DirectToIndirectFCR pass is deprecated.
| // | ||
| //===----------------------------------------------------------------------===// | ||
|
|
||
| #pragma once |
There was a problem hiding this comment.
We don't do this in lldb. Please use a regular header guard.
| bool DirectToIndirectFCR::canBeReplaced(const llvm::CallInst *ci) { | ||
| assert(ci); | ||
| auto *return_value_ty = ci->getType(); | ||
| if (!(return_value_ty->isIntegerTy() || return_value_ty->isVoidTy())) { |
There was a problem hiding this comment.
Mainly for my curiosity, what is the issue with other return types? Naively, I would have expected that, at the IR level, you could perform this kind of transformation for any return type.
There was a problem hiding this comment.
I would have expected that, at the IR level, you could perform this kind of transformation for any return type.
As you've expected I really can apply this pass on functions with any arguments' and return value's types. I just filter unsupported for some reasons function calls.
As for doubles, I think that the problem is in the ABI or MCJIT, because the IR is correct, while the resulting assembly from JIT compiler doesn't contain even float instructions, needed to pass function arguments and get a return value. My RISCV machine supports a float extension, so the assembly should contain float instructions. Currently the result of such expressions is a trash, like:
(double) $1 = 3.3033229080945744E-319
so I've decided to remove function calls that takes double arguments or have double return value.
As for pointers, as such no problems with them.
But if in function that takes char* argument pass a row c string, there will be a SIGSEGV. Most interesting that if pass a variable the function successfully finished:
#include <stdio.h>
void foo(const char *msg) {
printf("%s\n", msg);
}
int main() {
const char *msg = "msg";
foo(msg);
}
Process 302 stopped
* thread #1, name = 'pointer.x', stop reason = step over
frame #0: 0x00000000000105cc pointer.x`main at pointer.cpp:9:6
6
7 int main() {
8 const char *msg = "msg";
-> 9 foo(msg);
10 }
(lldb) expr foo("msg")
error: Execution was interrupted, reason: signal SIGSEGV: address not mapped to object (fault address: 0xfffffffff7b6d760).
The process has been returned to the state before expression evaluation.
(lldb) expr foo(msg)
msg
(lldb)
I think that the problem is in relocations.
There was a problem hiding this comment.
Yes, that sounds like a relocation problem. And I think that's probably the same with floating point arguments. If the generated code looks anything like this, then you're going to need a relocation to access the float argument in another section.
While I don't think you need to submit a fully functional implementation straight away (in fact, maybe the opposite is better), since this patch also introduces the "architecture incompatibility" feature, I think it'd be preferable to either fix it or just let it crash, as both of those things will probably reduce the scope of the PR.
There was a problem hiding this comment.
I have fixed expressions with pointer arguments / return value: ee88a87
PIC option is needed for RISC-V 64, because otherwise wrong relocations will be placed.
Nope, you can't specify an arbitrary 64-bit address in RISCV now. Here riscv-non-isa/riscv-elf-psabi-doc#388 (comment) they write that if Large Code Model is approved then at least 6 instructions well be need to do it: |
I don't suppose there's any chance of implementing some kind of provisional/experimental support for it in the compiler (since what you're basically doing here is implementing a large code model, but within a debugger)? |
How are you running the tests? We're working on getting the tests running in a sane fashion, using user space qemu. |
|
BTW, if you call SetCanInterpretFunctionCalls(true); somewhere, you can use the IR interpreter to call functions. PrepareTrivialCall below the implementation you do here, for JIT, handles that. |
dlav-sc
force-pushed
the
dlav-sc/lldb-expr-func-calls
branch
3 times, most recently
from
8d1b7a4
to
da97b8d
Compare
For now I just run Python API tests. They support remote debugging, so dotest.py has options like |
Yeah, it looks really suspicious.
I think you are right. |
Yes, agree with doing this separately so we can get this PR landed. |
|
Reminder to update the PR description now that the large code model has been implemented over in llvm. |
dlav-sc
force-pushed
the
dlav-sc/lldb-expr-func-calls
branch
2 times, most recently
from
b9275b3
to
bb86d31
Compare
There was a problem hiding this comment.
LGTM too
But please wait a bit for @lhames to chime in about the RuntimeDyld changes in case he has any comments/ideas for testing those
|
@lhames could you take a look, please? |
Hi @dlav-sc. The If you want to test the RuntimeDyld changes you could add |
Function calls support in LLDB expressions for RISCV: 1 of 4 Augments corresponding functionality to RISCV ABI, which allows to jit lldb expressions and thus make function calls inside them. Only function calls with integer and void function arguments and return value are supported.
Function calls support in LLDB expressions for RISCV: 2 of 4 Adds required RISCV relocations resolving functionality in lldb ExecutionEngine.
Function calls support in LLDB expressions for RISCV: 3 of 4 This patch sets large code model in MCJIT settings for RISC-V 64-bit targets that allows to make assembly jumps at any 64bit address. This is needed, because resulted jitted code may contain more that +-2GB jumps, that are not available in RISC-V with medium code model.
dlav-sc
force-pushed
the
dlav-sc/lldb-expr-func-calls
branch
from
bb86d31
to
05e62e5
Compare
Function calls support in LLDB expressions for RISCV: 4 of 4 This patch adds desired feature flags in MCJIT compiler to enable hard-float instructions if target supports them and allows to use floats and doubles in lldb expressions.
|
Thank you for the review. |
[lldb][RISCV] add jitted function calls to ABI Function calls support in LLDB expressions for RISCV: 1 of 4 Augments corresponding functionality to RISCV ABI, which allows to jit lldb expressions and thus make function calls inside them. Only function calls with integer and void function arguments and return value are supported. [lldb][RISCV] add JIT relocations resolver Function calls support in LLDB expressions for RISCV: 2 of 4 Adds required RISCV relocations resolving functionality in lldb ExecutionEngine. [lldb][RISCV] RISC-V large code model in lldb expressions Function calls support in LLDB expressions for RISCV: 3 of 4 This patch sets large code model in MCJIT settings for RISC-V 64-bit targets that allows to make assembly jumps at any 64bit address. This is needed, because resulted jitted code may contain more that +-2GB jumps, that are not available in RISC-V with medium code model. [lldb][RISCV] doubles support in lldb expressions Function calls support in LLDB expressions for RISCV: 4 of 4 This patch adds desired feature flags in MCJIT compiler to enable hard-float instructions if target supports them and allows to use floats and doubles in lldb expressions.
[lldb][RISCV] add jitted function calls to ABI Function calls support in LLDB expressions for RISCV: 1 of 4 Augments corresponding functionality to RISCV ABI, which allows to jit lldb expressions and thus make function calls inside them. Only function calls with integer and void function arguments and return value are supported. [lldb][RISCV] add JIT relocations resolver Function calls support in LLDB expressions for RISCV: 2 of 4 Adds required RISCV relocations resolving functionality in lldb ExecutionEngine. [lldb][RISCV] RISC-V large code model in lldb expressions Function calls support in LLDB expressions for RISCV: 3 of 4 This patch sets large code model in MCJIT settings for RISC-V 64-bit targets that allows to make assembly jumps at any 64bit address. This is needed, because resulted jitted code may contain more that +-2GB jumps, that are not available in RISC-V with medium code model. [lldb][RISCV] doubles support in lldb expressions Function calls support in LLDB expressions for RISCV: 4 of 4 This patch adds desired feature flags in MCJIT compiler to enable hard-float instructions if target supports them and allows to use floats and doubles in lldb expressions.
[lldb][RISCV] add jitted function calls to ABI Function calls support in LLDB expressions for RISCV: 1 of 4 Augments corresponding functionality to RISCV ABI, which allows to jit lldb expressions and thus make function calls inside them. Only function calls with integer and void function arguments and return value are supported. [lldb][RISCV] add JIT relocations resolver Function calls support in LLDB expressions for RISCV: 2 of 4 Adds required RISCV relocations resolving functionality in lldb ExecutionEngine. [lldb][RISCV] RISC-V large code model in lldb expressions Function calls support in LLDB expressions for RISCV: 3 of 4 This patch sets large code model in MCJIT settings for RISC-V 64-bit targets that allows to make assembly jumps at any 64bit address. This is needed, because resulted jitted code may contain more that +-2GB jumps, that are not available in RISC-V with medium code model. [lldb][RISCV] doubles support in lldb expressions Function calls support in LLDB expressions for RISCV: 4 of 4 This patch adds desired feature flags in MCJIT compiler to enable hard-float instructions if target supports them and allows to use floats and doubles in lldb expressions.
[lldb][RISCV] add jitted function calls to ABI Function calls support in LLDB expressions for RISCV: 1 of 4 Augments corresponding functionality to RISCV ABI, which allows to jit lldb expressions and thus make function calls inside them. Only function calls with integer and void function arguments and return value are supported. [lldb][RISCV] add JIT relocations resolver Function calls support in LLDB expressions for RISCV: 2 of 4 Adds required RISCV relocations resolving functionality in lldb ExecutionEngine. [lldb][RISCV] RISC-V large code model in lldb expressions Function calls support in LLDB expressions for RISCV: 3 of 4 This patch sets large code model in MCJIT settings for RISC-V 64-bit targets that allows to make assembly jumps at any 64bit address. This is needed, because resulted jitted code may contain more that +-2GB jumps, that are not available in RISC-V with medium code model. [lldb][RISCV] doubles support in lldb expressions Function calls support in LLDB expressions for RISCV: 4 of 4 This patch adds desired feature flags in MCJIT compiler to enable hard-float instructions if target supports them and allows to use floats and doubles in lldb expressions.
This patch adds desired feature flags in JIT compiler to enable hard-float instructions if target supports them and allows to use floats and doubles in lldb expressions. Fited tests: lldb-shell :: Expr/TestAnonNamespaceParamFunc.cpp lldb-shell :: Expr/TestIRMemoryMap.test lldb-shell :: Expr/TestStringLiteralExpr.test lldb-shell :: SymbolFile/DWARF/debug-types-expressions.test Similar as #99336 Depens on: #114741 Reviewed By: SixWeining Pull Request: #114742
This is necessary for supporting function calls in LLDB expressions for LoongArch. This patch is inspired by llvm#99336 and simply extracts the parts related to RuntimeDyld. Reviewed By: lhames Pull Request: llvm#114741
This patch adds desired feature flags in JIT compiler to enable hard-float instructions if target supports them and allows to use floats and doubles in lldb expressions. Fited tests: lldb-shell :: Expr/TestAnonNamespaceParamFunc.cpp lldb-shell :: Expr/TestIRMemoryMap.test lldb-shell :: Expr/TestStringLiteralExpr.test lldb-shell :: SymbolFile/DWARF/debug-types-expressions.test Similar as llvm#99336 Depens on: llvm#114741 Reviewed By: SixWeining Pull Request: llvm#114742
To make function calls inside lldb expressions ABI support and JIT engine support are required. This patch augments corresponding functionality to RISCV ABI and implements RISCV relocation resolver in MCJIT.
Also it sets large code model for RISC-V 64bit targets, which allows to make assembly jumps at any 64bit address. This is necessary, because jitted code can be placed too far from the code in the binary.
Fixed tests: