Build with traps on signed overflow

This is UB, we should not be doing it, and we can cheaply turn it into
crashes reliably. We would much rather have crashes than the program
doing something silly.

Benchmarks, but i wonder if they are nonsense because they get identical
times across compilers?!

| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|:---|---:|---:|---:|---:|
| `result-clang/bin/nix eval -f ../nixpkgs/pkgs/development/haskell-modules/hackage-packages.nix` | 375.5 ± 24.0 | 353.8 | 408.8 | 1.00 |
| `result-gcc/bin/nix eval -f ../nixpkgs/pkgs/development/haskell-modules/hackage-packages.nix` | 407.9 ± 26.0 | 385.1 | 449.5 | 1.09 ± 0.10 |
| `result-clangsan/bin/nix eval -f ../nixpkgs/pkgs/development/haskell-modules/hackage-packages.nix` | 382.2 ± 26.6 | 354.9 | 419.0 | 1.02 ± 0.10 |
| `result-gccsan/bin/nix eval -f ../nixpkgs/pkgs/development/haskell-modules/hackage-packages.nix` | 408.6 ± 24.6 | 384.5 | 441.9 | 1.09 ± 0.10 |

| Command | Mean [s] | Min [s] | Max [s] | Relative |
|:---|---:|---:|---:|---:|
| `result-clang/bin/nix search --no-eval-cache github:nixos/nixpkgs/e1fa12d4f6c6fe19ccb59cac54b5b3f25e160870 hello` | 17.199 ± 0.167 | 16.930 | 17.499 | 1.01 ± 0.01 |
| `result-gcc/bin/nix search --no-eval-cache github:nixos/nixpkgs/e1fa12d4f6c6fe19ccb59cac54b5b3f25e160870 hello` | 17.409 ± 0.126 | 17.242 | 17.633 | 1.02 ± 0.01 |
| `result-clangsan/bin/nix search --no-eval-cache github:nixos/nixpkgs/e1fa12d4f6c6fe19ccb59cac54b5b3f25e160870 hello` | 17.080 ± 0.137 | 16.879 | 17.350 | 1.00 |
| `result-gccsan/bin/nix search --no-eval-cache github:nixos/nixpkgs/e1fa12d4f6c6fe19ccb59cac54b5b3f25e160870 hello` | 17.396 ± 0.160 | 17.131 | 17.660 | 1.02 ± 0.01 |

| Command | Mean [s] | Min [s] | Max [s] | Relative |
|:---|---:|---:|---:|---:|
| `result-clang/bin/nix eval --raw --impure --expr 'with import <nixpkgs/nixos> {}; system'` | 6.267 ± 0.069 | 6.197 | 6.415 | 1.02 ± 0.01 |
| `result-gcc/bin/nix eval --raw --impure --expr 'with import <nixpkgs/nixos> {}; system'` | 6.232 ± 0.045 | 6.180 | 6.311 | 1.01 ± 0.01 |
| `result-clangsan/bin/nix eval --raw --impure --expr 'with import <nixpkgs/nixos> {}; system'` | 6.162 ± 0.020 | 6.133 | 6.196 | 1.00 |
| `result-gccsan/bin/nix eval --raw --impure --expr 'with import <nixpkgs/nixos> {}; system'` | 6.229 ± 0.031 | 6.199 | 6.289 | 1.01 ± 0.01 |

| Command | Mean [s] | Min [s] | Max [s] | Relative |
|:---|---:|---:|---:|---:|
| `GC_INITIAL_HEAP_SIZE=10g result-clang/bin/nix eval --raw --impure --expr 'with import <nixpkgs/nixos> {}; system'` | 4.683 ± 0.044 | 4.630 | 4.761 | 1.00 |
| `GC_INITIAL_HEAP_SIZE=10g result-gcc/bin/nix eval --raw --impure --expr 'with import <nixpkgs/nixos> {}; system'` | 4.750 ± 0.041 | 4.680 | 4.812 | 1.01 ± 0.01 |
| `GC_INITIAL_HEAP_SIZE=10g result-clangsan/bin/nix eval --raw --impure --expr 'with import <nixpkgs/nixos> {}; system'` | 4.703 ± 0.040 | 4.640 | 4.760 | 1.00 ± 0.01 |
| `GC_INITIAL_HEAP_SIZE=10g result-gccsan/bin/nix eval --raw --impure --expr 'with import <nixpkgs/nixos> {}; system'` | 4.766 ± 0.037 | 4.727 | 4.844 | 1.02 ± 0.01 |

Change-Id: I616ca3eab670317587d47b41870d8ac963c019ae
This commit is contained in:
jade 2024-03-24 23:45:25 -07:00
parent f52193544d
commit 94ea517dbe
4 changed files with 29 additions and 7 deletions

View file

@ -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

View file

@ -302,6 +302,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')

View file

@ -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

View file

@ -287,7 +287,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;