From 3397609520225827ad6c919aa58d6a938ac91f0a Mon Sep 17 00:00:00 2001 From: Magnus Skjegstad Date: Wed, 4 Sep 2024 10:48:25 +0200 Subject: [PATCH 1/9] chainloader: remove unused GSL dependency --- chainloader.nix | 4 ---- 1 file changed, 4 deletions(-) diff --git a/chainloader.nix b/chainloader.nix index 46122a052..cdd4261b5 100644 --- a/chainloader.nix +++ b/chainloader.nix @@ -43,8 +43,4 @@ stdenv.mkDerivation rec { pkgs.buildPackages.cmake pkgs.buildPackages.nasm ]; - - buildInputs = [ - pkgs.microsoft_gsl - ]; } From a53908225c1c82948ca334d2c10551ace8174b2c Mon Sep 17 00:00:00 2001 From: Magnus Skjegstad Date: Wed, 4 Sep 2024 10:48:44 +0200 Subject: [PATCH 2/9] tests: add timeout to grub test --- test/kernel/integration/grub/test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/kernel/integration/grub/test.py b/test/kernel/integration/grub/test.py index 866d830f3..a5a4b881f 100755 --- a/test/kernel/integration/grub/test.py +++ b/test/kernel/integration/grub/test.py @@ -14,4 +14,4 @@ # Create the GRUB image subprocess.check_call(["bash",grubify,"kernel_grub.elf.bin"]) -vm.boot(multiboot = False) +vm.boot(20, multiboot = False) From 251f458f30ce45c2261d862b41cf642739d9c1b0 Mon Sep 17 00:00:00 2001 From: Magnus Skjegstad Date: Wed, 4 Sep 2024 11:13:30 +0200 Subject: [PATCH 3/9] Build kernel with -fno-threadsafe-statics Disables automatic locking when static variables are initialised from constructors. To delay initialisation of some static variables until the kernel is ready they are initialised from a constructor. This can for instance be useful when waiting for BSS to be cleared, RNG to be initialised or heap to be ready. As we have thread support in libcxx, the compiler will automatically add surrounding calls to __cxa_guard_acquire and __cxa_guard_release to make sure the variable is only set by a single thread/process. While locking they call syscalls FUTEX and GETTID. As this may happen early in the boot process (pre-libc) we are not prepared to handle syscalls and the results may be unexpected. One side effect is that they set errno in libc which has not been initialised to point to a valid location yet. With thread safe statics disabled we avoid this issue, but we have to manually lock in the kernel when SMP is enabled. Alternatives: An alternative would be to set _LIBCPP_HAS_NO_THREADS=1. This would remove thread support - but also features that can be useful later for SMP support, such as std::lock_guard. We would still have to lock manually when initialising static variables in IncludeOS' SMP mode. Bonus: Use C++20 when building libraries --- cmake/includeos.cmake | 2 +- cmake/library.cmake | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/cmake/includeos.cmake b/cmake/includeos.cmake index 382816a44..fa85a8fcc 100644 --- a/cmake/includeos.cmake +++ b/cmake/includeos.cmake @@ -72,7 +72,7 @@ if (NOT ${PLATFORM} STREQUAL "userspace") set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${CAPABS} ${WARNS} -nostdlib -fno-omit-frame-pointer -c") else() # these kinda work with llvm - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${CAPABS} ${WARNS} -nostdlib -nostdlibinc -fno-omit-frame-pointer -c") + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${CAPABS} ${WARNS} -nostdlib -nostdlibinc -fno-omit-frame-pointer -c -fno-threadsafe-statics") set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${CAPABS} ${WARNS} -nostdlib -nostdlibinc -fno-omit-frame-pointer -c") endif() endif() diff --git a/cmake/library.cmake b/cmake/library.cmake index 8467faf21..cca47bd3d 100644 --- a/cmake/library.cmake +++ b/cmake/library.cmake @@ -57,11 +57,11 @@ if (debug) endif() if (CMAKE_COMPILER_IS_GNUCC) - set(CMAKE_CXX_FLAGS "-m32 -MMD ${CAPABS} ${WARNS} -nostdlib -fno-omit-frame-pointer -c -std=c++14 -D_LIBCPP_HAS_NO_THREADS=1") + set(CMAKE_CXX_FLAGS "-m32 -MMD ${CAPABS} ${WARNS} -nostdlib -fno-omit-frame-pointer -c -std=c++20 -D_LIBCPP_HAS_NO_THREADS=1") set(CMAKE_C_FLAGS "-m32 -MMD ${CAPABS} ${WARNS} -nostdlib -fno-omit-frame-pointer -c") else() # these kinda work with llvm - set(CMAKE_CXX_FLAGS "-MMD ${CAPABS} ${OPTIMIZE} ${WARNS} -nostdlib -nostdlibinc -fno-omit-frame-pointer -c -std=c++14 -D_LIBCPP_HAS_NO_THREADS=1") + set(CMAKE_CXX_FLAGS "-MMD ${CAPABS} ${OPTIMIZE} ${WARNS} -nostdlib -nostdlibinc -fno-omit-frame-pointer -c -std=c++20 -fno-threadsafe-statics -D_LIBCPP_HAS_NO_THREADS=1") set(CMAKE_C_FLAGS "-MMD ${CAPABS} ${OPTIMIZE} ${WARNS} -nostdlib -nostdlibinc -fno-omit-frame-pointer -c") endif() From 9c239a23138815143bf7c99800bdcd9f7d89147f Mon Sep 17 00:00:00 2001 From: Magnus Skjegstad Date: Wed, 4 Sep 2024 11:30:12 +0200 Subject: [PATCH 4/9] ist: replace libc memalign call with kalloc_align in SMP stack --- src/arch/x86_64/ist.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/arch/x86_64/ist.cpp b/src/arch/x86_64/ist.cpp index 96b4c42b8..059757e44 100644 --- a/src/arch/x86_64/ist.cpp +++ b/src/arch/x86_64/ist.cpp @@ -70,7 +70,7 @@ static stack create_stack_virt(size_t size, const char* name) } static stack create_stack_simple(size_t size, const char* /*name*/) { - auto* phys = (char*)memalign(4096, size); + auto* phys = (char*)kalloc_aligned(4096, size); uintptr_t sp = (uintptr_t) phys + size - 8; sp &= ~uintptr_t(0xf); return {(void*) sp, phys}; From 2f53cecf21260ae1da9800b67116103ab736b79a Mon Sep 17 00:00:00 2001 From: Magnus Skjegstad Date: Wed, 4 Sep 2024 11:39:14 +0200 Subject: [PATCH 5/9] x86_nano: add ELF symbols in stacktrace Since we already read the ELF symbols, this also initializes the parser so the symbols are shown in stacktrace output. This is useful when e.g. debugging chainloader crashes. Also removes unused reference to _init_bss. --- src/platform/x86_nano/kernel_start.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/platform/x86_nano/kernel_start.cpp b/src/platform/x86_nano/kernel_start.cpp index e18db5651..315fb8776 100644 --- a/src/platform/x86_nano/kernel_start.cpp +++ b/src/platform/x86_nano/kernel_start.cpp @@ -25,8 +25,8 @@ extern "C" { void __init_sanity_checks(); uintptr_t _move_symbols(uintptr_t loc); - void _init_bss(); void _init_heap(uintptr_t); + void _init_elf_parser(); void _init_syscalls(); } @@ -57,6 +57,9 @@ void kernel_start(uintptr_t magic, uintptr_t addr) // Initialize heap kernel::init_heap(free_mem_begin, mem_end); + // Get backtrace on nano too + _init_elf_parser(); + // Initialize system calls _init_syscalls(); From a2b82e3280d550bc9d889522b9395267c9222495 Mon Sep 17 00:00:00 2001 From: Magnus Skjegstad Date: Wed, 4 Sep 2024 12:21:36 +0200 Subject: [PATCH 6/9] Create default heap PMR allocator to avoid early calls to malloc Previously we used std::map in memmap, which would proceed to call libc's malloc() before libc was initialised. This uses std::pmr::map instead and sets the default PMR allocator to be the same allocator as we later use in mmap/malloc. This allocator is initialised by the kernel as part of the heap. This change also allows the kernel to transparently use other std::pmr implementations after heap initialisation to avoid malloc calls. --- api/kernel/memmap.hpp | 2 +- src/kernel/heap.cpp | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/api/kernel/memmap.hpp b/api/kernel/memmap.hpp index 3f8828598..1009c2388 100644 --- a/api/kernel/memmap.hpp +++ b/api/kernel/memmap.hpp @@ -324,7 +324,7 @@ class Fixed_memory_range { class Memory_map { public: using Key = uintptr_t; - using Map = std::map; + using Map = std::pmr::map; /** * Assign a fixed range of memory to a named purpose diff --git a/src/kernel/heap.cpp b/src/kernel/heap.cpp index 2e9a45a6f..2903dfd5d 100644 --- a/src/kernel/heap.cpp +++ b/src/kernel/heap.cpp @@ -70,5 +70,10 @@ void kernel::init_heap(uintptr_t free_mem_begin, uintptr_t memory_end) noexcept auto brk_end = __init_brk(kernel::heap_begin(), __brk_max); Expects(brk_end <= memory_end); __init_mmap(brk_end, memory_end); + + // Also set the PMR default allocator to use the same allocator as mmap + auto& alloc = os::mem::raw_allocator(); + std::pmr::set_default_resource(&alloc); + __heap_ready = true; } From 07bcb8f18b0be7c6a0d08d7158be465a009c1a4d Mon Sep 17 00:00:00 2001 From: Magnus Skjegstad Date: Wed, 4 Sep 2024 12:46:29 +0200 Subject: [PATCH 7/9] Use std::pmr::string instead of strdup() Now that we have a heap PMR allocator we can avoid the calls to strdup(). In init_libc it was also incorrectly linked to libc's strdup, which resulted in malloc-calls and brk syscalls before libc was initialised. --- src/crt/CMakeLists.txt | 3 +-- src/crt/string.c | 28 ------------------------- src/crt/string.h | 34 ------------------------------- src/kernel/multiboot.cpp | 2 +- src/platform/x86_pc/init_libc.cpp | 6 +++--- 5 files changed, 5 insertions(+), 68 deletions(-) delete mode 100644 src/crt/string.c delete mode 100644 src/crt/string.h diff --git a/src/crt/CMakeLists.txt b/src/crt/CMakeLists.txt index 9d0bd18e8..6dc335aab 100644 --- a/src/crt/CMakeLists.txt +++ b/src/crt/CMakeLists.txt @@ -1,8 +1,7 @@ -SET(SRCS +SET(SRCS c_abi.c ctype_b_loc.c ctype_tolower_loc.c - string.c quick_exit.cpp cxx_abi.cpp ) diff --git a/src/crt/string.c b/src/crt/string.c deleted file mode 100644 index 18d7a46fa..000000000 --- a/src/crt/string.c +++ /dev/null @@ -1,28 +0,0 @@ -// This file is a part of the IncludeOS unikernel - www.includeos.org -// -// Copyright 2015 Oslo and Akershus University College of Applied Sciences -// and Alfred Bratterud -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#include "string.h" - -extern void* kalloc(size_t size); - -char* strdup(const char* string) -{ - size_t len = (strlen(string) + 1) * sizeof(char); - char* dup = (char*) kalloc(len); - memcpy(dup, string, len); - return dup; -} diff --git a/src/crt/string.h b/src/crt/string.h deleted file mode 100644 index ce24514ae..000000000 --- a/src/crt/string.h +++ /dev/null @@ -1,34 +0,0 @@ -// This file is a part of the IncludeOS unikernel - www.includeos.org -// -// Copyright 2015 Oslo and Akershus University College of Applied Sciences -// and Alfred Bratterud -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#ifndef _CABI_STRING_H -#define _CABI_STRING_H - -#include - -#ifdef __cplusplus -extern "C" -{ -#endif - -extern char* strdup(const char* string); - - -#ifdef __cplusplus -} -#endif -#endif diff --git a/src/kernel/multiboot.cpp b/src/kernel/multiboot.cpp index 7328df644..e91f6ed6d 100644 --- a/src/kernel/multiboot.cpp +++ b/src/kernel/multiboot.cpp @@ -141,7 +141,7 @@ void kernel::multiboot(uint32_t boot_addr) if (info->flags & MULTIBOOT_INFO_CMDLINE) { const auto* cmdline = (const char*) (uintptr_t) info->cmdline; INFO2("* Booted with parameters @ %p: %s", cmdline, cmdline); - kernel::state().cmdline = strdup(cmdline); + kernel::state().cmdline = std::pmr::string(cmdline).data(); } if (info->flags & MULTIBOOT_INFO_MEM_MAP) { diff --git a/src/platform/x86_pc/init_libc.cpp b/src/platform/x86_pc/init_libc.cpp index 5cec36f41..25c5b20d9 100644 --- a/src/platform/x86_pc/init_libc.cpp +++ b/src/platform/x86_pc/init_libc.cpp @@ -103,9 +103,9 @@ namespace x86 int argc = 1; // Env vars - argv[2] = strdup("LC_CTYPE=C"); - argv[3] = strdup("LC_ALL=C"); - argv[4] = strdup("USER=root"); + argv[2] = std::pmr::string("LC_CTYPE=C").data(); + argv[3] = std::pmr::string("LC_ALL=C").data(); + argv[4] = std::pmr::string("USER=root").data(); argv[5] = 0x0; // auxiliary vector From 32f93f7c5be587bedda90bab73e57f31f6402a29 Mon Sep 17 00:00:00 2001 From: Magnus Skjegstad Date: Wed, 4 Sep 2024 12:51:35 +0200 Subject: [PATCH 8/9] Remove printf to avoid ioctl syscall This call to printf calls ioctl before libc is initialised. Replaced with INFO2() --- src/kernel/multiboot.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/kernel/multiboot.cpp b/src/kernel/multiboot.cpp index e91f6ed6d..f4a99bf7d 100644 --- a/src/kernel/multiboot.cpp +++ b/src/kernel/multiboot.cpp @@ -178,7 +178,7 @@ void kernel::multiboot(uint32_t boot_addr) //os::mem::map_avail({map.addr, map.addr, {os::mem::Access::read | os::mem::Access::write}, map.len}, "Reserved (Multiboot)"); } } - printf("\n"); + INFO2(""); } auto mods = os::modules(); From 7a65eda242ec3e62cb43689b43da03e815088c83 Mon Sep 17 00:00:00 2001 From: Magnus Skjegstad Date: Wed, 4 Sep 2024 12:53:54 +0200 Subject: [PATCH 9/9] Panic on syscalls before libc has been initialised Adds a new "syscalls_allowed" state variable to disallow syscalls before libc has been initialised. Since the libc intialisation itself uses syscalls we can't use libc_initialised, which will be set to true when libc returns from init. This commit depends on previous commits that clean up and remove calls to malloc and other syscalls in early boot. In particular we can't use the __cxa_guard-calls or std-implementations that rely on malloc being available. Std::pmr implementations should now work by default as long as the kernel heap has been initialised. --- src/include/kernel.hpp | 7 ++++++- src/musl/common.hpp | 7 +++++-- src/musl/stub.hpp | 5 +++++ src/platform/x86_pc/init_libc.cpp | 1 + 4 files changed, 17 insertions(+), 3 deletions(-) diff --git a/src/include/kernel.hpp b/src/include/kernel.hpp index ad313db53..02f129408 100644 --- a/src/include/kernel.hpp +++ b/src/include/kernel.hpp @@ -30,7 +30,8 @@ namespace kernel { struct State { bool running = true; bool boot_sequence_passed = false; - bool libc_initialized = false; + bool libc_initialized = false; // Set when __libc_main returns + bool allow_syscalls = false; // Set before calling into libc bool block_drivers_ready = false; bool timestamps = false; bool timestamps_ready = false; @@ -62,6 +63,10 @@ namespace kernel { return state().libc_initialized; } + inline bool allow_syscalls() noexcept { + return state().allow_syscalls; + } + inline bool block_drivers_ready() noexcept { return state().block_drivers_ready; } diff --git a/src/musl/common.hpp b/src/musl/common.hpp index 3d1d4cc65..40ac85391 100644 --- a/src/musl/common.hpp +++ b/src/musl/common.hpp @@ -51,8 +51,6 @@ inline constexpr auto& pr_param(std::ostream& out, L lhs, Args&&... rest){ template inline void strace_print(const char* name, Ret ret, Args&&... args){ - if (not kernel::state().libc_initialized) - return; std::stringstream out; out << name << "("; @@ -74,6 +72,11 @@ inline void strace_print(const char* name, Ret ret, Args&&... args){ // strace, calling the syscall, recording return value and printing if enabled template inline auto strace(Fn func, [[maybe_unused]]const char* name, Args&&... args) { + if (!kernel::state().allow_syscalls) { + fprintf(stderr, "Syscalls not allowed here. Unexpected call to %s - terminating\n", name); + Expects(kernel::state().allow_syscalls); + } + auto ret = func(args...); if constexpr (__strace) diff --git a/src/musl/stub.hpp b/src/musl/stub.hpp index 473c57877..99e1eae46 100644 --- a/src/musl/stub.hpp +++ b/src/musl/stub.hpp @@ -13,6 +13,11 @@ inline void stubtrace_print(const char* name, R ret, Args&&... args) { // calling the syscall, recording return value and only printing when strace is on template inline auto stubtrace(Fn func, const char* name[[maybe_unused]], Args&&... args) { + if (!kernel::state().allow_syscalls) { + fprintf(stderr, "Syscalls not allowed here. Unexpected call to %s (stub) - terminating\n", name); + Expects(kernel::state().allow_syscalls); + } + auto ret = func(args...); if constexpr (__strace) diff --git a/src/platform/x86_pc/init_libc.cpp b/src/platform/x86_pc/init_libc.cpp index 25c5b20d9..6ee010063 100644 --- a/src/platform/x86_pc/init_libc.cpp +++ b/src/platform/x86_pc/init_libc.cpp @@ -161,6 +161,7 @@ namespace x86 // GDB_ENTRY; PRATTLE("* Starting libc initialization\n"); + kernel::state().allow_syscalls = true; __libc_start_main(kernel_main, argc, argv.data()); } }