From a7a0fee1177902531369171fc962841a9a9f53a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Sat, 15 Aug 2026 01:03:37 +0200 Subject: [PATCH] Stop build_cc from defining options set to OFF `build.rs` passes CMake-style booleans through `define(key, "ON"/"OFF")`. CMake reads `-DKEY=OFF` as "not enabled", but the C preprocessor does not: `-DKEY=OFF` defines `KEY`, and snmalloc guards these options with `#if defined(KEY)` / `#ifdef KEY`. On the `build_cc` path every one of them is therefore enabled precisely when it was meant to be disabled. Two have live consumers in the library: `SNMALLOC_QEMU_WORKAROUND` is on for every `build_cc` build. On 64-bit targets that takes the `sizeclassconfig.h` branch intended for QEMU CI, raising `MIN_CHUNK_BITS` from 14 to 17 and `MAX_SMALL_SIZECLASS_BITS` from 16 to 19, so the slab/chunk split moves from 64 KiB to 512 KiB. On Linux it also trips the `#ifndef` in `pal_linux.h`, dropping the `madvise(MADV_DONTNEED)` fast path for zeroing large blocks and falling back to `memset` on every target, 32-bit included. `SNMALLOC_RUST_LIBC_API` compiles the libc API shims in `rust.cc` unconditionally, exporting symbols the `libc-api` feature is supposed to gate. `USE_SNMALLOC_STATS` is defined too but only reaches test code, so it is harmless here. `SNMALLOC_ENABLE_DYNAMIC_LOADING` and `SNMALLOC_USE_CXX17` have no preprocessor consumer at all; they are CMake options, so on the cc path they are inert in either direction. All five go through the same call, so all five are converted. Adds `define_bool` to `BuilderDefine` rather than fixing the flags one at a time, so the next boolean option cannot reintroduce this. The cc impl emits a bare `-DKEY` only when enabled; the cmake impl keeps passing `ON`/`OFF`, unchanged. This follows what `SNMALLOC_CHECK_LOADS`, `SNMALLOC_PAGEID` and `SNMALLOC_USE_WAIT_ON_ADDRESS` already do by hand with per-flag `cfg(feature = "build_cc")` branches. Verified by grepping the compiler invocation under `CC_ENABLE_DEBUG_OUTPUT=1`. Before, a default `build_cc` build emitted `-DSNMALLOC_QEMU_WORKAROUND=OFF`, `-DSNMALLOC_RUST_LIBC_API=OFF`, `-DUSE_SNMALLOC_STATS=OFF`, `-DSNMALLOC_ENABLE_DYNAMIC_LOADING=OFF` and `-DSNMALLOC_USE_CXX17=OFF`. After, none of them appear; building with `--features qemu,libc-api` emits `-DSNMALLOC_QEMU_WORKAROUND` and `-DSNMALLOC_RUST_LIBC_API` with no value. Binding tests pass on both the cc and cmake paths. --- snmalloc-rs/snmalloc-sys/build.rs | 34 ++++++++++++++++++++++++------- 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/snmalloc-rs/snmalloc-sys/build.rs b/snmalloc-rs/snmalloc-sys/build.rs index be7539839..9aa525c9d 100644 --- a/snmalloc-rs/snmalloc-sys/build.rs +++ b/snmalloc-rs/snmalloc-sys/build.rs @@ -217,6 +217,14 @@ impl BuildConfig { trait BuilderDefine { fn define(&mut self, key: &str, value: &str) -> &mut Self; + /// Set a CMake-style boolean option. + /// + /// CMake reads `-DKEY=OFF` as "not enabled". The C preprocessor does not: + /// `-DKEY=OFF` *defines* `KEY`, and snmalloc guards these options with + /// `#if defined(KEY)` / `#ifdef KEY`, so forwarding the "OFF" string on the + /// `build_cc` path enables every option it was meant to disable. Emit the + /// macro on that path only when the option is actually on. + fn define_bool(&mut self, key: &str, enabled: bool) -> &mut Self; fn flag_if_supported(&mut self, flag: &str) -> &mut Self; fn build_lib(&mut self, target_lib: &str) -> std::path::PathBuf; fn configure_output_dir(&mut self, out_dir: &str) -> &mut Self; @@ -229,7 +237,15 @@ impl BuilderDefine for cc::Build { fn define(&mut self, key: &str, value: &str) -> &mut Self { self.define(key, Some(value)) } - + + fn define_bool(&mut self, key: &str, enabled: bool) -> &mut Self { + if enabled { + self.define(key, None) + } else { + self + } + } + fn flag_if_supported(&mut self, flag: &str) -> &mut Self { self.flag_if_supported(flag) } @@ -261,7 +277,11 @@ impl BuilderDefine for cmake::Config { fn define(&mut self, key: &str, value: &str) -> &mut Self { self.define(key, value) } - + + fn define_bool(&mut self, key: &str, enabled: bool) -> &mut Self { + self.define(key, if enabled { "ON" } else { "OFF" }) + } + fn flag_if_supported(&mut self, _flag: &str) -> &mut Self { self } @@ -452,11 +472,11 @@ fn configure_platform(config: &mut BuildConfig) { // Feature configurations config.builder - .define("SNMALLOC_QEMU_WORKAROUND", if config.features.qemu { "ON" } else { "OFF" }) - .define("SNMALLOC_ENABLE_DYNAMIC_LOADING", if config.features.notls { "ON" } else { "OFF" }) - .define("USE_SNMALLOC_STATS", if config.features.stats { "ON" } else { "OFF" }) - .define("SNMALLOC_RUST_LIBC_API", if config.features.libc_api { "ON" } else { "OFF" }) - .define("SNMALLOC_USE_CXX17", if cfg!(feature = "usecxx17") { "ON" } else { "OFF" }); + .define_bool("SNMALLOC_QEMU_WORKAROUND", config.features.qemu) + .define_bool("SNMALLOC_ENABLE_DYNAMIC_LOADING", config.features.notls) + .define_bool("USE_SNMALLOC_STATS", config.features.stats) + .define_bool("SNMALLOC_RUST_LIBC_API", config.features.libc_api) + .define_bool("SNMALLOC_USE_CXX17", cfg!(feature = "usecxx17")); if config.features.tracing { config.builder.define("SNMALLOC_TRACING", "ON");