From e2f411f71d7a8906fee0422e273cd7a2d0585d7c Mon Sep 17 00:00:00 2001 From: tannevaled Date: Wed, 4 Nov 2026 22:07:17 +0100 Subject: [PATCH] Range#max: beginless Integer ranges, as 4.0 answers them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five of the 27 examples that went red when rbgo moved to Ruby 4.0 (#799) are one cause: `core/range/max_spec.rb` has `ruby_version_is "4.0"` blocks that rbgo refused outright. The file goes 30 → 35, fail=0 error=0. ## The reference range.c's `range_max` at v4.0.9 changed in two places that both read as "a nil begin is not a disqualification": int c = NIL_P(b) ? -1 : OPTIMIZED_CMP(b, e); // beginless sorts BELOW the end if (!NIL_P(b) && !RB_INTEGER_TYPE_P(b)) { // the Integer-begin demand is rb_raise(... "cannot exclude end value with non Integer begin value"); } // now guarded by !NIL_P(b) and the n-argument branch goes through `reverse_each`, which walks `(NIL_P(beg) || RB_INTEGER_TYPE_P(beg)) && RB_INTEGER_TYPE_P(end)` downward from the end with nothing but n to stop it. ## What moved here * `(...1).max` is 0, not a TypeError. * `(..1).max(n)` and `(...1).max(n)` count down from the end, unbounded below. * Bignum endpoints answer arithmetically: `(0...2**64).max(2)` no longer tries to materialise 2**64 elements. The predecessor of an exclusive end is taken in big.Int, so the int64 floor stops being a special case — `(...-2**63).max` is -9223372036854775809, a Bignum, as in MRI. * A beginless NON-Integer range raises TypeError "can't iterate from NilClass", not the old blanket RangeError. * `(..1).max(0)` is `[]`. ## The guard order is observable `range_max` asks for a block BEFORE it looks at n. `(..1).max(2) { |a,b| a<=>b }` raises in MRI; answering the n first returns `[1, 0]` and looks entirely reasonable. The check moved to the top of the method for that reason, and the test says so. ## Two tests were MOVED, not relaxed `max_n_beginless` and `max_excl_beginless_int` in range_conformance_test.go pinned the pre-4.0 refusals — ruby/spec marks both `ruby_version_is ""..."4.0"`. Each row now names the refusal that replaced it, so the file still asserts a refusal for each shape rather than losing one. ## Measured, not read Every expectation came from MRI 4.0.7 on this host through rubydiff, before and after. Three probe programs covering 36 shapes: the first two now AGREE byte-for-byte; the third has one remaining divergence, pre-existing and unrelated to beginless ranges — `(1..10).max(2) { |a,b| -(a<=>b) }` is `[1, 2]` in MRI and `[10, 9]` here, because rbgo answers `max(n)` arithmetically and ignores the block's ordering. Filed separately rather than smuggled in. ## Bound A beginless range has no lower endpoint to clamp n against, so n alone decides the allocation: `(..1).max(2**40)` is a request for 8 TiB. MRI bounds it inside `rb_ary_new2`, which refuses a length above ARY_MAX_SIZE with ArgumentError; this refuses with the same class, and the test asserts both the refusal and that an ordinary count of 1000 still works. Coverage: integerRangeMaxN, rangeBigEnds and toBigInt at 100.0%. The one uncovered line before the last test was rangeBigEnds' non-Integer-begin refusal, reachable as `(1.0..3).max(2)`. Co-Authored-By: Claude Opus 5 --- internal/vm/builtins.go | 154 ++++++++++++++++------- internal/vm/range_conformance_test.go | 16 ++- internal/vm/range_max_arithmetic_test.go | 13 +- internal/vm/range_max_beginless_test.go | 140 +++++++++++++++++++++ 4 files changed, 274 insertions(+), 49 deletions(-) create mode 100644 internal/vm/range_max_beginless_test.go diff --git a/internal/vm/builtins.go b/internal/vm/builtins.go index 8ba2d57b..3818ac0a 100644 --- a/internal/vm/builtins.go +++ b/internal/vm/builtins.go @@ -5711,14 +5711,28 @@ func (vm *VM) bootstrap() { }) vm.cRange.defineArgc("max", -1, func(vm *VM, self object.Value, args []object.Value, blk *Proc) object.Value { r := self.(*object.Range) - if len(args) > 0 { // max(n): the n largest, descending - if object.IsNil(r.Hi) { - raise("RangeError", "cannot get the maximum of endless range") + if object.IsNil(r.Hi) { + raise("RangeError", "cannot get the maximum of endless range") + } + // ⛔ range_max asks this BEFORE looking at the n argument, and the order + // is observable: `(..1).max(2) { |a,b| a<=>b }` raises in MRI, where + // answering the n first would hand back [1, 0]. A block, or an exclusive + // range whose end is not Numeric, has to walk elements, and a beginless + // range has none to offer. + if blk != nil || (r.Exclusive && !isNumericValue(r.Hi)) { + if object.IsNil(r.Lo) { + raise("RangeError", "cannot get the maximum of beginless range with custom comparison method") } - // A beginless range cannot be walked from its start, so its n largest - // are unobtainable (MRI raises RangeError rather than iterating). + } + if len(args) > 0 { // max(n): the n largest, descending + // A beginless NON-Integer range is not walkable at all: MRI's + // reverse_each falls through to Enumerable, whose #each raises + // "can't iterate from NilClass". A TypeError, not the RangeError + // that pre-4.0 refused every beginless range with. if object.IsNil(r.Lo) { - raise("RangeError", "cannot get the maximum of beginless range") + if _, ok := toBigInt(r.Hi); !ok { + raise("TypeError", "can't iterate from NilClass") + } } // ⛔ An Integer range answers ARITHMETICALLY. Building the range to // take n off the end costs O(size of the range) and nothing about @@ -5740,30 +5754,38 @@ func (vm *VM) bootstrap() { if blk != nil { return vm.rangeExtremeByBlock(r, blk, false) } - // The maximum comes from the endpoints without iterating: endless raises, an - // empty range is nil, an inclusive range is the end. An exclusive range with - // an Integer end is end-1; with any other end it is the last element reached + // The maximum comes from the endpoints without iterating: an empty range + // is nil, an inclusive range is the end. An exclusive range with an + // Integer end is end-1; with any other end it is the last element reached // by #succ (a String range yields it, a Float range raises TypeError). - if object.IsNil(r.Hi) { - raise("RangeError", "cannot get the maximum of endless range") - } if rangeEmpty(vm, r) { return object.NilV } if r.Exclusive { - // An Integer (or Bignum) end excludes by yielding its predecessor — but - // only for a bounded range: excluding an Integer end still requires an - // Integer begin (MRI 3.x), so a beginless exclusive Integer range raises. + // An Integer (or Bignum) end excludes by yielding its predecessor. + // + // ⛔ Until 4.0 this also demanded an Integer BEGIN, so `(...1).max` + // raised "cannot exclude end value with non Integer begin value" — a + // nil begin counting as "non Integer". range_max now guards that + // check with `!NIL_P(b) &&`, so a beginless range skips it and the + // answer is the predecessor of the end: MRI 4.0.7 gives `(...1).max` + // as 0, and `(...-2**63).max` as -9223372036854775809. switch r.Hi.(type) { case object.Integer, *object.Bignum: - if object.IsNil(r.Lo) { - raise("TypeError", "cannot exclude end value with non Integer begin value") + if !object.IsNil(r.Lo) { + switch r.Lo.(type) { + case object.Integer, *object.Bignum: + default: + raise("TypeError", "cannot exclude end value with non Integer begin value") + } } return vm.send(r.Hi, "-", []object.Value{object.IntValue(1)}, nil) } - // A numeric (Float/Rational) end, or a beginless range, has no - // predecessor reachable by #succ. - if object.IsNil(r.Lo) || isNumericValue(r.Hi) { + // A numeric (Float/Rational) end has no predecessor reachable by + // #succ. A beginless range never arrives here: a non-Numeric end took + // the walk-the-elements guard at the top of the method, and a Numeric + // one is caught by this raise. + if isNumericValue(r.Hi) { raise("TypeError", "cannot exclude non Integer end value") } // A non-empty iterable range (e.g. String): the last element before @@ -11991,49 +12013,97 @@ func (vm *VM) rangeFirstN(r *object.Range, n int) []object.Value { // rangeInts extracts integer endpoints. ok is false when either endpoint is not // an Integer (string/float ranges are not iterable in this phase). -// integerRangeMaxN answers Range#max(n) for a bounded Integer range without -// building the range, and reports false for anything else (a String range, a -// Float end, a Bignum endpoint) so the caller keeps its existing path. +// integerRangeMaxN answers Range#max(n) for an Integer range without building +// the range, and reports false for anything else (a String range, a Float end) +// so the caller keeps its existing path. // // ⛔ THE COUNT IS CLAMPED WITHOUT COMPUTING THE SIZE. `top - lo + 1` overflows // int64 for a range like (-2**62..2**62), and an overflowed size clamps n to a // negative number — a wrong answer on exactly the inputs this function exists -// to make cheap. The span is taken in uint64, where it is exact for every -// int64 pair with top >= lo, and compared against n instead. +// to make cheap. The arithmetic is done in big.Int, where no pair of endpoints +// overflows, and only the clamped count comes back to int64. +// +// Beginless ranges answer here too, since Ruby 4.0: range.c's range_max reaches +// its n-argument branch with `c = NIL_P(b) ? -1 : …`, so a nil begin is simply +// "below the end" rather than a refusal, and reverse_each walks +// `(NIL_P(beg) || RB_INTEGER_TYPE_P(beg)) && RB_INTEGER_TYPE_P(end)` downward +// from the end with nothing to stop it but n. Measured against MRI 4.0.7: +// `(..1).max(3)` is `[1, 0, -1]`. func integerRangeMaxN(r *object.Range, n int64) (object.Value, bool) { - lo, hi, ok := rangeInts(r) + lo, hi, ok := rangeBigEnds(r) if !ok { return nil, false } if n < 0 { - raise("ArgumentError", "negative array size") + raise("ArgumentError", "negative array size (or size too big)") } - top := hi + top := new(big.Int).Set(hi) if r.Exclusive { - // An exclusive Integer end excludes by yielding its predecessor. If hi - // is the smallest int64 there is no predecessor, and the range is empty - // anyway for any lo. - if hi == math.MinInt64 { + // An exclusive Integer end excludes by yielding its predecessor. In + // big.Int that is unconditional: the int64 floor needed a special case, + // `(...-2**63).max` is -9223372036854775809 in MRI, and it is a Bignum. + top.Sub(top, big.NewInt(1)) + } + count := n + if lo != nil { + if top.Cmp(lo) < 0 { return object.NewArrayFromSlice(nil), true } - top-- - } - if top < lo { - return object.NewArrayFromSlice(nil), true + // span is the number of steps from lo to top: exact, and only consulted + // when it is small enough to shorten the answer. + if span := new(big.Int).Sub(top, lo); span.IsInt64() && span.Int64() < n { + count = span.Int64() + 1 + } } - span := uint64(top) - uint64(lo) // exact: the number of steps from lo to top - count := n - if uint64(n) > span { - // n is int64 and n > span, so span+1 cannot overflow int64 here. - count = int64(span) + 1 + // ⛔ A beginless range has nothing to clamp against, so n alone decides the + // allocation and `(..1).max(2**40)` is a request for 8 TiB of slice. MRI + // bounds it in rb_ary_new2, which refuses a length over ARY_MAX_SIZE with + // ArgumentError; refuse here for the same reason, with the same class, so + // an untrusted n is a Ruby exception and not an OOM kill. + if count > maxRangeMaxN { + raise("ArgumentError", "negative array size (or size too big)") } out := make([]object.Value, count) + cur := new(big.Int).Set(top) + one := big.NewInt(1) for i := int64(0); i < count; i++ { - out[i] = object.IntValue(top - i) + out[i] = object.NormInt(new(big.Int).Set(cur)) + cur.Sub(cur, one) } return object.NewArrayFromSlice(out), true } +// maxRangeMaxN caps how many elements Range#max(n) will materialise. MRI's own +// ceiling is ARY_MAX_SIZE (LONG_MAX/sizeof(VALUE)), which no machine can +// allocate either; this is the point at which rbgo stops trying and raises the +// class MRI raises. +const maxRangeMaxN = 1 << 27 + +// rangeBigEnds returns r's Integer endpoints as big.Ints, with lo nil for a +// beginless range. ok is false if either present endpoint is not an Integer. +func rangeBigEnds(r *object.Range) (lo, hi *big.Int, ok bool) { + if hi, ok = toBigInt(r.Hi); !ok { + return nil, nil, false + } + if object.IsNil(r.Lo) { + return nil, hi, true + } + if lo, ok = toBigInt(r.Lo); !ok { + return nil, nil, false + } + return lo, hi, true +} + +func toBigInt(v object.Value) (*big.Int, bool) { + switch n := v.(type) { + case object.Integer: + return big.NewInt(int64(n)), true + case *object.Bignum: + return new(big.Int).Set(n.I), true + } + return nil, false +} + func rangeInts(r *object.Range) (lo, hi int64, ok bool) { li, lok := r.Lo.(object.Integer) hi2, hok := r.Hi.(object.Integer) diff --git a/internal/vm/range_conformance_test.go b/internal/vm/range_conformance_test.go index 573ca265..9640b6c6 100644 --- a/internal/vm/range_conformance_test.go +++ b/internal/vm/range_conformance_test.go @@ -259,8 +259,20 @@ end {"max_endless", `(1..).max`, "RangeError"}, {"max_n_endless", `(1..).max(2)`, "RangeError"}, - {"max_n_beginless", `(..1.0).max(2)`, "RangeError"}, - {"max_excl_beginless_int", `(...1).max`, "TypeError"}, + // ⛔ These two rows pinned the PRE-4.0 refusals, and ruby/spec marks both + // `ruby_version_is ""..."4.0"`. They are not relaxed here, they are + // MOVED to what 4.0 refuses instead, so the file still asserts a + // refusal for each shape: + // + // (..1.0).max(2) RangeError "beginless" -> TypeError, not walkable + // (...1).max TypeError "non Integer begin" -> answers 0 + // + // The second shape has no refusal left at all, so its row names the one + // that took its place: an exclusive beginless end that is not Numeric, + // which still cannot be walked. `(...1).max == 0` is asserted in + // range_max_beginless_test.go, where the answer belongs. + {"max_n_beginless_non_integer", `(..1.0).max(2)`, "TypeError"}, + {"max_excl_beginless_non_numeric", `(..."f").max`, "RangeError"}, {"max_excl_float", `(1.5...3.5).max`, "TypeError"}, {"max_block_beginless", `(..1).max {|a, b| a }`, "RangeError"}, diff --git a/internal/vm/range_max_arithmetic_test.go b/internal/vm/range_max_arithmetic_test.go index 73e8150b..fdf33122 100644 --- a/internal/vm/range_max_arithmetic_test.go +++ b/internal/vm/range_max_arithmetic_test.go @@ -42,11 +42,14 @@ func TestRangeMaxNIsArithmeticNotMaterialised(t *testing.T) { {"a range whose size overflows int64", `p(((-(2**62))..(2**62)).max(2))`, "[4611686018427387904, 4611686018427387903]"}, - // ⛔ AN EXCLUSIVE END AT THE int64 FLOOR has no predecessor to step down - // to, so `top--` would underflow. The range is empty for every possible - // begin, and the coverage gate is what showed this guard was never - // driven -- reachable from Ruby, since -2**63 is an Integer and not a - // Bignum. + // ⛔ AN EXCLUSIVE END AT THE int64 FLOOR once had no predecessor to step + // down to: `top--` underflowed, and the guard that caught it was found + // by the coverage gate rather than by anyone's imagination. The + // arithmetic is in big.Int now, so -2**63 - 1 simply exists and the + // special case is gone -- but the case is kept, because what it asserts + // is the ANSWER, not the guard. With a begin of 0 the range is empty + // whatever the end's predecessor is; `(...-2**63).max` below is the same + // predecessor with nothing to make it empty. {"an exclusive end at the int64 floor", `p((0...(-2**63)).max(2))`, "[]"}, // Controls: the paths this must NOT have taken over. diff --git a/internal/vm/range_max_beginless_test.go b/internal/vm/range_max_beginless_test.go new file mode 100644 index 00000000..ab664b30 --- /dev/null +++ b/internal/vm/range_max_beginless_test.go @@ -0,0 +1,140 @@ +// Copyright (c) the go-embedded-ruby/ruby authors +// +// SPDX-License-Identifier: BSD-3-Clause + +package vm + +import "testing" + +// Ruby 4.0 teaches Range#max to answer for beginless Integer ranges. range.c's +// range_max reaches its arithmetic with `c = NIL_P(b) ? -1 : OPTIMIZED_CMP(b, e)` +// -- a nil begin is simply "below the end" -- and guards the Integer-begin +// demand with `!NIL_P(b) &&`, so a beginless range skips it entirely. The n +// argument goes through reverse_each, which walks +// `(NIL_P(beg) || RB_INTEGER_TYPE_P(beg)) && RB_INTEGER_TYPE_P(end)` downward +// from the end with nothing but n to stop it. +// +// Every expectation below was taken from MRI 4.0.7 on this host, through +// rubydiff, not from reading range.c. The five ruby/spec examples behind +// `ruby_version_is "4.0"` in core/range/max_spec.rb are the first five rows. +func TestRangeMaxOnBeginlessRanges(t *testing.T) { + for _, tc := range []struct{ name, src, want string }{ + // The five spec examples. + {"exclusive beginless Integer", `p((...1).max)`, "0"}, + {"n maxima of a beginless range", `p((..1).max(2))`, "[1, 0]"}, + {"n maxima of an exclusive beginless range", `p((...1).max(2))`, "[0, -1]"}, + {"n maxima of a very large Integer range", `p((0...2**64).max(2))`, + "[18446744073709551615, 18446744073709551614]"}, + {"a beginless non-Integer range cannot be walked", + `begin; (..1.0).max(2); rescue TypeError => e; p e.message; end`, + `"can't iterate from NilClass"`}, + + // The same refusal for a String end, which is the other half of that + // example and reaches it by a different route: not Numeric, so not an + // Integer either. + {"a beginless String range cannot be walked", + `begin; (.."f").max(2); rescue TypeError => e; p e.message; end`, + `"can't iterate from NilClass"`}, + + // ⛔ The predecessor of an exclusive end is taken in big.Int, so the + // int64 floor is not a special case: MRI answers -9223372036854775809, + // a Bignum, and so must this. + {"an exclusive beginless end at the int64 floor", `p((...(-2**63)).max)`, + "-9223372036854775809"}, + {"n maxima below the int64 floor", `p((..(-2**63)).max(2))`, + "[-9223372036854775808, -9223372036854775809]"}, + {"n maxima below an exclusive int64 floor", `p((...(-2**63)).max(2))`, + "[-9223372036854775809, -9223372036854775810]"}, + + // Bignum endpoints on a bounded range, which the int64 helper used to + // decline and hand to the materialising path -- where a 2**64-element + // range is not an answer anyone receives. + {"a Bignum end on a bounded range", `p((0..2**64).max(2))`, + "[18446744073709551616, 18446744073709551615]"}, + {"a Bignum end on a beginless range", `p((..(2**64)).max(2))`, + "[18446744073709551616, 18446744073709551615]"}, + + // n of zero is an empty array, not a refusal: MRI builds an array of + // capacity 0 and reverse_each stops immediately. + {"zero maxima of a beginless range", `p((..1).max(0))`, "[]"}, + {"zero maxima of an exclusive beginless range", `p((...1).max(0))`, "[]"}, + + // A Float n is converted, as NUM2LONG does. + {"a Float count on a beginless range", `p((..1).max(2.7))`, "[1, 0]"}, + + // ⛔ THE ORDER OF THE GUARDS IS OBSERVABLE. range_max asks for a block + // BEFORE it looks at the n argument, so a beginless range with both + // raises rather than answering arithmetically. Answering first would + // return [1, 0] here and look perfectly reasonable. + {"a block beats the n argument on a beginless range", + `begin; (..1).max(2) { |a, b| a <=> b }; rescue RangeError => e; p e.message; end`, + `"cannot get the maximum of beginless range with custom comparison method"`}, + {"a block alone on a beginless range", + `begin; (..1).max { |a, b| a <=> b }; rescue RangeError => e; p e.message; end`, + `"cannot get the maximum of beginless range with custom comparison method"`}, + + // ⛔ An exclusive range whose end is not Numeric takes the same + // walk-the-elements route as a block does, so a beginless one raises + // the SAME RangeError -- not the TypeError about excluding a non + // Integer end, which is what rbgo used to say here. + {"an exclusive beginless non-Numeric end", + `begin; (..."f").max; rescue RangeError => e; p e.message; end`, + `"cannot get the maximum of beginless range with custom comparison method"`}, + + // ⛔ A non-Integer BEGIN with an Integer end declines the arithmetic and + // falls to the materialising path, which cannot start from a Float. + // Found by the coverage gate: rangeBigEnds' second refusal was the only + // uncovered line in this change, and it is reachable from Ruby. + {"a Float begin with an Integer end", + `begin; (1.0..3).max(2); rescue TypeError => e; p e.message; end`, + `"can't iterate from Float"`}, + + // Controls: what must NOT have moved. + {"an endless range still refuses", + `begin; (1..).max; rescue RangeError => e; p e.message; end`, + `"cannot get the maximum of endless range"`}, + {"an endless range with n still refuses", + `begin; (1..).max(2); rescue RangeError => e; p e.message; end`, + `"cannot get the maximum of endless range"`}, + {"a beginless inclusive range is still its end", `p((..1).max)`, "1"}, + {"a beginless Float range is still its end", `p((..1.0).max)`, "1.0"}, + {"an exclusive Float end still refuses", + `begin; (...1.0).max; rescue TypeError => e; p e.message; end`, + `"cannot exclude non Integer end value"`}, + {"an exclusive Rational end still refuses", + `begin; (...Rational(1)).max; rescue TypeError => e; p e.message; end`, + `"cannot exclude non Integer end value"`}, + {"a non-Integer begin still refuses an excluded end", + `begin; (1.0...2).max; rescue TypeError => e; p e.message; end`, + `"cannot exclude end value with non Integer begin value"`}, + {"a beginless minimum is still unobtainable", + `begin; (..1).min; rescue RangeError => e; p e.message; end`, + `"cannot get the minimum of beginless range"`}, + } { + t.Run(tc.name, func(t *testing.T) { + if got := runSrc(t, tc.src); got != tc.want { + t.Errorf("%s\n got %q\nwant %q", tc.src, got, tc.want) + } + }) + } +} + +// ⛔ A beginless range has no lower endpoint to clamp n against, so n alone +// decides the allocation: `(..1).max(2**40)` is a request for 8 TiB of slice, +// from one short line of untrusted Ruby. MRI bounds it inside rb_ary_new2, +// which refuses a length above ARY_MAX_SIZE with ArgumentError; rbgo refuses +// with the same class so the answer is an exception the program can rescue and +// not a process the kernel kills. +// +// The control matters as much as the refusal: a cap that also refused ordinary +// counts would pass this test while breaking every real use. +func TestRangeMaxNOnABeginlessRangeRefusesAnUnboundedAllocation(t *testing.T) { + const bomb = `begin; (..1).max(2**40); rescue ArgumentError => e; p e.class; end` + if got := runSrc(t, bomb); got != "ArgumentError" { + t.Errorf("(..1).max(2**40) = %q, want ArgumentError", got) + } + const ok = `p((..1).max(1000).length)` + if got := runSrc(t, ok); got != "1000" { + t.Errorf("(..1).max(1000).length = %q, want 1000 -- the cap refuses an ordinary count", got) + } +}