From e70ae36b3f84427c3eb8574adabaea8ef237103d Mon Sep 17 00:00:00 2001 From: Jade Lovelace Date: Thu, 14 Aug 2025 21:31:41 +0200 Subject: [PATCH] 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 Fixes: https://github.com/NixOS/nix/issues/12899 Upstream-PR: https://github.com/NixOS/nix/pull/13013 Change-Id: I6a6a6964cdd1a88714952e80c660d1fa57d1f2d8 --- .../rl-next/floor-ceil-corrupt-integers.md | 13 ++++ lix/libexpr/builtins/ceil.md | 11 +-- lix/libexpr/builtins/floor.md | 11 +-- lix/libexpr/primops.cc | 74 +++++++++++++++++-- .../floor-ceil-corrupt-integers.md | 14 ++++ lix/libutil/meson.build | 1 + .../ceil-bad-weird-int-depr.out.exp | 1 + .../ceil-bad-weird-int.err.exp | 11 +++ .../ceil-okay-weird-int.out.exp | 1 + .../floor-bad-weird-int-depr.out.exp | 1 + .../floor-bad-weird-int.err.exp | 11 +++ .../floor-okay-weird-int.out.exp | 1 + .../lang/floor-ceil-out-of-range/in.nix | 9 +++ .../lang/floor-ceil-out-of-range/test.toml | 30 ++++++++ tests/unit/libexpr/error_traces.cc | 26 ++++--- tests/unit/libexpr/primops.cc | 22 ++++++ 16 files changed, 212 insertions(+), 25 deletions(-) create mode 100644 doc/manual/rl-next/floor-ceil-corrupt-integers.md create mode 100644 lix/libutil/deprecated-features/floor-ceil-corrupt-integers.md create mode 100644 tests/functional2/lang/floor-ceil-out-of-range/ceil-bad-weird-int-depr.out.exp create mode 100644 tests/functional2/lang/floor-ceil-out-of-range/ceil-bad-weird-int.err.exp create mode 100644 tests/functional2/lang/floor-ceil-out-of-range/ceil-okay-weird-int.out.exp create mode 100644 tests/functional2/lang/floor-ceil-out-of-range/floor-bad-weird-int-depr.out.exp create mode 100644 tests/functional2/lang/floor-ceil-out-of-range/floor-bad-weird-int.err.exp create mode 100644 tests/functional2/lang/floor-ceil-out-of-range/floor-okay-weird-int.out.exp create mode 100644 tests/functional2/lang/floor-ceil-out-of-range/in.nix create mode 100644 tests/functional2/lang/floor-ceil-out-of-range/test.toml diff --git a/doc/manual/rl-next/floor-ceil-corrupt-integers.md b/doc/manual/rl-next/floor-ceil-corrupt-integers.md new file mode 100644 index 000000000..440f05034 --- /dev/null +++ b/doc/manual/rl-next/floor-ceil-corrupt-integers.md @@ -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`. diff --git a/lix/libexpr/builtins/ceil.md b/lix/libexpr/builtins/ceil.md index 78f123339..26183f705 100644 --- a/lix/libexpr/builtins/ceil.md +++ b/lix/libexpr/builtins/ceil.md @@ -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. diff --git a/lix/libexpr/builtins/floor.md b/lix/libexpr/builtins/floor.md index 9645a358c..24f58d626 100644 --- a/lix/libexpr/builtins/floor.md +++ b/lix/libexpr/builtins/floor.md @@ -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. diff --git a/lix/libexpr/primops.cc b/lix/libexpr/primops.cc index 588cce7b5..b12061c1c 100644 --- a/lix/libexpr/primops.cc +++ b/lix/libexpr/primops.cc @@ -36,6 +36,7 @@ #include #include +#include namespace nix { @@ -703,17 +704,80 @@ static Value prim_addErrorContext(EvalState & state, Value ** args) } } +static std::optional 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( + "%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("%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;}, diff --git a/lix/libutil/deprecated-features/floor-ceil-corrupt-integers.md b/lix/libutil/deprecated-features/floor-ceil-corrupt-integers.md new file mode 100644 index 000000000..4621bb7e1 --- /dev/null +++ b/lix/libutil/deprecated-features/floor-ceil-corrupt-integers.md @@ -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: . diff --git a/lix/libutil/meson.build b/lix/libutil/meson.build index df5cde21e..a2ca65a8a 100644 --- a/lix/libutil/meson.build +++ b/lix/libutil/meson.build @@ -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', diff --git a/tests/functional2/lang/floor-ceil-out-of-range/ceil-bad-weird-int-depr.out.exp b/tests/functional2/lang/floor-ceil-out-of-range/ceil-bad-weird-int-depr.out.exp new file mode 100644 index 000000000..23399bcf5 --- /dev/null +++ b/tests/functional2/lang/floor-ceil-out-of-range/ceil-bad-weird-int-depr.out.exp @@ -0,0 +1 @@ +9223090561878065152 diff --git a/tests/functional2/lang/floor-ceil-out-of-range/ceil-bad-weird-int.err.exp b/tests/functional2/lang/floor-ceil-out-of-range/ceil-bad-weird-int.err.exp new file mode 100644 index 000000000..9c2426713 --- /dev/null +++ b/tests/functional2/lang/floor-ceil-out-of-range/ceil-bad-weird-int.err.exp @@ -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 diff --git a/tests/functional2/lang/floor-ceil-out-of-range/ceil-okay-weird-int.out.exp b/tests/functional2/lang/floor-ceil-out-of-range/ceil-okay-weird-int.out.exp new file mode 100644 index 000000000..23399bcf5 --- /dev/null +++ b/tests/functional2/lang/floor-ceil-out-of-range/ceil-okay-weird-int.out.exp @@ -0,0 +1 @@ +9223090561878065152 diff --git a/tests/functional2/lang/floor-ceil-out-of-range/floor-bad-weird-int-depr.out.exp b/tests/functional2/lang/floor-ceil-out-of-range/floor-bad-weird-int-depr.out.exp new file mode 100644 index 000000000..23399bcf5 --- /dev/null +++ b/tests/functional2/lang/floor-ceil-out-of-range/floor-bad-weird-int-depr.out.exp @@ -0,0 +1 @@ +9223090561878065152 diff --git a/tests/functional2/lang/floor-ceil-out-of-range/floor-bad-weird-int.err.exp b/tests/functional2/lang/floor-ceil-out-of-range/floor-bad-weird-int.err.exp new file mode 100644 index 000000000..117fbe3cf --- /dev/null +++ b/tests/functional2/lang/floor-ceil-out-of-range/floor-bad-weird-int.err.exp @@ -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 diff --git a/tests/functional2/lang/floor-ceil-out-of-range/floor-okay-weird-int.out.exp b/tests/functional2/lang/floor-ceil-out-of-range/floor-okay-weird-int.out.exp new file mode 100644 index 000000000..23399bcf5 --- /dev/null +++ b/tests/functional2/lang/floor-ceil-out-of-range/floor-okay-weird-int.out.exp @@ -0,0 +1 @@ +9223090561878065152 diff --git a/tests/functional2/lang/floor-ceil-out-of-range/in.nix b/tests/functional2/lang/floor-ceil-out-of-range/in.nix new file mode 100644 index 000000000..73609bdf4 --- /dev/null +++ b/tests/functional2/lang/floor-ceil-out-of-range/in.nix @@ -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; +} diff --git a/tests/functional2/lang/floor-ceil-out-of-range/test.toml b/tests/functional2/lang/floor-ceil-out-of-range/test.toml new file mode 100644 index 000000000..3a6a017ed --- /dev/null +++ b/tests/functional2/lang/floor-ceil-out-of-range/test.toml @@ -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"] diff --git a/tests/unit/libexpr/error_traces.cc b/tests/unit/libexpr/error_traces.cc index f3163437c..5e6b0e2b8 100644 --- a/tests/unit/libexpr/error_traces.cc +++ b/tests/unit/libexpr/error_traces.cc @@ -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") + ); } diff --git a/tests/unit/libexpr/primops.cc b/tests/unit/libexpr/primops.cc index 9eefe0aa4..ed8462922 100644 --- a/tests/unit/libexpr/primops.cc +++ b/tests/unit/libexpr/primops.cc @@ -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::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::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) {