Merge changes from topic "undefined-behaviour" into main

* changes:
  releng: move officialRelease to version.json
  Add -Werror CI job
  ci: add a asan+ubsan test run on x86_64-linux
  tree-wide: add support for asan!

.clang-tidy

@@ -16,3 +16,6 @@ Checks:
   - -bugprone-unchecked-optional-access
     # many warnings, seems like a questionable lint
   - -bugprone-branch-clone
+
+CheckOptions:
+  bugprone-reserved-identifier.AllowedIdentifiers: '__asan_default_options'

flake.nix

@@ -59,7 +59,8 @@
         (Run `touch .nocontribmsg` to hide this message.)
       '';
 
-      officialRelease = false;
+      versionJson = builtins.fromJSON (builtins.readFile ./version.json);
+      officialRelease = versionJson.official_release;
 
       # Set to true to build the release notes for the next release.
       buildUnreleasedNotes = true;
@@ -275,6 +276,19 @@
 
         # System tests.
         tests = import ./tests/nixos { inherit lib nixpkgs nixpkgsFor; } // {
+          # This is x86_64-linux only, just because we have significantly
+          # cheaper x86_64-linux compute in CI.
+          # It is clangStdenv because clang's sanitizers are nicer.
+          asanBuild = self.packages.x86_64-linux.nix-clangStdenv.override {
+            sanitize = [
+              "address"
+              "undefined"
+            ];
+            # it is very hard to make *every* CI build use this option such
+            # that we don't wind up building Lix twice, so we do it here where
+            # we are already doing so.
+            werror = true;
+          };
 
           # Make sure that nix-env still produces the exact same result
           # on a particular version of Nixpkgs.
@@ -406,7 +420,7 @@
             pkgs: stdenv:
             let
               nix = pkgs.callPackage ./package.nix {
-                inherit stdenv officialRelease versionSuffix;
+                inherit stdenv versionSuffix;
                 busybox-sandbox-shell = pkgs.busybox-sandbox-shell or pkgs.default-busybox-sandbox;
                 internalApiDocs = false;
               };

meson.build

@@ -199,7 +199,11 @@ configdata = { }
 # Dependencies
 #
 
-boehm = dependency('bdw-gc', required : get_option('gc'), version : '>=8.2.6')
+gc_opt = get_option('gc').disable_if(
+  'address' in get_option('b_sanitize'),
+  error_message: 'gc does far too many memory crimes for ASan'
+)
+boehm = dependency('bdw-gc', required : gc_opt, version : '>=8.2.6')
 configdata += {
   'HAVE_BOEHMGC': boehm.found().to_int(),
 }
@@ -482,7 +486,14 @@ if cxx.get_id() == 'clang' and get_option('b_sanitize') != ''
   add_project_link_arguments('-shared-libsan', language : 'cpp')
 endif
 
