Merge pull request #2309 from mazunki/feat-debugging

fix several printf-related warnings

Author: MagnusS

api/arch/x86/paging.hpp

@@ -317,7 +317,7 @@ class Page_table {
         auto* sub = page_dir(&ent);
         Expects(sub != nullptr);
         if (print) {
-          printf("%.*s-+<%s> 0x%zx\n", print_lvl * 2, pad,
+          printf("%.*s-+<%s> %p\n", print_lvl * 2, pad,
                  util::Byte_r(page_size).to_string().c_str(), (void*)sub->start_addr());
         }
         sum += sub->summary(print, print_lvl + 1);

api/expects

@@ -32,28 +32,43 @@
 #endif
 
 #include <os.hpp>
-inline void __expect_fail(const char *expr, const char *file, int line, const char *func){
+inline void __expect_emit_failure(std::string_view msg, std::string_view panic_text) {
 #ifndef UNITTESTS
 #ifdef INCLUDEOS_SMP_ENABLE
   SMP::global_lock();
 #endif
-  fprintf(stderr, "%s:%i:%s %s \n",file, line, func, expr);
+  std::fprintf(stderr, "%.*s\n", int(msg.size()), msg.data());
   fflush(NULL);
 #ifdef INCLUDEOS_SMP_ENABLE
   SMP::global_unlock();
 #endif
-  os::panic(expr);
+  os::panic(std::string(panic_text).c_str());
 #else // TEST
+  (void) panic_text;
   // throw here to allow tests to capture the error
   #include <stdexcept>
-  #include <format>
-  auto msg = std::format("{}:{}:{} {}",file, line, func, expr);
-  throw std::runtime_error(msg);
+  throw std::runtime_error(std::string(msg));
 #endif
 }
 
-#define Expects(x) ((void)((x) || (__expect_fail("Expects failed: "#x, __FILE__, __LINE__, __func__),0)))
-#define Ensures(x) ((void)((x) || (__expect_fail("Ensures failed: "#x, __FILE__, __LINE__, __func__),0)))
+template <class... Args>
+inline void __expect_failf(const char *err_prefix, const char * /*cond*/, const char *file, int line, const char *func, std::format_string<Args...> fmt, Args&&... args){
+  auto reason_msg = std::format(fmt, std::forward<Args>(args)...);
+  auto error_msg = std::format("{}:{}:{}: {}: {}", file, line, func, err_prefix, reason_msg);
+  __expect_emit_failure(error_msg, reason_msg);
+}
+
+inline void __expect_failf(const char *err_prefix, const char *cond, const char *file, int line, const char *func){
+  auto reason_msg = std::format("{}: {}", err_prefix, cond);
+  auto error_msg = std::format("{}:{}:{}: {}", file, line, func, err_prefix);
+  __expect_emit_failure(error_msg, reason_msg);
+}
+
+#define Expects(cond) ((void)((cond) || (__expect_failf("Expects failed", #cond, __FILE__, __LINE__, __func__),0)))
+#define Ensures(cond) ((void)((cond) || (__expect_failf("Ensures failed", #cond, __FILE__, __LINE__, __func__),0)))
+
+#define Expectsf(cond, fmt, ...) ((void)((cond) || (__expect_failf("Expects failed", #cond, __FILE__, __LINE__, __func__, fmt, ##__VA_ARGS__),0)))
+#define Ensuresf(cond, fmt, ...) ((void)((cond) || (__expect_failf("Ensures failed", #cond, __FILE__, __LINE__, __func__, fmt, ##__VA_ARGS__),0)))
 
 namespace os {
 // parameter for noexcept specifier when bypassing noexcept for testing

api/util/elf_binary.inc

@@ -149,7 +149,7 @@ const typename Elf_binary<Arch>::Section_header& Elf_binary<Arch>::section_heade
 
 template <typename Arch>
 const typename Elf_binary<Arch>::Span Elf_binary<Arch>::section_data(const Section_header& sh) const {
-  return {data_.data() + sh.sh_offset, static_cast<std::ptrdiff_t>(sh.sh_size)};
+  return {data_.data() + static_cast<std::size_t>(sh.sh_offset), static_cast<std::size_t>(sh.sh_size)};
 };
 
 template <typename Arch>

cmake/os.cmake

@@ -194,6 +194,9 @@ function(os_add_executable TARGET NAME)
     endif()
   endforeach()
 
+  find_package(fmt CONFIG REQUIRED)
+  target_link_libraries(${ELF_TARGET} fmt::fmt)
+
   # TODO: if not debug strip
   if (CMAKE_BUILD_TYPE STREQUAL "Debug")
     set(STRIP_LV )

deps/libfmt/default.nix

@@ -0,0 +1,35 @@
+# note that there is also `<nixpkgs>.fmt`
+# so this entire file could just be `{ pkgs }: pkgs.fmt`
+{
+  pkgs,
+  stdenv ? pkgs.stdenv,
+  cmake ? pkgs.cmake
+}:
+let
+  libfmt = stdenv.mkDerivation rec {
+    pname = "fmt";
+    version = "12.0.0";
+
+    src = pkgs.fetchFromGitHub {
+      owner = "fmtlib";
+      repo  = "fmt";
+      rev   = "12.0.0";
+      hash = "sha256-AZDmIeU1HbadC+K0TIAGogvVnxt0oE9U6ocpawIgl6g=";
+    };
+
+    nativeBuildInputs = [ cmake ];
+
+    cmakeFlags = [
+      "-DCMAKE_TRY_COMPILE_TARGET_TYPE=STATIC_LIBRARY"
+      "-DBUILD_SHARED_LIBS=OFF"
+
+      "-DFMT_TEST=OFF"
+      "-DFMT_DOC=OFF"
+      "-DFMT_INSTALL=ON"
+    ];
+  };
+in
+  libfmt // {
+    include = "${libfmt}/include";
+    lib = "${libfmt}/lib";
+  }

develop.nix

@@ -83,11 +83,13 @@ includeos.pkgs.mkShell.override { inherit (includeos) stdenv; } rec {
     jq \
       --arg libcxx "${includeos.libraries.libcxx.include}" \
       --arg libc "${includeos.libraries.libc}"             \
-      --arg localsrc "${toString ./.}"                           \
+      --arg libfmt "${includeos.passthru.libfmt.include}"  \
+      --arg localsrc "${toString ./.}"                     \
       '
       map(.command |= ( .
           + " -isystem \($libcxx)"
           + " -isystem \($libc)/include"
+          + " -I \($libfmt)"
           | gsub("(?<a>-I)(?<b>/lib/LiveUpdate/include)"; .a + $localsrc + .b)
       ))
     ' "$CCDB" > "$tmp" && mv "$tmp" "$CCDB"

overlay.nix

@@ -87,10 +87,12 @@ final: prev: {
     inherit suppressTargetWarningHook;
 
     # Deps
-    uzlib = self.callPackage ./deps/uzlib/default.nix { };
     botan2 = self.callPackage ./deps/botan/default.nix { };
-    s2n-tls = self.callPackage ./deps/s2n/default.nix { };
+    libfmt = self.callPackage ./deps/libfmt/default.nix { };
     http-parser = self.callPackage ./deps/http-parser/default.nix { };
+    s2n-tls = self.callPackage ./deps/s2n/default.nix { };
+    uzlib = self.callPackage ./deps/uzlib/default.nix { };
+
     vmbuild = self.callPackage ./vmbuild.nix { };
 
     ccacheWrapper = prev.ccacheWrapper.override {
@@ -157,6 +159,7 @@ final: prev: {
         ++ prev.lib.optionals withCcache [self.ccacheWrapper ccacheNoticeHook];
 
       buildInputs = [
+        self.libfmt
         self.botan2
         self.http-parser
         prev.pkgsStatic.openssl
@@ -205,6 +208,7 @@ final: prev: {
         inherit (self) uzlib;
         inherit (self) http-parser;
         inherit (self) botan2;
+        inherit (self) libfmt;
         #inherit (self) s2n-tls;
         inherit (self) cmake;
         inherit (self) vmbuild;

src/CMakeLists.txt

@@ -15,6 +15,10 @@ include_directories(
   include
 )
 
+find_package(fmt CONFIG REQUIRED)
+get_target_property(libfmt_include_path fmt::fmt INTERFACE_INCLUDE_DIRECTORIES)
+include_directories(${libfmt_include_path})
+
 #TODO move to util and check if needed / can be changed ?..
 include_directories(${INCLUDEOS_ROOT}/lib/LiveUpdate/include)
 
@@ -66,6 +70,7 @@ FILE(WRITE ${CMAKE_CURRENT_BINARY_DIR}/version.h
 include_directories(${CMAKE_CURRENT_BINARY_DIR})
 
 add_library(os STATIC ${SRCS} ${OBJECTS} ${CMAKE_CURRENT_BINARY_DIR}/version.h)
+target_link_libraries(os PRIVATE fmt::fmt)
 
 #TODO check if this is almost correct for platform userspace
 if (NOT CMAKE_TESTING_ENABLED)

src/chainload/CMakeLists.txt

@@ -4,6 +4,7 @@ set(CMAKE_TRY_COMPILE_TARGET_TYPE STATIC_LIBRARY)
 project(chainloader C CXX)
 
 find_package(IncludeOS REQUIRED)
+find_package(fmt CONFIG REQUIRED)
 
 set(ARCH i686)
 set(PLATFORM nano)

src/include/kernel.hpp

@@ -26,6 +26,7 @@ namespace kernel {
   using namespace util;
   constexpr size_t default_max_mem = 2_GiB;
   constexpr uintptr_t page_shift   = 12;
+  constexpr size_t kprintf_max_size = 8192;
 
   struct State {
     bool running               = true;