Skip to content

Commit

Permalink
Kernel: Load executables on demand when symbolicating
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
awesomekling committed Mar 2, 2020
1 parent 055344f commit 678c870
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 12 deletions.
9 changes: 7 additions & 2 deletions Kernel/KSyms.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,11 @@ static void load_ksyms_from_data(const ByteBuffer& buffer)
hang();
return;
}

OwnPtr<Process::ELFBundle> elf_bundle;
if (Process::current)
elf_bundle = Process::current->elf_bundle();

struct RecognizedSymbol {
u32 address;
const KSym* ksym;
Expand Down Expand Up @@ -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)";
}
Expand Down
20 changes: 16 additions & 4 deletions Kernel/Process.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -947,7 +947,6 @@ int Process::do_exec(NonnullRefPtr<FileDescription> main_program_description, Ve
#endif
}

m_elf_loader = move(loader);
m_executable = main_program_description->custody();

m_promises = m_execpromises;
Expand Down Expand Up @@ -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";
}
Expand Down Expand Up @@ -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();
{
Expand Down Expand Up @@ -4815,4 +4813,18 @@ void Process::set_tty(TTY* tty)
m_tty = tty;
}

OwnPtr<Process::ELFBundle> Process::elf_bundle() const
{
if (!m_executable)
return nullptr;
auto bundle = make<ELFBundle>();
ASSERT(m_executable->inode().shared_vmobject());
auto& vmobject = *m_executable->inode().shared_vmobject();
bundle->region = MM.allocate_kernel_region_with_vmobject(const_cast<SharedInodeVMObject&>(vmobject), vmobject.size(), "ELF bundle", Region::Access::Read);
if (!bundle->region)
return nullptr;
bundle->elf_loader = make<ELFLoader>(bundle->region->vaddr().as_ptr(), bundle->region->size());
return bundle;
}

}
7 changes: 5 additions & 2 deletions Kernel/Process.h
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,11 @@ class Process : public InlineLinkedListNode<Process> {

Lock& big_lock() { return m_big_lock; }

const ELFLoader* elf_loader() const { return m_elf_loader.ptr(); }
struct ELFBundle {
OwnPtr<Region> region;
OwnPtr<ELFLoader> elf_loader;
};
OwnPtr<ELFBundle> elf_bundle() const;

int icon_id() const { return m_icon_id; }

Expand Down Expand Up @@ -495,7 +499,6 @@ class Process : public InlineLinkedListNode<Process> {
FixedArray<gid_t> m_extra_gids;

RefPtr<ProcessTracer> m_tracer;
OwnPtr<ELFLoader> m_elf_loader;

WeakPtr<Region> m_master_tls_region;
size_t m_master_tls_size { 0 };
Expand Down
9 changes: 5 additions & 4 deletions Kernel/Thread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
}
Expand Down Expand Up @@ -794,6 +794,7 @@ String Thread::backtrace_impl() const
}

auto& process = const_cast<Process&>(this->process());
auto elf_bundle = process.elf_bundle();
ProcessPagingScope paging_scope(process);

uintptr_t stack_ptr = start_frame;
Expand All @@ -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();
Expand Down

0 comments on commit 678c870

Please sign in to comment.