Use ELF program headers to load executables smarter.

This turned out way better than the old code. ELF loading is now quite
straightforward, and we don't need the weird concept of subregions anymore.

Next step is to respect the is_writable flag.

ELFLoader/ELFLoader.cpp

@@ -40,35 +40,23 @@ bool ELFLoader::layout()
     kprintf("ELFLoader: Layout\n");
 #endif
 
+    bool failed = false;
     m_image->for_each_program_header([&] (const ELFImage::ProgramHeader& program_header) {
         if (program_header.type() != PT_LOAD)
             return;
 #ifdef ELFLOADER_DEBUG
         kprintf("PH: L%x %u r:%u w:%u\n", program_header.laddr().get(), program_header.size_in_memory(), program_header.is_readable(), program_header.is_writable());
 #endif
+        m_execSpace.allocate_section(program_header.laddr(), program_header.size_in_memory(), program_header.alignment(), program_header.is_readable(), program_header.is_writable());
     });
 
-    bool failed = false;
-    dword highestOffset = 0;
-    dword sizeNeeded = 0;
-    m_image->forEachSection([&] (auto& section) {
-        if (section.offset() > highestOffset) {
-            highestOffset = section.offset();
-            sizeNeeded = highestOffset + section.size();
-        }
-    });
-#ifdef ELFLOADER_DEBUG
-    kprintf("ELFLoader: Highest section offset: %u, Size needed: %u\n", highestOffset, sizeNeeded);
-#endif
-    m_execSpace.allocateUniverse(sizeNeeded);
-
     m_image->forEachSectionOfType(SHT_PROGBITS, [this, &failed] (const ELFImage::Section& section) {
 #ifdef ELFLOADER_DEBUG
-        kprintf("ELFLoader: Allocating progbits section: %s\n", section.name());
+        kprintf("ELFLoader: Copying progbits section: %s\n", section.name());
 #endif
         if (!section.size())
             return true;
-        char* ptr = m_execSpace.allocateArea(section.name(), section.size(), section.offset(), LinearAddress(section.address()));
+        char* ptr = (char*)section.address();
         if (!ptr) {
             kprintf("ELFLoader: failed to allocate section '%s'\n", section.name());
             failed = true;
@@ -80,11 +68,11 @@ bool ELFLoader::layout()
     });
     m_image->forEachSectionOfType(SHT_NOBITS, [this, &failed] (const ELFImage::Section& section) {
 #ifdef ELFLOADER_DEBUG
-        kprintf("ELFLoader: Allocating nobits section: %s\n", section.name());
+        kprintf("ELFLoader: Copying nobits section: %s\n", section.name());
 #endif
         if (!section.size())
             return true;
-        char* ptr = m_execSpace.allocateArea(section.name(), section.size(), section.offset(), LinearAddress(section.address()));
+        char* ptr = (char*)section.address();
         if (!ptr) {
             kprintf("ELFLoader: failed to allocate section '%s'\n", section.name());
             failed = true;

ELFLoader/ExecSpace.cpp

@@ -15,10 +15,6 @@ ExecSpace::ExecSpace()
 
 ExecSpace::~ExecSpace()
 {
-    if (!hookableAlloc) {
-        for (auto& area : m_areas)
-            kfree(area->memory);
-    }
 }
 
 #ifdef SERENITY
@@ -110,20 +106,14 @@ void ExecSpace::allocateUniverse(size_t size)
         m_universe = static_cast<char*>(kmalloc(size));
 }
 
-char* ExecSpace::allocateArea(String&& name, unsigned size, dword offset, LinearAddress laddr)
+bool ExecSpace::allocate_section(LinearAddress laddr, size_t size, size_t alignment, bool is_readable, bool is_writable)
 {
-    ASSERT(m_universe);
+    ASSERT(alloc_section_hook);
-    char* ptr = m_universe + offset;
+    char namebuf[16];
-    m_areas.append(make<Area>(move(name), offset, ptr, size, laddr));
+    ksprintf(namebuf, "elf-%s%s", is_readable ? "r" : "", is_writable ? "w" : "");
-    return ptr;
+    auto* ptr = static_cast<char*>(alloc_section_hook(laddr, size, alignment, is_readable, is_writable, namebuf));
-}
+    m_allocated_regions.append(ptr);
-
+    return true;
-void ExecSpace::forEachArea(Function<void(const String& name, dword offset, size_t size, LinearAddress)> callback)
-{
-    for (auto& a : m_areas) {
-        auto& area = *a;
-        callback(area.name, area.offset, area.size, area.laddr);
-    }
 }
 
 void ExecSpace::addSymbol(String&& name, char* ptr, unsigned size)

ELFLoader/ExecSpace.h

@@ -5,28 +5,12 @@
 #include <AK/MappedFile.h>
 #include <AK/OwnPtr.h>
 #include <AK/Vector.h>
+#include "types.h"
 
 class ELFLoader;
 
 class ExecSpace {
 public:
-    struct Area {
-        Area(String&& n, dword o, char* m, unsigned s, LinearAddress l)
-            : name(move(n))
-            , offset(o)
-            , memory(m)
-            , size(s)
-            , laddr(l)
-        {
-        }
-
-        String name;
-        dword offset { 0 };
-        char* memory { 0 };
-        unsigned size { 0 };
-        LinearAddress laddr;
-    };
-
     struct PtrAndSize {
         PtrAndSize() { }
         PtrAndSize(char* p, unsigned s)
@@ -43,6 +27,7 @@ public:
     ~ExecSpace();
 
     Function<void*(const String&, size_t)> hookableAlloc;
+    Function<void*(LinearAddress, size_t, size_t, bool, bool, const String&)> alloc_section_hook;
 
 #ifdef SERENITY
     bool loadELF(ByteBuffer&&);
@@ -52,17 +37,16 @@ public:
 
     char* symbolPtr(const char* name);
 
-    char* allocateArea(String&& name, unsigned size, dword offset, LinearAddress);
     void addSymbol(String&& name, char* ptr, unsigned size);
 
     void allocateUniverse(size_t);
 
-    void forEachArea(Function<void(const String& name, dword offset, size_t size, LinearAddress)>);
+    bool allocate_section(LinearAddress, size_t, size_t alignment, bool is_readable, bool is_writable);
 
 private:
     void initializeBuiltins();
 
-    Vector<OwnPtr<Area>> m_areas;
+    Vector<char*> m_allocated_regions;
     HashMap<String, PtrAndSize> m_symbols;
     char* m_universe { nullptr };
 };

Kernel/MemoryManager.cpp

@@ -385,49 +385,6 @@ bool MemoryManager::unmapRegion(Process& process, Region& region)
     return true;
 }
 
-bool MemoryManager::unmapSubregion(Process& process, Subregion& subregion)
-{
-    InterruptDisabler disabler;
-    size_t numPages = subregion.size / PAGE_SIZE;
-    ASSERT(numPages);
-    for (size_t i = 0; i < numPages; ++i) {
-        auto laddr = subregion.linearAddress.offset(i * PAGE_SIZE);
-        auto pte = ensurePTE(process.m_page_directory, laddr);
-        pte.setPhysicalPageBase(0);
-        pte.setPresent(false);
-        pte.setWritable(false);
-        pte.setUserAllowed(false);
-        flushTLB(laddr);
-#ifdef MM_DEBUG
-        //dbgprintf("MM: >> Unmapped subregion %s L%x => P%x <<\n", subregion.name.characters(), laddr, zone.m_pages[i].get());
-#endif
-    }
-    return true;
-}
-
-bool MemoryManager::mapSubregion(Process& process, Subregion& subregion)
-{
-    InterruptDisabler disabler;
-    auto& region = *subregion.region;
-    auto& zone = *region.zone;
-    size_t firstPage = subregion.offset / PAGE_SIZE;
-    size_t numPages = subregion.size / PAGE_SIZE;
-    ASSERT(numPages);
-    for (size_t i = 0; i < numPages; ++i) {
-        auto laddr = subregion.linearAddress.offset(i * PAGE_SIZE);
-        auto pte = ensurePTE(process.m_page_directory, laddr);
-        pte.setPhysicalPageBase(zone.m_pages[firstPage + i].get());
-        pte.setPresent(true);
-        pte.setWritable(true);
-        pte.setUserAllowed(true);
-        flushTLB(laddr);
-#ifdef MM_DEBUG
-        //dbgprintf("MM: >> Mapped subregion %s L%x => P%x (%u into region)\n", subregion.name.characters(), laddr, zone.m_pages[firstPage + i].get(), subregion.offset);
-#endif
-    }
-    return true;
-}
-
 bool MemoryManager::mapRegion(Process& process, Region& region)
 {
     map_region_at_address(process.m_page_directory, region, region.linearAddress, true);

Kernel/MemoryManager.h

@@ -48,17 +48,6 @@ struct Region : public Retainable<Region> {
     String name;
 };
 
-struct Subregion {
-    Subregion(Region&, dword offset, size_t, LinearAddress, String&& name);
-    ~Subregion();
-
-    RetainPtr<Region> region;
-    dword offset;
-    size_t size { 0 };
-    LinearAddress linearAddress;
-    String name;
-};
-
 #define MM MemoryManager::the()
 
 class MemoryManager {
@@ -75,9 +64,6 @@ public:
 
     RetainPtr<Zone> createZone(size_t);
 
-    bool mapSubregion(Process&, Subregion&);
-    bool unmapSubregion(Process&, Subregion&);
-
     bool mapRegion(Process&, Region&);
     bool unmapRegion(Process&, Region&);
 

Kernel/ProcFileSystem.cpp

@@ -49,7 +49,7 @@ ByteBuffer procfs$pid_vm(Process& process)
 {
     ProcessInspectionScope scope(process);
     char* buffer;
-    auto stringImpl = StringImpl::createUninitialized(80 + process.regionCount() * 80 + 80 + process.subregionCount() * 80, buffer);
+    auto stringImpl = StringImpl::createUninitialized(80 + process.regionCount() * 80, buffer);
     memset(buffer, 0, stringImpl->length());
     char* ptr = buffer;
     ptr += ksprintf(ptr, "BEGIN       END         SIZE        NAME\n");
@@ -60,18 +60,6 @@ ByteBuffer procfs$pid_vm(Process& process)
             region->size,
             region->name.characters());
     }
-    if (process.subregionCount()) {
-        ptr += ksprintf(ptr, "\nREGION    OFFSET    BEGIN       END         SIZE        NAME\n");
-        for (auto& subregion : process.subregions()) {
-            ptr += ksprintf(ptr, "%x  %x  %x -- %x    %x    %s\n",
-                subregion->region->linearAddress.get(),
-                subregion->offset,
-                subregion->linearAddress.get(),
-                subregion->linearAddress.offset(subregion->size - 1).get(),
-                subregion->size,
-                subregion->name.characters());
-        }
-    }
     *ptr = '\0';
     return ByteBuffer::copy((byte*)buffer, ptr - buffer);
 }

Kernel/Process.cpp

@@ -134,19 +134,26 @@ Vector<Process*> Process::allProcesses()
     return processes;
 }
 
-Region* Process::allocateRegion(size_t size, String&& name)
+Region* Process::allocate_region(LinearAddress laddr, size_t size, String&& name, bool is_readable, bool is_writable)
 {
     // FIXME: This needs sanity checks. What if this overlaps existing regions?
+    if (laddr.is_null()) {
+        laddr = m_nextRegion;
+        m_nextRegion = m_nextRegion.offset(size).offset(PAGE_SIZE);
+    }
+
+    laddr.mask(0xfffff000);
 
     auto zone = MM.createZone(size);
     ASSERT(zone);
-    m_regions.append(adopt(*new Region(m_nextRegion, size, move(zone), move(name))));
+
-    m_nextRegion = m_nextRegion.offset(size).offset(16384);
+    m_regions.append(adopt(*new Region(laddr, size, move(zone), move(name))));
+
     MM.mapRegion(*this, *m_regions.last());
     return m_regions.last().ptr();
 }
 
-bool Process::deallocateRegion(Region& region)
+bool Process::deallocate_region(Region& region)
 {
     InterruptDisabler disabler;
     for (size_t i = 0; i < m_regions.size(); ++i) {
@@ -183,7 +190,7 @@ void* Process::sys$mmap(void* addr, size_t size)
     InterruptDisabler disabler;
     // FIXME: Implement mapping at a client-preferred address.
     ASSERT(addr == nullptr);
-    auto* region = allocateRegion(size, "mmap");
+    auto* region = allocate_region(LinearAddress(), size, "mmap");
     if (!region)
         return (void*)-1;
     MM.mapRegion(*this, *region);
@@ -196,7 +203,7 @@ int Process::sys$munmap(void* addr, size_t size)
     auto* region = regionFromRange(LinearAddress((dword)addr), size);
     if (!region)
         return -1;
-    if (!deallocateRegion(*region))
+    if (!deallocate_region(*region))
         return -1;
     return 0;
 }
@@ -229,17 +236,6 @@ Process* Process::fork(RegisterDump& regs)
         dbgprintf("fork: cloning Region{%p}\n", region.ptr());
 #endif
         auto cloned_region = region->clone();
-        // FIXME: Move subregions into Region?
-        for (auto& subregion : m_subregions) {
-            if (subregion->region.ptr() != region.ptr())
-                continue;
-#ifdef FORK_DEBUG
-            dbgprintf("fork: cloning Subregion{%p}\n", subregion.ptr());
-#endif
-            auto cloned_subregion = make<Subregion>(*cloned_region, subregion->offset, subregion->size, subregion->linearAddress, String(subregion->name));
-            child->m_subregions.append(move(cloned_subregion));
-            MM.mapSubregion(*child, *child->m_subregions.last());
-        }
         child->m_regions.append(move(cloned_region));
         MM.mapRegion(*child, *child->m_regions.last());
     }
@@ -312,18 +308,16 @@ int Process::exec(const String& path, Vector<String>&& arguments, Vector<String>
         InterruptDisabler disabler;
         // Okay, here comes the sleight of hand, pay close attention..
         auto old_regions = move(m_regions);
-        auto old_subregions = move(m_subregions);
         old_page_directory = m_page_directory;
         new_page_directory = reinterpret_cast<PageDirectory*>(kmalloc_page_aligned(sizeof(PageDirectory)));
         MM.populate_page_directory(*new_page_directory);
         m_page_directory = new_page_directory;
         MM.enter_process_paging_scope(*this);
-        space.hookableAlloc = [&] (const String& name, size_t size) {
+        space.alloc_section_hook = [&] (LinearAddress laddr, size_t size, size_t alignment, bool is_readable, bool is_writable, const String& name) {
-            if (!size)
+            ASSERT(size);
-                return (void*)nullptr;
             size = ((size / 4096) + 1) * 4096; // FIXME: Use ceil_div?
-            region = allocateRegion(size, String(name));
+            region = allocate_region(laddr, size, String(name), is_readable, is_writable);
-            return (void*)region->linearAddress.get();
+            return laddr.asPtr();
         };
         bool success = space.loadELF(move(elfData));
         if (!success) {
@@ -331,29 +325,16 @@ int Process::exec(const String& path, Vector<String>&& arguments, Vector<String>
             MM.enter_process_paging_scope(*this);
             MM.release_page_directory(*new_page_directory);
             m_regions = move(old_regions);
-            m_subregions = move(old_subregions);
             kprintf("sys$execve: Failure loading %s\n", path.characters());
             return -ENOEXEC;
         }
 
-        space.forEachArea([&] (const String& name, dword offset, size_t size, LinearAddress laddr) {
-            if (laddr.isNull())
-                return;
-            dword roundedOffset = offset & 0xfffff000;
-            size_t roundedSize = 4096 * ceilDiv((offset - roundedOffset) + size, 4096u);
-            LinearAddress roundedLaddr = laddr;
-            roundedLaddr.mask(0xfffff000);
-            m_subregions.append(make<Subregion>(*region, roundedOffset, roundedSize, roundedLaddr, String(name)));
-            MM.mapSubregion(*this, *m_subregions.last());
-        });
-
         entry_eip = (dword)space.symbolPtr("_start");
         if (!entry_eip) {
             m_page_directory = old_page_directory;
             MM.enter_process_paging_scope(*this);
             MM.release_page_directory(*new_page_directory);
             m_regions = move(old_regions);
-            m_subregions = move(old_subregions);
             return -ENOEXEC;
         }
     }
@@ -375,7 +356,7 @@ int Process::exec(const String& path, Vector<String>&& arguments, Vector<String>
     m_tss.fs = 0x23;
     m_tss.ss = 0x23;
     m_tss.cr3 = (dword)m_page_directory;
-    auto* stack_region = allocateRegion(defaultStackSize, "stack");
+    auto* stack_region = allocate_region(LinearAddress(), defaultStackSize, "stack");
     ASSERT(stack_region);
     m_stackTop3 = stack_region->linearAddress.offset(defaultStackSize).get() & 0xfffffff8;
     m_tss.esp = m_stackTop3;
@@ -512,7 +493,7 @@ Process* Process::create_user_process(const String& path, uid_t uid, gid_t gid,
 
 int Process::sys$get_environment(char*** environ)
 {
-    auto* region = allocateRegion(4096, "environ");
+    auto* region = allocate_region(LinearAddress(), PAGE_SIZE, "environ");
     if (!region)
         return -ENOMEM;
     MM.mapRegion(*this, *region);
@@ -531,7 +512,7 @@ int Process::sys$get_environment(char*** environ)
 
 int Process::sys$get_arguments(int* argc, char*** argv)
 {
-    auto* region = allocateRegion(4096, "argv");
+    auto* region = allocate_region(LinearAddress(), PAGE_SIZE, "argv");
     if (!region)
         return -ENOMEM;
     MM.mapRegion(*this, *region);
@@ -656,7 +637,7 @@ Process::Process(String&& name, uid_t uid, gid_t gid, pid_t parentPID, RingLevel
         if (fork_parent) {
             m_stackTop3 = fork_parent->m_stackTop3;
         } else {
-            auto* region = allocateRegion(defaultStackSize, "stack");
+            auto* region = allocate_region(LinearAddress(), defaultStackSize, "stack");
             ASSERT(region);
             m_stackTop3 = region->linearAddress.offset(defaultStackSize).get() & 0xfffffff8;
             m_tss.esp = m_stackTop3;
@@ -703,18 +684,6 @@ void Process::dumpRegions()
             region->size,
             region->name.characters());
     }
-
-    kprintf("Process %s(%u) subregions:\n", name().characters(), pid());
-    kprintf("REGION    OFFSET    BEGIN       END         SIZE        NAME\n");
-    for (auto& subregion : m_subregions) {
-        kprintf("%x  %x  %x -- %x    %x    %s\n",
-            subregion->region->linearAddress.get(),
-            subregion->offset,
-            subregion->linearAddress.get(),
-            subregion->linearAddress.offset(subregion->size - 1).get(),
-            subregion->size,
-            subregion->name.characters());
-    }
 }
 
 void Process::notify_waiters(pid_t waitee, int exit_status, int signal)
@@ -1303,19 +1272,6 @@ Region::~Region()
 {
 }
 
-Subregion::Subregion(Region& r, dword o, size_t s, LinearAddress l, String&& n)\
-    : region(r)
-    , offset(o)
-    , size(s)
-    , linearAddress(l)
-    , name(move(n))
-{
-}
-
-Subregion::~Subregion()
-{
-}
-
 bool Process::isValidAddressForKernel(LinearAddress laddr) const
 {
     // We check extra carefully here since the first 4MB of the address space is identity-mapped.

Kernel/Process.h

@@ -12,7 +12,6 @@
 class FileHandle;
 class PageDirectory;
 class Region;
-class Subregion;
 class Zone;
 
 class Process : public InlineLinkedListNode<Process> {
@@ -130,8 +129,6 @@ public:
 
     size_t regionCount() const { return m_regions.size(); }
     const Vector<RetainPtr<Region>>& regions() const { return m_regions; }
-    size_t subregionCount() const { return m_regions.size(); }
-    const Vector<OwnPtr<Subregion>>& subregions() const { return m_subregions; }
     void dumpRegions();
 
     void didSchedule() { ++m_timesScheduled; }
@@ -203,14 +200,12 @@ private:
 
     TTY* m_tty { nullptr };
 
-    Region* allocateRegion(size_t, String&& name);
+    Region* allocate_region(LinearAddress, size_t, String&& name, bool is_readable = true, bool is_writable = true);
-    Region* allocateRegion(size_t, String&& name, LinearAddress);
+    bool deallocate_region(Region& region);
-    bool deallocateRegion(Region& region);
 
     Region* regionFromRange(LinearAddress, size_t);
 
     Vector<RetainPtr<Region>> m_regions;
-    Vector<OwnPtr<Subregion>> m_subregions;
 
     // FIXME: Implement some kind of ASLR?
     LinearAddress m_nextRegion;

Kernel/types.h

@@ -84,7 +84,7 @@ public:
     LinearAddress() { }
     explicit LinearAddress(dword address) : m_address(address) { }
 
-    bool isNull() const { return m_address == 0; }
+    bool is_null() const { return m_address == 0; }
 
     LinearAddress offset(dword o) const { return LinearAddress(m_address + o); }
     dword get() const { return m_address; }