libexpr/builtins: correctly handle integer edge cases in floor/ceil

We choose to throw an error in the invalid-input space where we *can* produce a valid (but wrong) result to allow ourselves to, mirroring CppNix, change the semantics to not corrupt it at all in the future, while having a middle state where it is an error. This is a largely-rewritten version of https://github.com/NixOS/nix/pull/13013. Co-authored-by: rootile <lix@rootile.de> Fixes: https://github.com/NixOS/nix/issues/12899 Upstream-PR: https://github.com/NixOS/nix/pull/13013 Change-Id: I6a6a6964cdd1a88714952e80c660d1fa57d1f2d8
2026-04-30 16:10:47 +02:00
parent 89f5974125
commit e70ae36b3f
16 changed files with 212 additions and 25 deletions
@@ -0,0 +1,13 @@
+---
+synopsis: builtins.floor/builtins.ceil handle out-of-range inputs correctly
+issues: [nix#12899]
+cls: [3923]
+prs: [nix#13013]
+category: "Breaking Changes"
+credits: [jade, nan-git, rootile]
+---
+Previously, `builtins.floor` and `builtins.ceil` always cast the input into a floating point value before running the operation and casting the floating point result back into an integer.
+No checks were made for precision loss in either coercing integer inputs or converting the output to an integer (and in fact in the latter case, invoked undefined behaviour).
+
+Now, Lix checks for precision loss on integer input (to avoid a silent eval semantics change if we were to simply pass it through as-is) and on integer output.
+If your code fails to evaluate after this change, use `--extra-deprecated-features floor-ceil-corrupt-integers`.
@@ -1,9 +1,10 @@
 ---
 name: ceil
-args: [double]
+args: [number]
 ---
-Converts an IEEE-754 double-precision floating-point number (*double*) to
-the next higher integer.
+Returns an integer value containing the smallest integer greater than or equal to *number* (which is either a floating-point or integer value).

-If the datatype is neither an integer nor a "float", an evaluation error will be
-thrown.
+If the result is out of range for the integer type, such as NaN, infinity, or a number with magnitude out of range, Lix throws an evaluation error.
+
+Lix currently throws an evaluation error for some *integer* inputs between 2\*\*52 and 2\*\*63 - 1 as those previously experienced floating-point precision loss due to a Nix bug (https://github.com/NixOS/nix/issues/12899).
+In a future release, such integers will be passed through.
@@ -1,9 +1,10 @@
 ---
 name: floor
-args: [double]
+args: [number]
 ---
-Converts an IEEE-754 double-precision floating-point number (*double*) to
-the next lower integer.
+Returns an integer value containing the largest integer less than or equal to *number* (which is either a float or integer).

-If the datatype is neither an integer nor a "float", an evaluation error will be
-thrown.
+If the result is out of range for the integer type, such as NaN, infinity, or a number with magnitude out of range, Lix throws an evaluation error.
+
+Lix currently throws an evaluation error for some *integer* inputs between 2\*\*52 and 2\*\*63 - 1 as those previously experienced floating-point precision loss due to a Nix bug (https://github.com/NixOS/nix/issues/12899).
+In a future release, such integers will be passed through.
@@ -36,6 +36,7 @@
 #include <dlfcn.h>

 #include <cmath>
+#include <cfenv>

 namespace nix {

@@ -703,17 +704,80 @@ static Value prim_addErrorContext(EvalState & state, Value ** args)
    }
 }

+static std::optional<NixInt> floatToIntChecked(NixFloat f)
+{
+// Required to detect overflows when converting floats to integers
+#pragma STDC FENV_ACCESS ON
+
+    std::feclearexcept(FE_ALL_EXCEPT);
+    auto converted = llrint(f);
+    if (std::fetestexcept(FE_ALL_EXCEPT)) {
+        return std::nullopt;
+    }
+    return NixInt{converted};
+}
+
+/*
+  Note [floor/ceil corrupt integers]:
+  Integers above 2**54 that aren't a power of two get corrupted when passed
+  through floor/ceil.
+
+  Corrupting integers would become impossible if we just passed through integer
+  inputs, since the corruption actually happens when casting from int to float,
+  *not* from float to int, which is the one we actually safely cast.
+
+  Floats generate an error as intended if they are out of range.
+
+  In a future release, we'd like to pass through all integers, but it would be
+  an eval semantics change, so it's safer to error first before relaxing the
+  semantics again.
+ */
+
+using FloorCeilFunc = auto (*)(NixFloat) -> NixFloat;
+
+static Value
+floorCeil(std::string_view const which, FloorCeilFunc f, EvalState & state, NixFloat value, Value * arg0)
+{
+    bool isInt = arg0->type() == nInt;
+
+    NixFloat result = f(value);
+
+    if (auto checked = floatToIntChecked(result); checked.has_value()) {
+        // See Note [floor/ceil corrupt integers].
+        if (isInt && *checked != arg0->integer()
+            && !featureSettings.isEnabled(DeprecatedFeature::FloorCeilCorruptIntegers))
+        {
+            state.ctx.errors
+                .make<EvalError>(
+                    "%s was corrupting your integer (was %d, became %d) in previous versions due to a "
+                    "historical Nix bug (https://github.com/NixOS/nix/issues/12899).\n"
+                    "This may be changed in the future to pass through integers as-is, which will change the "
+                    "semantics of this code.\n"
+                    "To suppress this error, use %s",
+                    which,
+                    arg0->integer(),
+                    *checked,
+                    "--extra-deprecated-features floor-ceil-corrupt-integers"
+                )
+                .debugThrow();
+        }
+        return {NewValueAs::integer, NixInt::Inner(*checked)};
+    } else {
+        state.ctx.errors.make<EvalError>("%s result %f is out of range for Nix integer (i64)", which, result)
+            .debugThrow();
+    }
+}
+
 static Value prim_ceil(EvalState & state, Value ** args)
 {
-    auto value = state.forceFloat(*args[0], noPos,
-            "while evaluating the first argument passed to builtins.ceil");
-    return {NewValueAs::integer, NixInt::Inner(ceil(value))};
+    auto value = state.forceFloat(*args[0], noPos, "while evaluating the argument passed to builtins.ceil");
+    return floorCeil("builtins.ceil", ceil, state, value, args[0]);
 }

 static Value prim_floor(EvalState & state, Value ** args)
 {
-    auto value = state.forceFloat(*args[0], noPos, "while evaluating the first argument passed to builtins.floor");
-    return {NewValueAs::integer, NixInt::Inner(floor(value))};
+    auto value = state.forceFloat(*args[0], noPos, "while evaluating the argument passed to builtins.floor");
+    return floorCeil("builtins.floor", floor, state, value, args[0]);
 }

 /* Try evaluating the argument. Success => {success=true; value=something;},
@@ -0,0 +1,14 @@
+---
+name: floor-ceil-corrupt-integers
+internalName: FloorCeilCorruptIntegers
+timeline:
+  - date: 2026-04-30
+    release: 2.96.0
+    cls: [3923]
+    message: Introduced as evaluation-time error.
+---
+Allow `builtins.floor` and `builtins.ceil` to corrupt integer inputs outside of the safe range to store in floats without precision loss (as in previous versions) rather than throwing an evaluation error.
+
+In a future Lix release, `builtins.floor` and `builtins.ceil` will pass through integer inputs unchanged.
+
+See: <https://github.com/NixOS/nix/issues/12899>.
@@ -177,6 +177,7 @@ deprecated_feature_definitions = files(
  'deprecated-features/broken-string-indentation.md',
  'deprecated-features/cr-line-endings.md',
  'deprecated-features/floating-without-zero.md',
+  'deprecated-features/floor-ceil-corrupt-integers.md',
  'deprecated-features/nix-path-shadow.md',
  'deprecated-features/nul-bytes.md',
  'deprecated-features/or-as-identifier.md',
@@ -0,0 +1 @@
+9223090561878065152
@@ -0,0 +1,11 @@
+error:
+       … while calling the 'ceil' builtin
+         at /pwd/in.nix:7:24:
+            6|   floor-okay-weird-int = builtins.floor big;
+            7|   ceil-bad-weird-int = builtins.ceil (big - 1);
+             |                        ^
+            8|   ceil-okay-weird-int = builtins.ceil big;
+
+       error: builtins.ceil was corrupting your integer (was 9223090561878065151, became 9223090561878065152) in previous versions due to a historical Nix bug (https://github.com/NixOS/nix/issues/12899).
+       This may be changed in the future to pass through integers as-is, which will change the semantics of this code.
+       To suppress this error, use --extra-deprecated-features floor-ceil-corrupt-integers
@@ -0,0 +1 @@
+9223090561878065152
@@ -0,0 +1 @@
+9223090561878065152
@@ -0,0 +1,11 @@
+error:
+       … while calling the 'floor' builtin
+         at /pwd/in.nix:5:25:
+            4| {
+            5|   floor-bad-weird-int = builtins.floor (big - 1);
+             |                         ^
+            6|   floor-okay-weird-int = builtins.floor big;
+
+       error: builtins.floor was corrupting your integer (was 9223090561878065151, became 9223090561878065152) in previous versions due to a historical Nix bug (https://github.com/NixOS/nix/issues/12899).
+       This may be changed in the future to pass through integers as-is, which will change the semantics of this code.
+       To suppress this error, use --extra-deprecated-features floor-ceil-corrupt-integers
@@ -0,0 +1 @@
+9223090561878065152
@@ -0,0 +1,9 @@
+let
+  big = 65536 * 65536 * 65536 * 32767;
+in
+{
+  floor-bad-weird-int = builtins.floor (big - 1);
+  floor-okay-weird-int = builtins.floor big;
+  ceil-bad-weird-int = builtins.ceil (big - 1);
+  ceil-okay-weird-int = builtins.ceil big;
+}
@@ -0,0 +1,30 @@
+[[test]]
+name = "floor-bad-weird-int"
+runner = "eval-fail"
+flags = ["-A", "floor-bad-weird-int"]
+
+[[test]]
+name = "floor-bad-weird-int-depr"
+runner = "eval-okay"
+flags = ["--extra-deprecated-features", "floor-ceil-corrupt-integers", "-A", "floor-bad-weird-int"]
+
+[[test]]
+name = "floor-okay-weird-int"
+runner = "eval-okay"
+flags = ["-A", "floor-okay-weird-int"]
+
+[[test]]
+name = "ceil-bad-weird-int-depr"
+runner = "eval-okay"
+flags = ["--extra-deprecated-features", "floor-ceil-corrupt-integers", "-A", "ceil-bad-weird-int"]
+
+[[test]]
+name = "ceil-bad-weird-int"
+runner = "eval-fail"
+flags = ["-A", "ceil-bad-weird-int"]
+
+
+[[test]]
+name = "ceil-okay-weird-int"
+runner = "eval-okay"
+flags = ["-A", "ceil-okay-weird-int"]
@@ -268,20 +268,26 @@ namespace nix {


    TEST_F(ErrorTraceTest, ceil) {
-        ASSERT_TRACE2("ceil \"foo\"",
-                      TypeError,
-                      HintFmt("expected a float but found %s: %s", "a string", Uncolored(ANSI_MAGENTA "\"foo\"" ANSI_NORMAL)),
-                      HintFmt("while evaluating the first argument passed to builtins.ceil"));
-
+        ASSERT_TRACE2(
+            "ceil \"foo\"",
+            TypeError,
+            HintFmt(
+                "expected a float but found %s: %s", "a string", Uncolored(ANSI_MAGENTA "\"foo\"" ANSI_NORMAL)
+            ),
+            HintFmt("while evaluating the argument passed to builtins.ceil")
+        );
    }


    TEST_F(ErrorTraceTest, floor) {
-        ASSERT_TRACE2("floor \"foo\"",
-                      TypeError,
-                      HintFmt("expected a float but found %s: %s", "a string", Uncolored(ANSI_MAGENTA "\"foo\"" ANSI_NORMAL)),
-                      HintFmt("while evaluating the first argument passed to builtins.floor"));
-
+        ASSERT_TRACE2(
+            "floor \"foo\"",
+            TypeError,
+            HintFmt(
+                "expected a float but found %s: %s", "a string", Uncolored(ANSI_MAGENTA "\"foo\"" ANSI_NORMAL)
+            ),
+            HintFmt("while evaluating the argument passed to builtins.floor")
+        );
    }


@@ -65,11 +65,33 @@ namespace nix {
    TEST_F(PrimOpTest, ceil) {
        auto v = eval("builtins.ceil 1.9");
        ASSERT_THAT(v, IsIntEq(2));
+
+        auto intMin = eval("builtins.ceil (-4611686018427387904 - 4611686018427387904)");
+        ASSERT_THAT(intMin, IsIntEq(std::numeric_limits<NixInt::Inner>::min()));
+        ASSERT_THROW(eval("builtins.ceil 1.0e200"), EvalError);
+        ASSERT_THROW(eval("builtins.ceil -1.0e200"), EvalError);
+        ASSERT_THROW(eval("builtins.ceil (1.0e200 * 1.0e200)"), EvalError); // inf
+        ASSERT_THROW(eval("builtins.ceil (-1.0e200 * 1.0e200)"), EvalError); // -inf
+        ASSERT_THROW(eval("builtins.ceil (1.0e200 * 1.0e200 - 1.0e200 * 1.0e200)"), EvalError); // nan
+        // bugs in previous Nix versions
+        ASSERT_THROW(eval("builtins.ceil (4611686018427387904 + 4611686018427387903)"), EvalError);
+        ASSERT_THROW(eval("builtins.ceil (-4611686018427387904 - 4611686018427387903)"), EvalError);
    }

    TEST_F(PrimOpTest, floor) {
        auto v = eval("builtins.floor 1.9");
        ASSERT_THAT(v, IsIntEq(1));
+
+        auto intMin = eval("builtins.floor (-4611686018427387904 - 4611686018427387904)");
+        ASSERT_THAT(intMin, IsIntEq(std::numeric_limits<NixInt::Inner>::min()));
+        ASSERT_THROW(eval("builtins.floor 1.0e200"), EvalError);
+        ASSERT_THROW(eval("builtins.floor -1.0e200"), EvalError);
+        ASSERT_THROW(eval("builtins.floor (1.0e200 * 1.0e200)"), EvalError); // inf
+        ASSERT_THROW(eval("builtins.floor (-1.0e200 * 1.0e200)"), EvalError); // -inf
+        ASSERT_THROW(eval("builtins.floor (1.0e200 * 1.0e200 - 1.0e200 * 1.0e200)"), EvalError); // nan
+        // bugs in previous Nix versions
+        ASSERT_THROW(eval("builtins.floor (4611686018427387904 + 4611686018427387903)"), EvalError);
+        ASSERT_THROW(eval("builtins.floor (-4611686018427387904 - 4611686018427387903)"), EvalError);
    }

    TEST_F(PrimOpTest, tryEvalFailure) {