[lldb][FreeBSD] Add dynamic loader handle class for FreeBSD Kernel #67106
Conversation
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
@llvm/pr-subscribers-lldb ChangesThis commit is moved from llvm-phabricator. Patch is 34.73 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/67106.diff 6 Files Affected:
diff --git a/lldb/source/Plugins/DynamicLoader/CMakeLists.txt b/lldb/source/Plugins/DynamicLoader/CMakeLists.txt
index f357fea02efbe68..30607159acdc088 100644
--- a/lldb/source/Plugins/DynamicLoader/CMakeLists.txt
+++ b/lldb/source/Plugins/DynamicLoader/CMakeLists.txt
@@ -1,4 +1,5 @@
add_subdirectory(Darwin-Kernel)
+add_subdirectory(FreeBSD-Kernel)
add_subdirectory(MacOSX-DYLD)
add_subdirectory(POSIX-DYLD)
add_subdirectory(Static)
diff --git a/lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/CMakeLists.txt b/lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/CMakeLists.txt
new file mode 100644
index 000000000000000..76daf0a327cf97b
--- /dev/null
+++ b/lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/CMakeLists.txt
@@ -0,0 +1,13 @@
+add_lldb_library(lldbPluginDynamicLoaderFreeBSDKernel PLUGIN
+ DynamicLoaderFreeBSDKernel.cpp
+
+ LINK_LIBS
+ lldbBreakpoint
+ lldbCore
+ lldbHost
+ lldbInterpreter
+ lldbSymbol
+ lldbTarget
+ lldbUtility
+ lldbPluginObjectFileELF
+ )
diff --git a/lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/DynamicLoaderFreeBSDKernel.cpp b/lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/DynamicLoaderFreeBSDKernel.cpp
new file mode 100644
index 000000000000000..9712da3ecdc8299
--- /dev/null
+++ b/lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/DynamicLoaderFreeBSDKernel.cpp
@@ -0,0 +1,771 @@
+#include "lldb/Breakpoint/StoppointCallbackContext.h"
+#include "lldb/Core/Debugger.h"
+#include "lldb/Core/Module.h"
+#include "lldb/Core/ModuleSpec.h"
+#include "lldb/Core/PluginManager.h"
+#include "lldb/Core/Section.h"
+#include "lldb/Core/StreamFile.h"
+#include "lldb/Interpreter/OptionValueProperties.h"
+#include "lldb/Symbol/LocateSymbolFile.h"
+#include "lldb/Symbol/ObjectFile.h"
+#include "lldb/Target/OperatingSystem.h"
+#include "lldb/Target/RegisterContext.h"
+#include "lldb/Target/StackFrame.h"
+#include "lldb/Target/Target.h"
+#include "lldb/Target/Thread.h"
+#include "lldb/Target/ThreadPlanRunToAddress.h"
+#include "lldb/Utility/DataBuffer.h"
+#include "lldb/Utility/DataBufferHeap.h"
+#include "lldb/Utility/LLDBLog.h"
+#include "lldb/Utility/Log.h"
+#include "lldb/Utility/State.h"
+
+#include "Plugins/ObjectFile/ELF/ObjectFileELF.h"
+
+#include "DynamicLoaderFreeBSDKernel.h"
+#include <memory>
+#include <mutex>
+
+using namespace lldb;
+using namespace lldb_private;
+
+LLDB_PLUGIN_DEFINE(DynamicLoaderFreeBSDKernel)
+
+void DynamicLoaderFreeBSDKernel::Initialize() {
+ PluginManager::RegisterPlugin(GetPluginNameStatic(),
+ GetPluginDescriptionStatic(), CreateInstance,
+ DebuggerInit);
+}
+
+void DynamicLoaderFreeBSDKernel::Terminate() {
+ PluginManager::UnregisterPlugin(CreateInstance);
+}
+
+llvm::StringRef DynamicLoaderFreeBSDKernel::GetPluginDescriptionStatic() {
+ return "The Dynamic Loader Plugin For FreeBSD Kernel";
+}
+
+static bool is_kernel(Module *module) {
+ if (!module)
+ return false;
+
+ ObjectFile *objfile = module->GetObjectFile();
+ if (!objfile)
+ return false;
+ if (objfile->GetType() != ObjectFile::eTypeExecutable)
+ return false;
+ if (objfile->GetStrata() != ObjectFile::eStrataUnknown &&
+ objfile->GetStrata() != ObjectFile::eStrataUser)
+ return false;
+
+ return true;
+}
+
+static bool is_kmod(Module *module) {
+ if (!module)
+ return false;
+ if (!module->GetObjectFile())
+ return false;
+ ObjectFile *objfile = module->GetObjectFile();
+ if (objfile->GetType() != ObjectFile::eTypeObjectFile && objfile->GetType() != ObjectFile::eTypeSharedLibrary)
+ return false;
+
+ return true;
+}
+
+static bool is_reloc(Module *module) {
+ if (!module)
+ return false;
+ if (!module->GetObjectFile())
+ return false;
+ ObjectFile *objfile = module->GetObjectFile();
+ if (objfile->GetType() != ObjectFile::eTypeObjectFile)
+ return false;
+
+ return true;
+}
+
+// Instantiate Function of the FreeBSD Kernel Dynamic Loader Plugin called when
+// Register the Plugin
+DynamicLoader *
+DynamicLoaderFreeBSDKernel::CreateInstance(lldb_private::Process *process,
+ bool force) {
+ // Check the environment when the plugin is not force loaded
+ Log *log = GetLog(LLDBLog::DynamicLoader);
+ LLDB_LOGF(log, "DynamicLoaderFreeBSDKernel::CreateInstance: "
+ "Try to create instance");
+ if (!force) {
+ Module *exec = process->GetTarget().GetExecutableModulePointer();
+ // Check if the target is kernel
+ if (exec && !is_kernel(exec)) {
+ return nullptr;
+ }
+
+ const llvm::Triple &triple_ref =
+ process->GetTarget().GetArchitecture().GetTriple();
+ if (!triple_ref.isOSFreeBSD()) {
+ return nullptr;
+ }
+ }
+
+ // At this point we have checked the target is a FreeBSD kernel and all we
+ // have to do is to find the kernel address
+ const addr_t kernel_address = FindFreeBSDKernel(process);
+
+ if (CheckForKernelImageAtAddress(process, kernel_address).IsValid())
+ return new DynamicLoaderFreeBSDKernel(process, kernel_address);
+
+ return nullptr;
+}
+
+addr_t
+DynamicLoaderFreeBSDKernel::FindFreeBSDKernel(lldb_private::Process *process) {
+ addr_t kernel_addr = process->GetImageInfoAddress();
+ if (kernel_addr == LLDB_INVALID_ADDRESS)
+ kernel_addr = FindKernelAtLoadAddress(process);
+ return kernel_addr;
+}
+
+// Get the kernel address if the kernel is not loaded with a slide
+addr_t DynamicLoaderFreeBSDKernel::FindKernelAtLoadAddress(
+ lldb_private::Process *process) {
+ Module *exe_module = process->GetTarget().GetExecutableModulePointer();
+
+ if (!is_kernel(exe_module))
+ return LLDB_INVALID_ADDRESS;
+
+ ObjectFile *exe_objfile = exe_module->GetObjectFile();
+
+ if (!exe_objfile->GetBaseAddress().IsValid())
+ return LLDB_INVALID_ADDRESS;
+
+ if (CheckForKernelImageAtAddress(
+ process, exe_objfile->GetBaseAddress().GetFileAddress())
+ .IsValid())
+ return exe_objfile->GetBaseAddress().GetFileAddress();
+
+ return LLDB_INVALID_ADDRESS;
+}
+
+// Read ELF header from memry and return
+bool DynamicLoaderFreeBSDKernel::ReadELFHeader(Process *process,
+ lldb::addr_t addr,
+ llvm::ELF::Elf32_Ehdr &header,
+ bool *read_error) {
+ Status error;
+ if (read_error)
+ *read_error = false;
+
+ if (process->ReadMemory(addr, &header, sizeof(header), error) !=
+ sizeof(header)) {
+ if (read_error)
+ *read_error = true;
+ return false;
+ }
+
+ if (!header.checkMagic())
+ return false;
+
+ return true;
+}
+
+// Check the correctness of Kernel and return UUID
+lldb_private::UUID DynamicLoaderFreeBSDKernel::CheckForKernelImageAtAddress(
+ Process *process, lldb::addr_t addr, bool *read_error) {
+ Log *log = GetLog(LLDBLog::DynamicLoader);
+
+ if (addr == LLDB_INVALID_ADDRESS) {
+ if (read_error)
+ *read_error = true;
+ return UUID();
+ }
+
+ LLDB_LOGF(log,
+ "DynamicLoaderFreeBSDKernel::CheckForKernelImageAtAddress: "
+ "looking for kernel binary at 0x%" PRIx64,
+ addr);
+
+ llvm::ELF::Elf32_Ehdr header;
+ if (!ReadELFHeader(process, addr, header))
+ return UUID();
+
+ // Check header type
+ if (header.e_type != llvm::ELF::ET_EXEC)
+ return UUID();
+
+ ModuleSP memory_module_sp =
+ process->ReadModuleFromMemory(FileSpec("temp_freebsd_kernel"), addr);
+ if (!memory_module_sp.get())
+ return UUID();
+
+ ObjectFile *exe_objfile = memory_module_sp->GetObjectFile();
+ if (exe_objfile == nullptr) {
+ LLDB_LOGF(log,
+ "DynamicLoaderFreeBSDKernel::CheckForKernelImageAtAddress "
+ "found a binary at 0x%" PRIx64
+ " but could not create an object file from memory",
+ addr);
+ return UUID();
+ }
+
+ if (is_kernel(memory_module_sp.get())) {
+ ArchSpec kernel_arch(
+ llvm::ELF::convertEMachineToArchName(header.e_machine));
+
+ if (!process->GetTarget().GetArchitecture().IsCompatibleMatch(kernel_arch))
+ process->GetTarget().SetArchitecture(kernel_arch);
+
+ if (log) {
+ std::string uuid_str;
+ if (memory_module_sp->GetUUID().IsValid()) {
+ uuid_str = "with UUID ";
+ uuid_str += memory_module_sp->GetUUID().GetAsString();
+ } else {
+ uuid_str = "and no LC_UUID found in load commands ";
+ }
+ LLDB_LOGF(log,
+ "DynamicLoaderFreeBSDKernel::CheckForKernelImageAtAddress: "
+ "kernel binary image found at 0x%" PRIx64 " with arch '%s' %s",
+ addr, kernel_arch.GetTriple().str().c_str(), uuid_str.c_str());
+ }
+
+ return memory_module_sp->GetUUID();
+ }
+
+ return UUID();
+}
+
+void DynamicLoaderFreeBSDKernel::DebuggerInit(
+ lldb_private::Debugger &debugger) {
+}
+
+DynamicLoaderFreeBSDKernel::DynamicLoaderFreeBSDKernel(Process *process,
+ addr_t kernel_address)
+ : DynamicLoader(process), m_process(process),
+ m_linker_file_list_struct_addr(LLDB_INVALID_ADDRESS),
+ m_linker_file_head_addr(LLDB_INVALID_ADDRESS),
+ m_kernel_load_address(kernel_address),
+ m_mutex() {
+ process->SetCanRunCode(false);
+}
+
+DynamicLoaderFreeBSDKernel::~DynamicLoaderFreeBSDKernel() { Clear(true); }
+
+void DynamicLoaderFreeBSDKernel::Update() {
+ LoadKernelModules();
+ SetNotificationBreakPoint();
+}
+
+// Create in memory Module at the load address
+bool DynamicLoaderFreeBSDKernel::KModImageInfo::ReadMemoryModule(
+ lldb_private::Process *process) {
+ Log *log = GetLog(LLDBLog::DynamicLoader);
+ if (m_memory_module_sp)
+ return true;
+ if (m_load_address == LLDB_INVALID_ADDRESS)
+ return false;
+
+ FileSpec file_spec(m_name.c_str());
+
+ ModuleSP memory_module_sp;
+
+ llvm::ELF::Elf32_Ehdr elf_eheader;
+ size_t size_to_read = 512;
+
+ if (ReadELFHeader(process, m_load_address, elf_eheader)) {
+ if (elf_eheader.e_ident[llvm::ELF::EI_CLASS] == llvm::ELF::ELFCLASS32) {
+ size_to_read = sizeof(llvm::ELF::Elf32_Ehdr) +
+ elf_eheader.e_phnum * elf_eheader.e_phentsize;
+ } else if (elf_eheader.e_ident[llvm::ELF::EI_CLASS] ==
+ llvm::ELF::ELFCLASS64) {
+ llvm::ELF::Elf64_Ehdr elf_eheader;
+ Status error;
+ if (process->ReadMemory(m_load_address, &elf_eheader, sizeof(elf_eheader),
+ error) == sizeof(elf_eheader))
+ size_to_read = sizeof(llvm::ELF::Elf64_Ehdr) +
+ elf_eheader.e_phnum * elf_eheader.e_phentsize;
+ }
+ }
+
+ memory_module_sp =
+ process->ReadModuleFromMemory(file_spec, m_load_address, size_to_read);
+
+ if (!memory_module_sp)
+ return false;
+
+ bool this_is_kernel = is_kernel(memory_module_sp.get());
+
+ if (!m_uuid.IsValid() && memory_module_sp->GetUUID().IsValid())
+ m_uuid = memory_module_sp->GetUUID();
+
+ m_memory_module_sp = memory_module_sp;
+ m_is_kernel = this_is_kernel;
+
+ // The kernel binary is from memory
+ if (this_is_kernel) {
+ if (log)
+ LLDB_LOGF(log,
+ "KextImageInfo::ReadMemoryModule read the kernel binary out "
+ "of memory");
+
+ if (memory_module_sp->GetArchitecture().IsValid())
+ process->GetTarget().SetArchitecture(memory_module_sp->GetArchitecture());
+ }
+
+ return true;
+}
+
+bool DynamicLoaderFreeBSDKernel::KModImageInfo::LoadImageUsingMemoryModule(
+ lldb_private::Process *process) {
+ Log *log = GetLog(LLDBLog::DynamicLoader);
+
+ if (IsLoaded())
+ return true;
+
+ Target &target = process->GetTarget();
+
+ if (IsKernel() && m_uuid.IsValid()) {
+ Stream &s = target.GetDebugger().GetOutputStream();
+ s.Printf("Kernel UUID: %s\n", m_uuid.GetAsString().c_str());
+ s.Printf("Load Address: 0x%" PRIx64 "\n", m_load_address);
+ }
+
+ // Test if the module is loaded into the taget,
+ // maybe the module is loaded manually by user by doing target module add
+ // So that we have to create the module manually
+ if (!m_module_sp) {
+ const ModuleList &target_images = target.GetImages();
+ m_module_sp = target_images.FindModule(m_uuid);
+
+ // Search in the file system
+ if (!m_module_sp) {
+ ModuleSpec module_spec(FileSpec(GetPath()), target.GetArchitecture());
+ if (IsKernel()) {
+ Status error;
+ if (Symbols::DownloadObjectAndSymbolFile(module_spec, error, true)) {
+ if (FileSystem::Instance().Exists(module_spec.GetFileSpec()))
+ m_module_sp = std::make_shared<Module>(module_spec.GetFileSpec(),
+ target.GetArchitecture());
+ }
+ }
+
+ if (!m_module_sp)
+ m_module_sp = target.GetOrCreateModule(module_spec, true);
+ if (IsKernel() && !m_module_sp) {
+ Stream &s = target.GetDebugger().GetOutputStream();
+ s.Printf("WARNING: Unable to locate kernel binary on the debugger "
+ "system.\n");
+ }
+ }
+
+ if (m_module_sp) {
+ // If the file is not kernel or kmod, the target should be loaded once and
+ // don't reload again
+ if (!IsKernel() && !is_kmod(m_module_sp.get())) {
+ ModuleSP existing_module_sp = target.GetImages().FindModule(m_uuid);
+ if (existing_module_sp &&
+ existing_module_sp->IsLoadedInTarget(&target)) {
+ LLDB_LOGF(log,
+ "'%s' with UUID %s is not a kmod or kernel, and is "
+ "already registered in target, not loading.",
+ m_name.c_str(), m_uuid.GetAsString().c_str());
+ return true;
+ }
+ }
+ m_uuid = m_module_sp->GetUUID();
+
+ // or append to the images
+ target.GetImages().AppendIfNeeded(m_module_sp, false);
+ }
+ }
+
+ // If this file is relocatable kernel module(x86_64), adjust it's section(PT_LOAD segment) and return
+ // Because the kernel module's load address is the text section.
+ // lldb cannot create full memory module upon relocatable file
+ // So what we do is to set the load address only.
+ if (is_kmod(m_module_sp.get()) && is_reloc(m_module_sp.get())) {
+ m_stop_id = process->GetStopID();
+ bool changed;
+ m_module_sp->SetLoadAddress(target, m_load_address, true, changed);
+ return true;
+ }
+
+ if (m_module_sp)
+ ReadMemoryModule(process);
+
+ // Calculate the slides of in memory module
+ if (!m_memory_module_sp || !m_module_sp) {
+ m_module_sp.reset();
+ return false;
+ }
+
+ ObjectFile *ondisk_object_file = m_module_sp->GetObjectFile();
+ ObjectFile *memory_object_file = m_memory_module_sp->GetObjectFile();
+
+ if (!ondisk_object_file || !memory_object_file)
+ m_module_sp.reset();
+
+ // Find the slide address
+ addr_t fixed_slide = LLDB_INVALID_ADDRESS;
+ if (ObjectFileELF *memory_objfile_elf =
+ llvm::dyn_cast<ObjectFileELF>(memory_object_file)) {
+ addr_t load_address = memory_object_file->GetBaseAddress().GetFileAddress();
+
+ if (load_address != LLDB_INVALID_ADDRESS &&
+ m_load_address != load_address) {
+ fixed_slide = m_load_address - load_address;
+ LLDB_LOGF(log,
+ "kmod %s in-memory LOAD vmaddr is not correct, using a "
+ "fixed slide of 0x%" PRIx64,
+ m_name.c_str(), fixed_slide);
+ }
+ }
+
+ SectionList *ondisk_section_list = ondisk_object_file->GetSectionList();
+ SectionList *memory_section_list = memory_object_file->GetSectionList();
+
+ if (memory_section_list && ondisk_object_file) {
+ const uint32_t num_ondisk_sections = ondisk_section_list->GetSize();
+ uint32_t num_load_sections = 0;
+
+ for (uint32_t section_idx = 0; section_idx < num_ondisk_sections;
+ ++section_idx) {
+ SectionSP on_disk_section_sp =
+ ondisk_section_list->GetSectionAtIndex(section_idx);
+
+ if (!on_disk_section_sp)
+ continue;
+ if (fixed_slide != LLDB_INVALID_ADDRESS) {
+ target.SetSectionLoadAddress(on_disk_section_sp,
+ on_disk_section_sp->GetFileAddress() +
+ fixed_slide);
+
+ } else {
+ const Section *memory_section =
+ memory_section_list
+ ->FindSectionByName(on_disk_section_sp->GetName())
+ .get();
+ if (memory_section) {
+ target.SetSectionLoadAddress(on_disk_section_sp,
+ memory_section->GetFileAddress());
+ ++num_load_sections;
+ }
+ }
+ }
+
+ if (num_load_sections)
+ m_stop_id = process->GetStopID();
+ else
+ m_module_sp.reset();
+ } else {
+ m_module_sp.reset();
+ }
+
+ if (IsLoaded() && m_module_sp && IsKernel()) {
+ Stream &s = target.GetDebugger().GetOutputStream();
+ ObjectFile *kernel_object_file = m_module_sp->GetObjectFile();
+ if (kernel_object_file) {
+ addr_t file_address =
+ kernel_object_file->GetBaseAddress().GetFileAddress();
+ if (m_load_address != LLDB_INVALID_ADDRESS &&
+ file_address != LLDB_INVALID_ADDRESS) {
+ s.Printf("Kernel slide 0x%" PRIx64 " in memory.\n",
+ m_load_address - file_address);
+ s.Printf("Loaded kernel file %s\n",
+ m_module_sp->GetFileSpec().GetPath().c_str());
+ }
+ }
+ s.Flush();
+ }
+
+ return IsLoaded();
+}
+
+// This function is work for kernel file, others it wil reset load address and
+// return false
+bool DynamicLoaderFreeBSDKernel::KModImageInfo::LoadImageUsingFileAddress(
+ lldb_private::Process *process) {
+ if (IsLoaded())
+ return true;
+
+ if (m_module_sp) {
+ bool changed = false;
+ if (m_module_sp->SetLoadAddress(process->GetTarget(), 0, true, changed))
+ m_stop_id = process->GetStopID();
+ }
+
+ return false;
+}
+
+// Get the head of found_list
+bool DynamicLoaderFreeBSDKernel::ReadKmodsListHeader() {
+ std::lock_guard<decltype(m_mutex)> guard(m_mutex);
+
+ if (m_linker_file_list_struct_addr.IsValid()) {
+ // Get tqh_first struct element from linker_files
+ Status error;
+ addr_t address = m_process->ReadPointerFromMemory(
+ m_linker_file_list_struct_addr.GetLoadAddress(&m_process->GetTarget()),
+ error);
+ if (address != LLDB_INVALID_ADDRESS && error.Success()) {
+ m_linker_file_head_addr = Address(address);
+ } else {
+ m_linker_file_list_struct_addr.Clear();
+ return false;
+ }
+
+ if (!m_linker_file_head_addr.IsValid() ||
+ m_linker_file_head_addr.GetFileAddress() == 0) {
+ m_linker_file_list_struct_addr.Clear();
+ return false;
+ }
+ }
+ return true;
+}
+
+// Parse Kmod info in found_list
+bool DynamicLoaderFreeBSDKernel::ParseKmods(Address linker_files_head_addr) {
+ std::lock_guard<decltype(m_mutex)> guard(m_mutex);
+ KModImageInfo::collection_type linker_files_list;
+ Log *log = GetLog(LLDBLog::DynamicLoader);
+
+ if (!ReadAllKmods(linker_files_head_addr, linker_files_list))
+ return false;
+ LLDB_LOGF(
+ log,
+ "Kmod-changed breakpoint hit, there are %lu kernel modules currently.\n",
+ linker_files_list.size());
+
+ ModuleList &modules = m_process->GetTarget().GetImages();
+ ModuleList remove_modules;
+ ModuleList add_modules;
+
+ for (ModuleSP module : modules.Modules()) {
+ if (is_kernel(module.get()))
+ continue;
+ if (is_kmod(module.get()))
+ remove_modules.AppendIfNeeded(module);
+ }
+
+ m_process->GetTarget().ModulesDidUnload(remove_modules, false);
+
+ for (KModImageInfo &image_info : linker_files_list) {
+ if (m_kld_name_to_uuid.find(image_info.GetName()) !=
+ m_kld_name_to_uuid.end())
+ image_info.SetUUID(m_kld_name_to_uuid[image_info.GetName()]);
+ bool failed_to_load = false;
+ if (!image_info.LoadImageUsingMemoryModule(m_process)) {
+ image_info.LoadImageUsingFileAddress(m_process);
+ failed_to_load = true;
+ } else {
+ m_linker_files_list.push_back(image_info);
+ m_kld_name_to_uuid[image_info.GetName()] = image_info.GetUUID();
+ }
+
+ if (!failed_to_load)
+ add_modules.AppendIfNeeded(image_info.GetModule());
+ }
+ m_process->GetTarget().ModulesDidLoad(add_modules);
+ return true;
+}
+
+// Read all kmod from a given arrays of list
+bool DynamicLoaderFreeBSDKernel::ReadAllKmods(
+ Address linker_fil...
[truncated]
|
aokblast
force-pushed
the
lldb_dynamic_loader_fbsdkernel
branch
4 times, most recently
from
4deac1a to
a61068e
Compare
lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/DynamicLoaderFreeBSDKernel.h
Outdated
Show resolved
Hide resolved
lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/DynamicLoaderFreeBSDKernel.cpp
Outdated
Show resolved
Hide resolved
lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/DynamicLoaderFreeBSDKernel.cpp
Show resolved
Hide resolved
| @@ -0,0 +1,165 @@ | |||
| #ifndef LLDB_SOURCE_PLUGINS_DYNAMICLOADER_FREEBSD_KERNEL_DYNAMICLOADERFREEBSDKERNEL_H | |||
There was a problem hiding this comment.
This file needs a license header.
lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/DynamicLoaderFreeBSDKernel.cpp
Outdated
Show resolved
Hide resolved
lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/DynamicLoaderFreeBSDKernel.cpp
Outdated
Show resolved
Hide resolved
| if (m_linker_file_head_addr.IsValid()) { | ||
| if (!ParseKmods(m_linker_file_head_addr)) | ||
| m_linker_files_list.clear(); | ||
| return true; |
There was a problem hiding this comment.
What does this return true indicate? The ParseKmods invocation 2 lines above this could fail and all of the linker files could be blown away with the following call to clear. Is that not a condition you would want to consider?
There was a problem hiding this comment.
Oh, it does nothing for now because I don't do anything when the kmod reading failed.
I leave it as a remind if I want to log anything when this function execute failed.
I don't know if I should modify it to return void or just reserve it.
| kmods_list.emplace_back(); | ||
| KModImageInfo &kmod_info = kmods_list.back(); | ||
| kmod_info.SetName(kld_filename); | ||
| kmod_info.SetLoadAddress(kld_load_addr); | ||
| kmod_info.SetPath(kld_pathname); | ||
| current_kld = | ||
| m_process->ReadPointerFromMemory(current_kld + kld_off_next, error); |
There was a problem hiding this comment.
Shouldn't you check the result of those memory reads above before mutating state here? Otherwise you may end up with stale or otherwise incorrect information, no?
There was a problem hiding this comment.
Shouldn't you check the result of those memory reads above before mutating state here? Otherwise you may end up with stale or otherwise incorrect information, no?
This will return LLDB_INVALID_ADDRESS if it fails, so you can check the error if you want, or just need to deal with LLDB_INVALID_ADDRESS correctly.
There was a problem hiding this comment.
But yes, check if you got valid strings back for "kld_filename", "kld_pathname" and "kld_load_addr != LLDB_INVALID_ADDRESS would be good idea before adding the to the kmods_list. The "current_kld" should be checked against zero and LLDB_INVALID_ADDRESS in the while loop
There was a problem hiding this comment.
Can I just check the error that was passed into the function?
Update: I found the implementation in ProcessFreeBSDKernel and found that it will change the error parameter instead of return LLDB_INVALID_ADDRESS. So I think checking the error parameter is more reasonable in this case?
There was a problem hiding this comment.
Either one is fine, you could also do both? This is one place where LLDB can be a little frustrating because there are 2 ways to detect the error and it's not obvious which one you should be checking.
lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/DynamicLoaderFreeBSDKernel.cpp
Outdated
Show resolved
Hide resolved
lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/DynamicLoaderFreeBSDKernel.cpp
Outdated
Show resolved
Hide resolved
lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/DynamicLoaderFreeBSDKernel.cpp
Outdated
Show resolved
Hide resolved
lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/DynamicLoaderFreeBSDKernel.cpp
Show resolved
Hide resolved
lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/DynamicLoaderFreeBSDKernel.cpp
Outdated
Show resolved
Hide resolved
lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/DynamicLoaderFreeBSDKernel.cpp
Outdated
Show resolved
Hide resolved
lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/DynamicLoaderFreeBSDKernel.cpp
Show resolved
Hide resolved
| kmods_list.emplace_back(); | ||
| KModImageInfo &kmod_info = kmods_list.back(); | ||
| kmod_info.SetName(kld_filename); | ||
| kmod_info.SetLoadAddress(kld_load_addr); | ||
| kmod_info.SetPath(kld_pathname); | ||
| current_kld = | ||
| m_process->ReadPointerFromMemory(current_kld + kld_off_next, error); |
There was a problem hiding this comment.
But yes, check if you got valid strings back for "kld_filename", "kld_pathname" and "kld_load_addr != LLDB_INVALID_ADDRESS would be good idea before adding the to the kmods_list. The "current_kld" should be checked against zero and LLDB_INVALID_ADDRESS in the while loop
aokblast
force-pushed
the
lldb_dynamic_loader_fbsdkernel
branch
2 times, most recently
from
f760aa7 to
a9febfc
Compare
|
I fix all mentioned issue and test on both x86-64 and arm64. |
| //===-- DynamicLoaderFreeBSDKernel.h | ||
| //------------------------------------------===// |
There was a problem hiding this comment.
Fix this comment line to be a single line and also include the C++ stuff for editors:
//===-- DynamicLoaderFreeBSDKernel.h -----------------------*- C++ -*-===//
|
Needs to be adapted for f2d32dd, (I've applied the change locally for testing, and will just fold it into the patch series if otherwise ready to go.) |
|
Excuse me. If the code works well now, I want to ask if I need to modify the commit message by myself so the message looks tidy, or let somebody landing this pull request modify the message? |
This will be the first substantial change I'd land after LLVM moved to use pull requests, so I'm not completely certain on the process. I have the commits from this pull request on my local machine for testing and when we used Phabricator I'd just squash them, edit the commit message, and push. That raises another point, the author is currently: |
|
Thanks, maybe |
@clayborg this change LGTM for FreeBSD. I'm still not fully sorted on the approach for landing the commits post transition to GitHub. Also see the comment about author name above. |
There is a web page that covers the new submission process: https://llvm.org/docs/GitHub.html I haven't done it yet either |
|
https://llvm.org/docs/GitHub.html#landing-your-change
So I suspect the best way to go here is @aokblast locally squashes these commits into one, updates author if desired, fixes up commit messages if necessary, and then force-pushes to this pull request. Then @clayborg or I push the merge button. |
|
Oh, I think we should also mention GSoC in the commit message. Something like |
|
As my mentor Li-Wen Hsu suggest, It is better to separate this patch into two commits. One for DynamicLoaderFreeBSDKernel class, and another for the modification about ObjectFile.cpp. Should I use 1 commits to include both of these change or separate it as two? |
I am ok with it being one patch since it is related to implementing kernel debugging. |
aokblast
force-pushed
the
lldb_dynamic_loader_fbsdkernel
branch
from
81acacb to
17cad83
Compare
|
OK, I have modified the commit message. |
aokblast
force-pushed
the
lldb_dynamic_loader_fbsdkernel
branch
from
17cad83 to
2d24381
Compare
aokblast
force-pushed
the
lldb_dynamic_loader_fbsdkernel
branch
from
2d24381 to
a05c9b5
Compare
|
Hmm, when I attempt to close it via the GitHub UI I get: I think I can just fetch the commit locally and then push it to the tree |
This patch add dynamicloader plguin for freebsd kernel coredump on lldb. The implementation is by parsing linker_files structure and get all loaded kernel modules. This patch was part of FreeBSD's participation in Google Summer of Code 2023
aokblast
force-pushed
the
lldb_dynamic_loader_fbsdkernel
branch
from
a05c9b5 to
f4d7761
Compare
|
I try to change the committer of my patch and I think it works now. |
|
FYI on our Windows on Arm (aka AArch64) bot we got this warning: And this one on the 32 bit Arm Linux bot: AArch64 Linux was fine. So I assume each one is using one of There is probably a way to print that in a portable way but I don't recall it at the moment. |
|
I'm not sure why @aokblast's reply didn't appear here, but indeed |
|
I send another PR in 68210. I don't know if it is the correct way to submit the solution of this issue. BTW, I have check the whole code so that there is no another code segment have this issue |
This commit is moved from llvm-phabricator.
The implemtation support parsing kernel module for FreeBSD Kernel and has been test on x86-64 and arm64.
In summary, this class parse the linked list resides in the kernel memory that record all kernel module and load the debug symbol file to facilitate debug process