Merge "Build with traps on signed overflow" into main

4 files changed, 29 insertions, 7 deletions

diff --git a/local.mk b/local.mk
index 3f3abb9f0..a756c8272 100644
--- a/local.mk
+++ b/local.mk
@@ -1,4 +1,11 @@
-GLOBAL_CXXFLAGS += -Wno-deprecated-declarations -Werror=switch
+# 2024-03-24: jade benchmarked the default sanitize reporting in clang and got
+# a regression of about 10% on hackage-packages.nix with clang. So we are trapping instead.
+#
+# This has an overhead of 0-4% on gcc and unmeasurably little on clang, in
+# Nix evaluation benchmarks.
+DEFAULT_SANITIZE_FLAGS = -fsanitize=signed-integer-overflow -fsanitize-undefined-trap-on-error
+GLOBAL_CXXFLAGS += -Wno-deprecated-declarations -Werror=switch $(DEFAULT_SANITIZE_FLAGS)
+GLOBAL_LDFLAGS += $(DEFAULT_SANITIZE_FLAGS)
 # Allow switch-enum to be overridden for files that do not support it, usually because of dependency headers.
 ERROR_SWITCH_ENUM = -Werror=switch-enum
 
diff --git a/meson.build b/meson.build
index 8766fbf8c..14051a130 100644
--- a/meson.build
+++ b/meson.build
@@ -305,6 +305,23 @@ add_project_arguments(
   language : 'cpp',
 )
 
+if cxx.get_id() in ['gcc', 'clang']
+  # 2024-03-24: jade benchmarked the default sanitize reporting in clang and got
+  # a regression of about 10% on hackage-packages.nix with clang. So we are trapping instead.
+  #
+  # This has an overhead of 0-4% on gcc and unmeasurably little on clang, in
+  # Nix evaluation benchmarks.
+  #
+  # N.B. Meson generates a completely nonsense warning here:
+  # https://github.com/mesonbuild/meson/issues/9822
+  # Both of these args cannot be written in the default meson configuration.
+  # b_sanitize=signed-integer-overflow is ignored, and
+  # -fsanitize-undefined-trap-on-error is not representable.
+  sanitize_args = ['-fsanitize=signed-integer-overflow', '-fsanitize-undefined-trap-on-error']
+  add_project_arguments(sanitize_args, language: 'cpp')
+  add_project_link_arguments(sanitize_args, language: 'cpp')
+endif
+
 add_project_link_arguments('-pthread', language : 'cpp')
 if cxx.get_linker_id() in ['ld.bfd', 'ld.gold']
   add_project_link_arguments('-Wl,--no-copy-dt-needed-entries', language : 'cpp')
diff --git a/mk/libraries.mk b/mk/libraries.mk
index 1bc73d7f7..f9d427b10 100644
--- a/mk/libraries.mk
+++ b/mk/libraries.mk
@@ -78,11 +78,7 @@ define build-library
         $(1)_LDFLAGS += -undefined suppress -flat_namespace
       endif
     else
-      ifndef HOST_DARWIN
-        ifndef HOST_CYGWIN
-          $(1)_LDFLAGS += -Wl,-z,defs
-        endif
-      endif
+      # -Wl,-z,defs is broken with sanitizers on Linux/clang at least.
     endif
 
     ifndef HOST_DARWIN
diff --git a/package.nix b/package.nix
index a47814bd8..5def8416a 100644
--- a/package.nix
+++ b/package.nix
@@ -297,7 +297,9 @@ in stdenv.mkDerivation (finalAttrs: {
 
   strictDeps = true;
 
-  hardeningDisable = lib.optional stdenv.hostPlatform.isStatic "pie";
+  # strictoverflow is disabled because we trap on signed overflow instead
+  hardeningDisable = [ "strictoverflow" ]
+    ++ lib.optional stdenv.hostPlatform.isStatic "pie";
 
   meta.platforms = lib.platforms.unix;