+# Clang gets grumpy about missing libasan symbols if -shared-libasan is not
+# passed when building shared libs, at least on Linux
+if cxx.get_id() == 'clang' and 'address' in get_option('b_sanitize')
+  add_project_link_arguments('-shared-libasan', 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')
 endif
@@ -497,7 +508,7 @@ endif
 # maintainers/buildtime_report.sh BUILD-DIR to simply work in clang builds.
 #
 # They can also be manually viewed at https://ui.perfetto.dev
-if get_option('profile-build').require(meson.get_compiler('cpp').get_id() == 'clang').enabled()
+if get_option('profile-build').require(cxx.get_id() == 'clang').enabled()
   add_project_arguments('-ftime-trace', language: 'cpp')
 endif
 

package.nix

@@ -52,16 +52,24 @@
 
   pname ? "lix",
   versionSuffix ? "",
-  officialRelease ? false,
+  officialRelease ? __forDefaults.versionJson.official_release,
   # Set to true to build the release notes for the next release.
   buildUnreleasedNotes ? true,
   internalApiDocs ? false,
 
+  # List of Meson sanitize options. Accepts values of b_sanitize, e.g.
+  # "address", "undefined", "thread".
+  sanitize ? null,
+  # Turn compiler warnings into errors.
+  werror ? false,
+
   # Not a real argument, just the only way to approximate let-binding some
   # stuff for argument defaults.
   __forDefaults ? {
     canRunInstalled = stdenv.buildPlatform.canExecute stdenv.hostPlatform;
 
+    versionJson = builtins.fromJSON (builtins.readFile ./version.json);
+
     boehmgc-nix = boehmgc.override { enableLargeConfig = true; };
 
     editline-lix = editline.overrideAttrs (prev: {
@@ -77,8 +85,7 @@ let
   inherit (lib) fileset;
   inherit (stdenv) hostPlatform buildPlatform;
 
-  versionJson = builtins.fromJSON (builtins.readFile ./version.json);
-  version = versionJson.version + versionSuffix;
+  version = __forDefaults.versionJson.version + versionSuffix;
 
   aws-sdk-cpp-nix = aws-sdk-cpp.override {
     apis = [
@@ -166,6 +173,12 @@ stdenv.mkDerivation (finalAttrs: {
   dontBuild = false;
 
   mesonFlags =
+    let
+      sanitizeOpts = lib.optionals (sanitize != null) (
+        [ "-Db_sanitize=${builtins.concatStringsSep "," sanitize}" ]
+        ++ lib.optional (builtins.elem "address" sanitize) "-Dgc=disabled"
+      );
+    in
     lib.optionals hostPlatform.isLinux [
       # You'd think meson could just find this in PATH, but busybox is in buildInputs,
       # which don't actually get added to PATH. And buildInputs is correct over
@@ -181,8 +194,10 @@ stdenv.mkDerivation (finalAttrs: {
       (lib.mesonEnable "internal-api-docs" internalApiDocs)
       (lib.mesonBool "enable-tests" finalAttrs.finalPackage.doCheck)
       (lib.mesonBool "enable-docs" canRunInstalled)
+      (lib.mesonBool "werror" werror)
     ]
-    ++ lib.optional (hostPlatform != buildPlatform) "--cross-file=${mesonCrossFile}";
+    ++ lib.optional (hostPlatform != buildPlatform) "--cross-file=${mesonCrossFile}"
+    ++ sanitizeOpts;
 
   # We only include CMake so that Meson can locate toml11, which only ships CMake dependency metadata.
   dontUseCmakeConfigure = true;
@@ -367,8 +382,6 @@ stdenv.mkDerivation (finalAttrs: {
       pegtl
       ;
 
-    inherit officialRelease;
-
     # The collection of dependency logic for this derivation is complicated enough that
     # it's easier to parameterize the devShell off an already called package.nix.
     mkDevShell =

releng/README.md

@@ -30,7 +30,7 @@ First, we prepare the release. `python -m releng prepare` is used for this.
 Then we tag the release with `python -m releng tag`:
 
 * Git HEAD is detached.
-* `officialRelease = true` is set in `flake.nix`, this is committed, and a
+* `"official_release": true` is set in `version.json`, this is committed, and a
   release is tagged.
 * The tag is merged back into the last branch (either `main` for new releases
   or `release-MAJOR` for maintenance releases) with `git merge -s ours VERSION`

releng/create_release.xsh

@@ -11,7 +11,7 @@ from . import environment
 from .environment import RelengEnvironment
 from . import keys
 from . import docker
-from .version import VERSION, RELEASE_NAME, MAJOR
+from .version import VERSION, RELEASE_NAME, MAJOR, OFFICIAL_RELEASE
 from .gitutils import verify_are_on_tag, git_preconditions
 from . import release_notes
 
@@ -39,12 +39,18 @@ def setup_creds(env: RelengEnvironment):
 
 
 def official_release_commit_tag(force_tag=False):
-    print('[+] Setting officialRelease in flake.nix and tagging')
+    print('[+] Setting officialRelease in version.json and tagging')
     prev_branch = $(git symbolic-ref --short HEAD).strip()
 
     git switch --detach
-    sed -i 's/officialRelease = false/officialRelease = true/' flake.nix
-    git add flake.nix
+
+    # Must be done in two parts due to buffering (opening the file immediately
+    # would truncate it).
+    new_version_json = $(jq --indent 4 '.official_release = true' version.json)
+    with open('version.json', 'w') as fh:
+        fh.write(new_version_json)
+    git add version.json
+
     message = f'release: {VERSION} "{RELEASE_NAME}"\n\nRelease produced with releng/create_release.xsh'
     git commit -m @(message)
     git tag @(['-f'] if force_tag else []) -a -m @(message) @(VERSION)
@@ -250,15 +256,14 @@ def build_manual(eval_result):
 
 
 def upload_manual(env: RelengEnvironment):
-    stable = json.loads($(nix eval --json '.#nix.officialRelease'))
-    if stable:
+    if OFFICIAL_RELEASE:
         version = MAJOR
     else:
         version = 'nightly'
 
     print('[+] aws s3 sync manual')
     aws s3 sync @(MANUAL)/ @(env.docs_bucket)/manual/lix/@(version)/
-    if stable:
+    if OFFICIAL_RELEASE:
         aws s3 sync @(MANUAL)/ @(env.docs_bucket)/manual/lix/stable/
 
 

releng/version.py

@@ -4,3 +4,4 @@ version_json = json.load(open('version.json'))
 VERSION = version_json['version']
 MAJOR = '.'.join(VERSION.split('.')[:2])
 RELEASE_NAME = version_json['release_name']
+OFFICIAL_RELEASE = version_json['official_release']

src/asan-options/asan-options.cc

@@ -0,0 +1,17 @@
+/// @file This is very bothersome code that has to be included in every
+/// executable to get the correct default ASan options. I am so sorry.
+
+extern "C" [[gnu::retain]] const char *__asan_default_options()
+{
+    // We leak a bunch of memory knowingly on purpose. It's not worthwhile to
+    // diagnose that memory being leaked for now.
+    //
+    // Instruction bytes are useful for finding the actual code that
+    // corresponds to an ASan report.
+    //
+    // TODO: setting log_path=asan.log or not: neither works, since you can't
+    // write to the fs in certain places in the testsuite, but you also cannot
+    // write arbitrarily to stderr in other places so the reports get eaten.
+    // pain 🥖
+    return "halt_on_error=1:abort_on_error=1:detect_leaks=0:print_summary=1:dump_instruction_bytes=1";
+}

src/meson.build

@@ -12,10 +12,19 @@ subdir('libmain')
 # libcmd depends on everything
 subdir('libcmd')
 
-
 # The rest of the subdirectories aren't separate components,
 # just source files in another directory, so we process them here.
 
+# Static library that just sets default ASan options. It needs to be included
+# in every executable.
+asanoptions = static_library(
+  'libasanoptions',
+  files('asan-options/asan-options.cc'),
+)
+libasanoptions = declare_dependency(
+  link_whole: asanoptions
+)
+
 build_remote_sources = files(
   'build-remote/build-remote.cc',
 )

src/nix/meson.build

@@ -80,6 +80,7 @@ nix = executable(
   profiles_md_gen,
   nix2_commands_sources,
   dependencies : [
+    libasanoptions,
     liblixcmd,
     liblixutil_mstatic,
     liblixstore_mstatic,

tests/functional/repl_characterization/meson.build

@@ -7,6 +7,7 @@ repl_characterization_tester = executable(
   'test-repl-characterization',
   repl_characterization_tester_sources,
   dependencies : [
+    libasanoptions,
     liblixutil,
     liblixutil_test_support,
     sodium,

tests/functional/test-libstoreconsumer/meson.build

@@ -2,6 +2,7 @@ libstoreconsumer_tester = executable(
   'test-libstoreconsumer',
   'main.cc',
   dependencies : [
+    libasanoptions,
     liblixutil,
     liblixstore,
     sodium,

tests/unit/meson.build

@@ -11,6 +11,10 @@
 # functions, the result would be way less readable than just a bit of copypasta.
 # It's only ~200 lines; better to just refactor the tests themselves which we'll want to do anyway.
 
+default_test_env = {
+  'ASAN_OPTIONS': 'detect_leaks=0:halt_on_error=1:abort_on_error=1:print_summary=1:dump_instruction_bytes=1'
+}
+
 libutil_test_support_sources = files(
   'libutil-support/tests/cli-literate-parser.cc',
   'libutil-support/tests/hash.cc',
@@ -63,6 +67,7 @@ libutil_tester = executable(
   'liblixutil-tests',
   libutil_tests_sources,
   dependencies : [
+    libasanoptions,
     rapidcheck,
     gtest,
     boehm,
@@ -78,7 +83,7 @@ test(
   'libutil-unit-tests',
   libutil_tester,
   args : tests_args,
-  env : {
+  env : default_test_env + {
     '_NIX_TEST_UNIT_DATA': meson.project_source_root() / 'tests/unit/libutil/data',
   },
   suite : 'check',
@@ -132,6 +137,7 @@ libstore_tester = executable(
   'liblixstore-tests',
   libstore_tests_sources,
   dependencies : [
+    libasanoptions,
     liblixstore_test_support,
     liblixutil_test_support,
     liblixstore_mstatic,
@@ -147,7 +153,7 @@ test(
   'libstore-unit-tests',
   libstore_tester,
   args : tests_args,
-  env : {
+  env : default_test_env + {
     '_NIX_TEST_UNIT_DATA': meson.project_source_root() / 'tests/unit/libstore/data',
   },
   suite : 'check',
@@ -196,6 +202,7 @@ libexpr_tester = executable(
   'liblixexpr-tests',
   libexpr_tests_sources,
   dependencies : [
+    libasanoptions,
     liblixexpr_test_support,
     liblixstore_test_support,
     liblixstore_mstatic,
@@ -214,7 +221,7 @@ test(
   'libexpr-unit-tests',
   libexpr_tester,
   args : tests_args,
-  env : {
+  env : default_test_env + {
     '_NIX_TEST_UNIT_DATA': meson.project_source_root() / 'tests/unit/libexpr/data',
   },
   suite : 'check',
@@ -226,6 +233,7 @@ libcmd_tester = executable(
   'liblixcmd-tests',
   files('libcmd/args.cc'),
   dependencies : [
+    libasanoptions,
     liblixcmd,
     liblixutil,
     liblixmain,
@@ -241,7 +249,7 @@ test(
   'libcmd-unit-tests',
   libcmd_tester,
   args : tests_args,
-  env : {
+  env : default_test_env + {
     # No special meaning here, it's just a file laying around that is unlikely to go anywhere
     # any time soon.
     '_NIX_TEST_UNIT_DATA': meson.project_source_root() / 'src/nix-env/buildenv.nix',
@@ -272,6 +280,7 @@ test(
   'libmain-unit-tests',
   libmain_tester,
   args : tests_args,
+  env : default_test_env,
   suite : 'check',
   protocol : 'gtest',
 )

version.json

@@ -1,4 +1,5 @@
 {
     "version": "2.91.0-dev",
+    "official_release": false,
     "release_name": "TBA"
 }