From fd3e3d5e28f729c05427d1c3cfe95e1ae3f62e39 Mon Sep 17 00:00:00 2001 From: Daniel Bertalan Date: Sat, 7 May 2022 18:11:00 +0200 Subject: [PATCH] LibC+Kernel: Prevent string functions from calling themselves Most of the string.h and wchar.h functions are implemented quite naively at the moment, and GCC's pattern recognition pass might realize what we are trying to do, and transform them into libcalls. This is usually a useful optimization, but not when we're implementing the functions themselves :^) Relevant discussion from the GCC Bugzilla: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102725 This prevents the infamous recursive `strlen`. A more proper fix would be writing these functions in assembly. That would likely give a small performance boost as well ;) --- Kernel/CMakeLists.txt | 5 +++++ Userland/DynamicLoader/CMakeLists.txt | 6 ++++++ Userland/Libraries/LibC/CMakeLists.txt | 5 +++++ 3 files changed, 16 insertions(+) diff --git a/Kernel/CMakeLists.txt b/Kernel/CMakeLists.txt index 3f5328b3e59..cae9bb2fd01 100644 --- a/Kernel/CMakeLists.txt +++ b/Kernel/CMakeLists.txt @@ -486,6 +486,11 @@ if (CMAKE_CXX_COMPILER_ID STREQUAL "GNU") set(TARGET_STRING "") + + # Prevent naively implemented string functions (like strlen) from being "optimized" into a call to themselves. + set_source_files_properties(MiniStdlib.cpp + PROPERTIES COMPILE_FLAGS "-fno-tree-loop-distribution -fno-tree-loop-distribute-patterns") + add_link_options(LINKER:-z,pack-relative-relocs) else() # Assume Clang add_compile_options(-Waddress-of-packed-member) diff --git a/Userland/DynamicLoader/CMakeLists.txt b/Userland/DynamicLoader/CMakeLists.txt index eaaf9b075ee..55c86fc19e4 100644 --- a/Userland/DynamicLoader/CMakeLists.txt +++ b/Userland/DynamicLoader/CMakeLists.txt @@ -36,6 +36,12 @@ set_source_files_properties (../Libraries/LibC/ssp.cpp PROPERTIES COMPILE_FLAGS # Prevent GCC from removing null checks by marking the `FILE*` argument non-null set_source_files_properties(../Libraries/LibC/stdio.cpp PROPERTIES COMPILE_FLAGS "-fno-builtin-fputc -fno-builtin-fputs -fno-builtin-fwrite") +# Prevent naively implemented string functions (like strlen) from being "optimized" into a call to themselves. +if (CMAKE_CXX_COMPILER_ID STREQUAL "GNU") + set_source_files_properties(../Libraries/LibC/string.cpp ../Libraries/LibC/wchar.cpp + PROPERTIES COMPILE_FLAGS "-fno-tree-loop-distribution -fno-tree-loop-distribute-patterns") +endif() + add_executable(Loader.so ${SOURCES}) if (CMAKE_CXX_COMPILER_ID STREQUAL "GNU") diff --git a/Userland/Libraries/LibC/CMakeLists.txt b/Userland/Libraries/LibC/CMakeLists.txt index 2842d32fe05..9485cc602dd 100644 --- a/Userland/Libraries/LibC/CMakeLists.txt +++ b/Userland/Libraries/LibC/CMakeLists.txt @@ -142,6 +142,11 @@ set_source_files_properties(stdio.cpp PROPERTIES COMPILE_FLAGS "-fno-builtin-fpu # Add in the `posix_memalign` symbol to avoid breaking existing binaries. set_source_files_properties(stdlib.cpp PROPERTIES COMPILE_FLAGS "-DSERENITY_LIBC_SHOW_POSIX_MEMALIGN") +# Prevent naively implemented string functions (like strlen) from being "optimized" into a call to themselves. +if (CMAKE_CXX_COMPILER_ID STREQUAL "GNU") + set_source_files_properties(string.cpp wchar.cpp PROPERTIES COMPILE_FLAGS "-fno-tree-loop-distribution -fno-tree-loop-distribute-patterns") +endif() + add_library(LibCStaticWithoutDeps STATIC ${SOURCES}) target_link_libraries(LibCStaticWithoutDeps PUBLIC ssp LibTimeZone PRIVATE NoCoverage) add_dependencies(LibCStaticWithoutDeps LibM LibSystem LibUBSanitizer)