Kernel+Userland: Make the stack alignment comply with the System V ABI

The System V ABI for both x86 and x86_64 requires that the stack pointer
is 16-byte aligned on entry. Previously we did not align the stack
pointer properly.

As far as "main" was concerned the stack alignment was correct even
without this patch due to how the C++ _start function and the kernel
interacted, i.e. the kernel misaligned the stack as far as the ABI
was concerned but that misalignment (read: it was properly aligned for
a regular function call - but misaligned in terms of what the ABI
dictates) was actually expected by our _start function.

8 files changed, 69 insertions, 17 deletions

diff --git a/Kernel/Syscalls/execve.cpp b/Kernel/Syscalls/execve.cpp
index 297a1b6c6e..94ae1c373b 100644
--- a/Kernel/Syscalls/execve.cpp
+++ b/Kernel/Syscalls/execve.cpp
@@ -147,10 +147,10 @@ static KResultOr<FlatPtr> make_userspace_context_for_main_thread([[maybe_unused]
     regs.rsi = argv;
     regs.rdx = envp;
 #endif
-    push_on_new_stack(0); // return address
 
-    VERIFY((new_sp + sizeof(void*)) % 16 == 0);
+    VERIFY(new_sp % 16 == 0);
 
+    // FIXME: The way we're setting up the stack and passing arguments to the entry point isn't ABI-compliant
     return new_sp;
 }
 
diff --git a/Userland/DynamicLoader/CMakeLists.txt b/Userland/DynamicLoader/CMakeLists.txt
index 3e95c8a814..af20a733a2 100644
--- a/Userland/DynamicLoader/CMakeLists.txt
+++ b/Userland/DynamicLoader/CMakeLists.txt
@@ -10,10 +10,10 @@ file(GLOB LIBC_SOURCES2 "../Libraries/LibC/*/*.cpp")
 
 if ("${SERENITY_ARCH}" STREQUAL "i686")
     file(GLOB LIBC_SOURCES3 "../Libraries/LibC/arch/i386/*.S")
-    set(ELF_SOURCES ${ELF_SOURCES} ../Libraries/LibELF/Arch/i386/plt_trampoline.S)
+    set(ELF_SOURCES ${ELF_SOURCES} ../Libraries/LibELF/Arch/i386/entry.S ../Libraries/LibELF/Arch/i386/plt_trampoline.S)
 elseif ("${SERENITY_ARCH}" STREQUAL "x86_64")
     file(GLOB LIBC_SOURCES3 "../Libraries/LibC/arch/x86_64/*.S")
-    set(ELF_SOURCES ${ELF_SOURCES} ../Libraries/LibELF/Arch/x86_64/plt_trampoline.S)
+    set(ELF_SOURCES ${ELF_SOURCES} ../Libraries/LibELF/Arch/x86_64/entry.S ../Libraries/LibELF/Arch/x86_64/plt_trampoline.S)
 endif()
 
 file(GLOB LIBSYSTEM_SOURCES "../Libraries/LibSystem/*.cpp")
diff --git a/Userland/DynamicLoader/main.cpp b/Userland/DynamicLoader/main.cpp
index 6fecfbc60f..6156151aa6 100644
--- a/Userland/DynamicLoader/main.cpp
+++ b/Userland/DynamicLoader/main.cpp
@@ -94,9 +94,15 @@ this helper program directly.
 extern "C" {
 
 // The compiler expects a previous declaration
-void _start(int, char**, char**);
+void _entry(int, char**, char**);
 
-void _start(int argc, char** argv, char** envp)
+asm(
+    ".globl _start\n"
+    "_start:\n"
+    "push $0\n"
+    "jmp _entry@plt\n");
+
+void _entry(int argc, char** argv, char** envp)
 {
     char** env;
     for (env = envp; *env; ++env) {
diff --git a/Userland/Libraries/LibC/CMakeLists.txt b/Userland/Libraries/LibC/CMakeLists.txt
index 592ad1fb2e..defdeaf701 100644
--- a/Userland/Libraries/LibC/CMakeLists.txt
+++ b/Userland/Libraries/LibC/CMakeLists.txt
@@ -64,10 +64,10 @@ file(GLOB ELF_SOURCES CONFIGURE_DEPENDS "../LibELF/*.cpp")
 
 if ("${SERENITY_ARCH}" STREQUAL "i686")
     set(ASM_SOURCES "arch/i386/setjmp.S")
-    set(ELF_SOURCES ${ELF_SOURCES} ../LibELF/Arch/i386/plt_trampoline.S)
+    set(ELF_SOURCES ${ELF_SOURCES} ../LibELF/Arch/i386/entry.S ../LibELF/Arch/i386/plt_trampoline.S)
 elseif ("${SERENITY_ARCH}" STREQUAL "x86_64")
     set(ASM_SOURCES "arch/x86_64/setjmp.S")
-    set(ELF_SOURCES ${ELF_SOURCES} ../LibELF/Arch/x86_64/plt_trampoline.S)
+    set(ELF_SOURCES ${ELF_SOURCES} ../LibELF/Arch/x86_64/entry.S ../LibELF/Arch/x86_64/plt_trampoline.S)
 endif()
 
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-unknown-warning-option -DSERENITY_LIBC_BUILD")
diff --git a/Userland/Libraries/LibC/crt0.cpp b/Userland/Libraries/LibC/crt0.cpp
index ecee9384a9..02ab162785 100644
--- a/Userland/Libraries/LibC/crt0.cpp
+++ b/Userland/Libraries/LibC/crt0.cpp
@@ -19,9 +19,15 @@ extern u32 __stack_chk_guard;
 int main(int, char**, char**);
 
 // Tell the compiler that this may be called from somewhere else.
-int _start(int argc, char** argv, char** env);
+int _entry(int argc, char** argv, char** env);
 
-int _start(int argc, char** argv, char** env)
+asm(
+    ".globl _start\n"
+    "_start:\n"
+    "push $0\n"
+    "jmp _entry@plt\n");
+
+int _entry(int argc, char** argv, char** env)
 {
     u32 original_stack_chk = __stack_chk_guard;
     arc4random_buf(&__stack_chk_guard, sizeof(__stack_chk_guard));
diff --git a/Userland/Libraries/LibELF/Arch/i386/entry.S b/Userland/Libraries/LibELF/Arch/i386/entry.S
new file mode 100644
index 0000000000..8fc185398c
--- /dev/null
+++ b/Userland/Libraries/LibELF/Arch/i386/entry.S
@@ -0,0 +1,28 @@
+/*
+ * Copyright (c) 2021, Gunnar Beutner <gbeutner@serenityos.org>
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+    .align 4
+    .globl _invoke_entry
+    .hidden _invoke_entry
+    .type _invoke_entry,@function
+_invoke_entry: # (argc, argv, envp, entry)
+    addl $4, %esp # return address
+    popl %edi # argc
+    popl %esi # argv
+    popl %edx # envp
+    popl %ecx # entry
+
+    // The System V ABI for x86 and x86_64 prescribes that the stack pointer is 16-byte aligned
+    andl $~15, %esp
+
+    // We're going to push three arguments so we need to align the stack for that
+    subl $4, %esp
+
+    // FIXME: The way we're setting up the stack and passing arguments to the entry point isn't ABI-compliant
+    pushl %edx
+    pushl %esi
+    pushl %edi
+    jmp *%ecx
diff --git a/Userland/Libraries/LibELF/Arch/x86_64/entry.S b/Userland/Libraries/LibELF/Arch/x86_64/entry.S
new file mode 100644
index 0000000000..1195cfded8
--- /dev/null
+++ b/Userland/Libraries/LibELF/Arch/x86_64/entry.S
@@ -0,0 +1,16 @@
+/*
+ * Copyright (c) 2021, Gunnar Beutner <gbeutner@serenityos.org>
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+    .align 4
+    .globl _invoke_entry
+    .hidden _invoke_entry
+    .type _invoke_entry,@function
+_invoke_entry: # (argc, argv, envp, entry)
+    // The System V ABI for x86 and x86_64 prescribes that the stack pointer is 16-byte aligned
+    andq $~15, %rsp
+
+    // FIXME: The way we're setting up the stack and passing arguments to the entry point isn't ABI-compliant
+    jmp *%rcx
diff --git a/Userland/Libraries/LibELF/DynamicLinker.cpp b/Userland/Libraries/LibELF/DynamicLinker.cpp
index cc93fea4ed..2aaa5b4175 100644
--- a/Userland/Libraries/LibELF/DynamicLinker.cpp
+++ b/Userland/Libraries/LibELF/DynamicLinker.cpp
@@ -41,6 +41,8 @@ using LibCExitFunction = void (*)(int);
 using DlIteratePhdrCallbackFunction = int (*)(struct dl_phdr_info*, size_t, void*);
 using DlIteratePhdrFunction = int (*)(DlIteratePhdrCallbackFunction, void*);
 
+extern "C" [[noreturn]] void _invoke_entry(int argc, char** argv, char** envp, EntryPointFunction entry);
+
 static size_t s_current_tls_offset = 0;
 static size_t s_total_tls_size = 0;
 static size_t s_allocated_tls_block_size = 0;
@@ -550,14 +552,8 @@ void ELF::DynamicLinker::linker_main(String&& main_program_name, int main_progra
     if (s_do_breakpoint_trap_before_entry) {
         asm("int3");
     }
-    rc = entry_point_function(argc, argv, envp);
-    dbgln_if(DYNAMIC_LOAD_DEBUG, "rc: {}", rc);
-    if (s_libc_exit != nullptr) {
-        s_libc_exit(rc);
-    } else {
-        _exit(rc);
-    }
 
+    _invoke_entry(argc, argv, envp, entry_point_function);
     VERIFY_NOT_REACHED();
 }