ELFLoader should fail with an error message for unresolved symbols.

Author: https://github.com/awesomekling
Commit: https://github.com/SerenityOS/serenity/commit/16f318408d2

AK/Assertions.h

@@ -1,7 +1,7 @@
 #pragma once
 
 #ifdef SERENITY
-#include "kassert.h"
+#include <Kernel/kassert.h>
 #else
 #include <assert.h>
 #define ASSERT(x) assert(x)

AK/Compiler.h

@@ -0,0 +1,9 @@
+#pragma once
+
+#define PACKED __attribute__ ((packed))
+#define NORETURN __attribute__ ((noreturn))
+#define ALWAYS_INLINE __attribute__ ((always_inline))
+#define NEVER_INLINE __attribute__ ((noinline))
+#define MALLOC_ATTR __attribute__ ((malloc))
+#define PURE __attribute__ ((pure))
+

AK/RetainPtr.h

@@ -1,5 +1,6 @@
 #pragma once
 
+#include "Compiler.h"
 #include "Types.h"
 
 namespace AK {

AK/kmalloc.cpp

@@ -1,10 +1,20 @@
-#include <cstdio>
-#include "SimpleMalloc.h"
 #include "kmalloc.h"
+
+#ifndef SERENITY
+#include <cstdio>
 #include <cstdlib>
+#endif
+
+#if defined(SERENITY) && defined(USERLAND)
+#define USE_SYSTEM_MALLOC
+#endif
 
 #define USE_SYSTEM_MALLOC
 
+#ifndef USE_SYSTEM_MALLOC
+#include "SimpleMalloc.h"
+#endif
+
 #ifdef USE_SYSTEM_MALLOC
 
 extern "C" {

AK/kmalloc.h

@@ -1,7 +1,11 @@
 #pragma once
 
 #ifdef SERENITY
+#ifdef USERLAND
+#include <LibC/stdlib.h>
+#else
 #include <Kernel/kmalloc.h>
+#endif
 #else
 #include <new>
 
@@ -10,7 +14,7 @@
 extern "C" {
 
 void* kcalloc(size_t nmemb, size_t size);
-void* kmalloc(size_t size) __attribute__ ((malloc));
+void* kmalloc(size_t size) MALLOC_ATTR;
 void kfree(void* ptr);
 void* krealloc(void* ptr, size_t size);
 

ELFLoader/ELFImage.h

@@ -147,22 +147,28 @@ inline void ELFImage::forEachSectionOfType(unsigned type, F func) const
 {
     for (unsigned i = 0; i < sectionCount(); ++i) {
         auto& section = this->section(i);
-        if (section.type() == type)
-            func(section);
+        if (section.type() == type) {
+            if (!func(section))
+                break;
+        }
     }
 }
 
 template<typename F>
 inline void ELFImage::RelocationSection::forEachRelocation(F func) const
 {
-    for (unsigned i = 0; i < relocationCount(); ++i)
-        func(relocation(i));
+    for (unsigned i = 0; i < relocationCount(); ++i) {
+        if (!func(relocation(i)))
+            break;
+    }
 }
 
 template<typename F>
 inline void ELFImage::forEachSymbol(F func) const
 {
-    for (unsigned i = 0; i < symbolCount(); ++i)
-        func(symbol(i));
+    for (unsigned i = 0; i < symbolCount(); ++i) {
+        if (!func(symbol(i)))
+            break;
+    }
 }
 

ELFLoader/ELFLoader.cpp

@@ -25,26 +25,38 @@ bool ELFLoader::load()
     if (!m_image->isValid())
         return false;
 
-    layout();
+    if (!layout())
+        return false;
     exportSymbols();
-    performRelocations();
+    if (!performRelocations())
+        return false;
 
     return true;
 }
 
-void ELFLoader::layout()
+bool ELFLoader::layout()
 {
 #ifdef ELFLOADER_DEBUG
     kprintf("[ELFLoader] Layout\n");
 #endif
-    m_image->forEachSectionOfType(SHT_PROGBITS, [this] (const ELFImage::Section& section) {
+    bool failed = false;
+    m_image->forEachSectionOfType(SHT_PROGBITS, [this, &failed] (const ELFImage::Section& section) {
 #ifdef ELFLOADER_DEBUG
         kprintf("[ELFLoader] Allocating progbits section: %s\n", section.name());
 #endif
+        if (!section.size())
+            return true;
         char* ptr = m_execSpace.allocateArea(section.name(), section.size());
+        if (!ptr) {
+            kprintf("ELFLoader: failed to allocate section '%s'\n", section.name());
+            failed = true;
+            return false;
+        }
         memcpy(ptr, section.rawData(), section.size());
         m_sections.set(section.name(), move(ptr));
+        return true;
     });
+    return !failed;
 }
 
 void* ELFLoader::lookup(const ELFImage::Symbol& symbol)
@@ -67,23 +79,30 @@ char* ELFLoader::areaForSectionName(const char* name)
     return nullptr;
 }
 
-void ELFLoader::performRelocations()
+bool ELFLoader::performRelocations()
 {
 #ifdef ELFLOADER_DEBUG
     kprintf("[ELFLoader] Performing relocations\n");
 #endif
 
-    m_image->forEachSectionOfType(SHT_PROGBITS, [this] (const ELFImage::Section& section) {
+    bool failed = false;
+
+    m_image->forEachSectionOfType(SHT_PROGBITS, [this, &failed] (const ELFImage::Section& section) -> bool {
         auto& relocations = section.relocations();
         if (relocations.isUndefined())
-            return;
-        relocations.forEachRelocation([this, section] (const ELFImage::Relocation& relocation) {
+            return true;
+        relocations.forEachRelocation([this, section, &failed] (const ELFImage::Relocation& relocation) {
             auto symbol = relocation.symbol();
             auto& patchPtr = *reinterpret_cast<ptrdiff_t*>(areaForSection(section) + relocation.offset());
 
             switch (relocation.type()) {
             case R_386_PC32: {
                 char* targetPtr = (char*)lookup(symbol);
+                if (!targetPtr) {
+                    kprintf("ELFLoader: unresolved symbol '%s'\n", symbol.name());
+                    failed = true;
+                    return false;
+                }
                 ptrdiff_t relativeOffset = (char*)targetPtr - ((char*)&patchPtr + 4);
 #ifdef ELFLOADER_DEBUG
                 kprintf("[ELFLoader] Relocate PC32:  offset=%x, symbol=%u(%s) value=%x target=%p, offset=%d\n",
@@ -115,8 +134,11 @@ void ELFLoader::performRelocations()
                 ASSERT_NOT_REACHED();
                 break;
             }
+            return true;
         });
+        return !failed;
     });
+    return !failed;
 }
 
 void ELFLoader::exportSymbols()
@@ -128,6 +150,7 @@ void ELFLoader::exportSymbols()
         if (symbol.type() == STT_FUNC)
             m_execSpace.addSymbol(symbol.name(), areaForSection(symbol.section()) + symbol.value(), symbol.size());
         // FIXME: What about other symbol types?
+        return true;
     });
 }
 

ELFLoader/ELFLoader.h

@@ -19,8 +19,8 @@ public:
     bool load();
 
 private:
-    void layout();
-    void performRelocations();
+    bool layout();
+    bool performRelocations();
     void exportSymbols();
     void* lookup(const ELFImage::Symbol&);
     char* areaForSection(const ELFImage::Section&);

Kernel/Task.cpp

@@ -201,6 +201,7 @@ Task* Task::create(const String& path, uid_t uid, gid_t gid, pid_t parentPID)
     bool success = space.loadELF(move(elfData));
     if (!success) {
         delete t;
+        kprintf("Failure loading ELF %s\n", path.characters());
         return nullptr;
     }
 

Kernel/kprintf.h

@@ -1,5 +1,7 @@
 #pragma once
 
+#include <AK/Compiler.h>
+
 int kprintf(const char *fmt, ...);
 int ksprintf(char* buf, const char *fmt, ...);
 

LibC/types.h

@@ -12,7 +12,7 @@ typedef signed char signed_byte;
 
 typedef dword uid_t;
 typedef dword gid_t;
-typedef dword pid_t;
+typedef int pid_t;
 
 typedef dword size_t;
 typedef signed_dword ssize_t;

LibC/unistd.cpp

@@ -9,12 +9,12 @@ uid_t getuid()
     return Syscall::invoke(Syscall::PosixGetuid);
 }
 
-uid_t getgid()
+gid_t getgid()
 {
     return Syscall::invoke(Syscall::PosixGetgid);
 }
 
-uid_t getpid()
+pid_t getpid()
 {
     return Syscall::invoke(Syscall::PosixGetpid);
 }

Userland/Makefile

@@ -20,7 +20,7 @@ FLAVOR_FLAGS = -fomit-frame-pointer -mregparm=3 -march=i386 -m32 -fno-exceptions
 OPTIMIZATION_FLAGS = -Os -fno-asynchronous-unwind-tables
 INCLUDE_FLAGS = -I.. -I.
 
-DEFINES = -DSERENITY -DSANITIZE_PTRS
+DEFINES = -DSERENITY -DSANITIZE_PTRS -DUSERLAND
 
 CXXFLAGS = $(WARNING_FLAGS) $(OPTIMIZATION_FLAGS) $(USERLAND_FLAGS) $(FLAVOR_FLAGS) $(ARCH_FLAGS) $(STANDARD_FLAGS) $(INCLUDE_FLAGS) $(DEFINES)
 CXX = g++