From 678c87087d73812105b5682bb6eb27148d3b7dcb Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Mon, 2 Mar 2020 10:40:40 +0100 Subject: [PATCH] Kernel: Load executables on demand when symbolicating Previously we would map the entire executable of a program in its own address space (but make it unavailable to userspace code.) This patch removes that and changes the symbolication code to remap the executable on demand (and into the kernel's own address space instead of the process address space.) This opens up a couple of further simplifications that will follow. --- Kernel/KSyms.cpp | 9 +++++++-- Kernel/Process.cpp | 20 ++++++++++++++++---- Kernel/Process.h | 7 +++++-- Kernel/Thread.cpp | 9 +++++---- 4 files changed, 33 insertions(+), 12 deletions(-) diff --git a/Kernel/KSyms.cpp b/Kernel/KSyms.cpp index 2e70264a15..e7b55d6896 100644 --- a/Kernel/KSyms.cpp +++ b/Kernel/KSyms.cpp @@ -127,6 +127,11 @@ static void load_ksyms_from_data(const ByteBuffer& buffer) hang(); return; } + + OwnPtr elf_bundle; + if (Process::current) + elf_bundle = Process::current->elf_bundle(); + struct RecognizedSymbol { u32 address; const KSym* ksym; @@ -154,8 +159,8 @@ static void load_ksyms_from_data(const ByteBuffer& buffer) if (!symbol.address) break; if (!symbol.ksym) { - if (Process::current && Process::current->elf_loader() && Process::current->elf_loader()->has_symbols()) { - dbg() << String::format("%p", symbol.address) << " " << Process::current->elf_loader()->symbolicate(symbol.address); + if (elf_bundle && elf_bundle->elf_loader->has_symbols()) { + dbg() << String::format("%p", symbol.address) << " " << elf_bundle->elf_loader->symbolicate(symbol.address); } else { dbg() << String::format("%p", symbol.address) << " (no ELF symbols for process)"; } diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp index fd3b0fc59a..00b172a440 100644 --- a/Kernel/Process.cpp +++ b/Kernel/Process.cpp @@ -947,7 +947,6 @@ int Process::do_exec(NonnullRefPtr main_program_description, Ve #endif } - m_elf_loader = move(loader); m_executable = main_program_description->custody(); m_promises = m_execpromises; @@ -1516,8 +1515,8 @@ void Process::crash(int signal, u32 eip) if (eip >= 0xc0000000 && ksyms_ready) { auto* ksym = ksymbolicate(eip); dbg() << "\033[31;1m" << String::format("%p", eip) << " " << (ksym ? demangle(ksym->name) : "(k?)") << " +" << (ksym ? eip - ksym->address : 0) << "\033[0m\n"; - } else if (m_elf_loader) { - dbg() << "\033[31;1m" << String::format("%p", eip) << " " << m_elf_loader->symbolicate(eip) << "\033[0m\n"; + } else if (auto elf_bundle = this->elf_bundle()) { + dbg() << "\033[31;1m" << String::format("%p", eip) << " " << elf_bundle->elf_loader->symbolicate(eip) << "\033[0m\n"; } else { dbg() << "\033[31;1m" << String::format("%p", eip) << " (?)\033[0m\n"; } @@ -3051,7 +3050,6 @@ void Process::finalize() m_cwd = nullptr; m_root_directory = nullptr; m_root_directory_relative_to_global_root = nullptr; - m_elf_loader = nullptr; disown_all_shared_buffers(); { @@ -4815,4 +4813,18 @@ void Process::set_tty(TTY* tty) m_tty = tty; } +OwnPtr Process::elf_bundle() const +{ + if (!m_executable) + return nullptr; + auto bundle = make(); + ASSERT(m_executable->inode().shared_vmobject()); + auto& vmobject = *m_executable->inode().shared_vmobject(); + bundle->region = MM.allocate_kernel_region_with_vmobject(const_cast(vmobject), vmobject.size(), "ELF bundle", Region::Access::Read); + if (!bundle->region) + return nullptr; + bundle->elf_loader = make(bundle->region->vaddr().as_ptr(), bundle->region->size()); + return bundle; +} + } diff --git a/Kernel/Process.h b/Kernel/Process.h index 96360398f5..66424c9c50 100644 --- a/Kernel/Process.h +++ b/Kernel/Process.h @@ -386,7 +386,11 @@ public: Lock& big_lock() { return m_big_lock; } - const ELFLoader* elf_loader() const { return m_elf_loader.ptr(); } + struct ELFBundle { + OwnPtr region; + OwnPtr elf_loader; + }; + OwnPtr elf_bundle() const; int icon_id() const { return m_icon_id; } @@ -495,7 +499,6 @@ private: FixedArray m_extra_gids; RefPtr m_tracer; - OwnPtr m_elf_loader; WeakPtr m_master_tls_region; size_t m_master_tls_size { 0 }; diff --git a/Kernel/Thread.cpp b/Kernel/Thread.cpp index e2a9e7e2bd..bf7d1df503 100644 --- a/Kernel/Thread.cpp +++ b/Kernel/Thread.cpp @@ -754,7 +754,7 @@ struct RecognizedSymbol { const KSym* ksym; }; -static bool symbolicate(const RecognizedSymbol& symbol, const Process& process, StringBuilder& builder) +static bool symbolicate(const RecognizedSymbol& symbol, const Process& process, StringBuilder& builder, Process::ELFBundle* elf_bundle) { if (!symbol.address) return false; @@ -764,8 +764,8 @@ static bool symbolicate(const RecognizedSymbol& symbol, const Process& process, if (!is_user_address(VirtualAddress(symbol.address))) { builder.append("0xdeadc0de\n"); } else { - if (!Scheduler::is_active() && process.elf_loader() && process.elf_loader()->has_symbols()) - builder.appendf("%p %s\n", symbol.address, process.elf_loader()->symbolicate(symbol.address).characters()); + if (!Scheduler::is_active() && elf_bundle && elf_bundle->elf_loader->has_symbols()) + builder.appendf("%p %s\n", symbol.address, elf_bundle->elf_loader->symbolicate(symbol.address).characters()); else builder.appendf("%p\n", symbol.address); } @@ -794,6 +794,7 @@ String Thread::backtrace_impl() const } auto& process = const_cast(this->process()); + auto elf_bundle = process.elf_bundle(); ProcessPagingScope paging_scope(process); uintptr_t stack_ptr = start_frame; @@ -815,7 +816,7 @@ String Thread::backtrace_impl() const StringBuilder builder; for (auto& symbol : recognized_symbols) { - if (!symbolicate(symbol, process, builder)) + if (!symbolicate(symbol, process, builder, elf_bundle.ptr())) break; } return builder.to_string();