Skip to content

Commit fef9b5d

Browse files
committed
fix(core): an all-zero magnitude never overflows, at any exponent (#2356)
Int32::Parse("0E30") threw OverflowException. .NET returns 0. THE RECORDED ANSWER WAS WRONG, AND SO WAS THE FIRST CORRECTION. Both are documented rather than quietly replaced. The ticket asked whether an all-zero magnitude with a non-positive scale overflows 'as .NET's source says it does'. It never overflows, at any scale. The decisive line is Number.Parsing.Common.cs:259-268, which runs after the trailing-token loop and, when StateNonZero was never set, forces Scale = 0 and clears IsNegative for an integer buffer with no decimal separator. The scale an all-zero magnitude would have overflowed on is discarded before the fold ever sees it. A middle answer was derived and is also wrong, and is recorded as a trap for the next reader: because line 103 makes a leading zero skip that same block, it does not advance the scale either, so the count of zeros cannot matter -- '0E-2' and '00E-2' must agree. The old gated pin asserted the opposite for the positive case and is replaced, not extended. Its own comment named why it was unreliable: 'the reading is an unexecutable source trace'. Four mutations, two caught, two reported honestly. M2 (dropping the !sawDecimal guard) is not observable in this port, because the unsigned path rejects '-' in the grammar -- filed as #2362, and the test now says so instead of implying a defence it cannot provide. M4 is semantically identical after normalisation. A pure widening: nothing that parsed before stops parsing. Gate 17,271 run, 0 failed, 38 executables. Record: docs/Migration-IntegerAllZeroMagnitude.md.
1 parent 15cd0d9 commit fef9b5d

4 files changed

Lines changed: 176 additions & 15 deletions

File tree

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
<!-- SPDX-License-Identifier: MIT -->
2+
<!-- Copyright (c) Robert Vokac and contributors -->
3+
4+
# Migration — an all-zero magnitude never overflows (ticket #2356)
5+
6+
*2026-08-18.* `Int32::Parse("0E30", NumberStyles::Any)` threw `OverflowException`. .NET returns
7+
`0`. So does this port now.
8+
9+
Landed under `docs/StandingApprovals.md` SA-5 (derivation from the reference). A **widening**: no
10+
input that parsed before stops parsing. No signature, layout, vtable or `noexcept` change.
11+
12+
---
13+
14+
## 1. What changed
15+
16+
| Call | Was | Is |
17+
|---|---|---|
18+
| `Parse("0E30")` | `OverflowException` | `0` |
19+
| `Parse("0E100000000")` | `OverflowException` | `0` |
20+
| `Parse("0E-1")`, `"0E-2"`, `"00E-2"`, `"0.0"` | `0` | `0` — unchanged |
21+
| `Parse("65E-1")`, `"1E100000000"` | `OverflowException` | unchanged |
22+
| every input with a nonzero digit || **unchanged** |
23+
24+
## 2. Why — and why the recorded answer was wrong twice
25+
26+
The deferred ticket asked whether an all-zero magnitude with a **non-positive scale** overflows
27+
*"as .NET's source says it does"*. It does not, at any scale, and the pin that guarded the
28+
question asserted the opposite for the positive case.
29+
30+
The pin's own comment named the reason it was unreliable — *"the reading is an unexecutable source
31+
trace"* — and the line the trace had missed is `Number.Parsing.Common.cs:259-268`, which runs
32+
after the trailing-token loop:
33+
34+
```csharp
35+
if ((state & StateNonZero) == 0)
36+
{
37+
if (number.Kind != NumberBufferKind.Decimal) number.Scale = 0;
38+
if (number.Kind == NumberBufferKind.Integer && !StateDecimal) number.IsNegative = false;
39+
}
40+
```
41+
42+
`StateNonZero` is set only inside `if (ch != '0' || (state & StateNonZero) != 0)`
43+
(`Number.Parsing.Common.cs:103`), so an all-zero magnitude never sets it — and **the scale it
44+
would have overflowed on is discarded before `TryNumberBufferToBinaryInteger` ever sees it.**
45+
46+
**A middle answer was reached and is also wrong**, so it is recorded here rather than left as a
47+
trap for the next reader. Because line 103 makes a *leading* zero skip the whole block, that zero
48+
does not advance the scale either. So the count of zeros written cannot matter: `"0E-2"` and
49+
`"00E-2"` must agree, and they do. An implementation that distinguishes them is wrong in one
50+
direction or the other.
51+
52+
## 3. The second half of the same normalisation
53+
54+
.NET also drops the **sign** — but only for an integer buffer that saw no decimal separator. So
55+
`UInt32.Parse("-0")` is `0` in .NET while `UInt32.Parse("-0.0")` overflows.
56+
57+
This port does not reproduce that, and the reason is a pre-existing, deliberate deviation rather
58+
than an oversight: the unsigned grammar rejects `-` outright, so `UInt32::Parse("-0")` and
59+
`UInt32::Parse("-1")` are both `FormatException` where .NET answers `0` and `OverflowException`.
60+
Repairing the `"-0"` row alone would align one spelling and leave the other diverging, which is
61+
worse than the present consistent rule. **Ticket #2362** holds the family.
62+
63+
One honest consequence, measured: the `!sawDecimal` guard in the signed core is therefore
64+
**unobservable today** — a mutation that drops it is not caught by any test, and was measured not
65+
to be. It is kept as a faithful transcription that becomes live when #2362 lands, and the test
66+
says so rather than implying a defence it cannot provide.
67+
68+
## 4. To migrate
69+
70+
Nothing to do. Code that caught `OverflowException` around a zero-valued literal with a large
71+
exponent will simply stop seeing it.

modules/core/include/System/detail/IntegerNumberStylesParser.hpp

Lines changed: 38 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,7 @@ struct IntegerNumberStylesParser {
155155
int digitsCount = 0; ///< .NET's `number.DigitsCount`.
156156
long long scale = 0; ///< .NET's `number.Scale`.
157157
bool any = false; ///< .NET's `StateDigits` — at least one digit was seen.
158+
bool sawDecimal = false; ///< .NET's `StateDecimal` — a decimal separator was consumed.
158159
bool overflowed = false; ///< The significant digits do not fit in 64 bits.
159160
};
160161

@@ -203,7 +204,7 @@ struct IntegerNumberStylesParser {
203204
}
204205
++i;
205206
} else if (allowDecimalPoint && !sawDecimal && s[i] == kDecimalSeparator) {
206-
sawDecimal = true; ++i;
207+
sawDecimal = true; scan.sawDecimal = true; ++i;
207208
} else if (allowThousands && scan.any && !sawDecimal && s[i] == kGroupSeparator) {
208209
++i;
209210
} else break;
@@ -247,13 +248,13 @@ struct IntegerNumberStylesParser {
247248
* however small the number looks. `"65E-1"` is 6.5 and fails here, which is exactly what
248249
* .NET's own `Int32Tests.cs:548` pins.
249250
*
250-
* @note An all-zero magnitude with a non-positive scale is the one case this port does not
251-
* follow; see the class doc-comment's deviation note and ticket #2356.
251+
* @note An all-zero magnitude needs no special case here: the caller has already applied
252+
* .NET's normalisation, which leaves it at scale 0 with no digits, so the general path
253+
* below accepts it and yields 0. Ticket #2356.
252254
*/
253255
static bool TryFoldScale(DigitScan& scan) {
254256
if (scan.overflowed) return false;
255257
if (scan.scale > kMaxSignificantScale) return false;
256-
if (scan.digitsCount == 0) { scan.magnitude = 0; return true; }
257258
if (scan.scale < scan.digitsCount) return false;
258259
for (long long k = scan.scale - scan.digitsCount; k > 0; --k) {
259260
if (scan.magnitude > UINT64_MAX / 10) return false;
@@ -335,6 +336,32 @@ struct IntegerNumberStylesParser {
335336

336337
if (haveParen) return false; // '(' opened but never closed
337338
if (i != n) return false;
339+
340+
// .NET's all-zero normalisation (`Number.Parsing.Common.cs:259-268`), transcribed at its
341+
// own position -- AFTER the trailing-token loop, which is what makes it reach a TRAILING
342+
// sign as well as a leading one.
343+
//
344+
// if ((state & StateNonZero) == 0) {
345+
// if (number.Kind != NumberBufferKind.Decimal) number.Scale = 0;
346+
// if (number.Kind == NumberBufferKind.Integer && !StateDecimal) number.IsNegative = false;
347+
// }
348+
//
349+
// `StateNonZero` is set only inside .NET's `if (ch != '0' || StateNonZero)` block, so it
350+
// means "a nonzero digit was seen" -- exactly this scan's `digitsCount != 0`, since a
351+
// zero enters the buffer only once a nonzero digit has. TWO CONSEQUENCES, and both are
352+
// easy to get wrong in opposite directions:
353+
//
354+
// * an all-zero magnitude NEVER overflows, at ANY exponent, because the scale it would
355+
// have overflowed on is discarded first -- "0E30" and "0E100000000" are 0, not
356+
// OverflowException (ticket #2356, where the recorded premise was the reverse);
357+
// * the sign is dropped too, so `UInt32::Parse("-0")` is 0 -- but ONLY when no decimal
358+
// separator was seen, which is why `UInt32::Parse("-0.0")` still fails. That
359+
// asymmetry is .NET's, not this port's convenience.
360+
if (scan.digitsCount == 0) {
361+
scan.scale = 0;
362+
if (!scan.sawDecimal) negative = false;
363+
}
364+
338365
// Format-grammar validity takes precedence over the scale overflow, matching real .NET's
339366
// own documented precedence rule -- checked only after the two lines above.
340367
if (!TryFoldScale(scan)) { overflowed = true; return true; }
@@ -431,6 +458,13 @@ struct IntegerNumberStylesParser {
431458
if (i < n && s[i] == '-') return false; // trailing minus: same rejection as leading
432459

433460
if (i != n) return false;
461+
462+
// .NET's all-zero normalisation, the same transcription as the signed core above
463+
// (`Number.Parsing.Common.cs:259-268`). Only the scale half applies here: a '-' never
464+
// reaches this point on the unsigned path, which is this parser's own long-standing
465+
// deviation and is documented on TryParseUnsignedCore.
466+
if (scan.digitsCount == 0) scan.scale = 0;
467+
434468
if (!TryFoldScale(scan)) { overflowed = true; return true; }
435469

436470
result = static_cast<SharpRuntime::ulongcs>(scan.magnitude);

modules/core/tests/System/NumberStylesExtendedTests.cpp

Lines changed: 67 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -424,23 +424,79 @@ TEST(IntegerAllowExponentTests, Fix2268_EveryIntegerWrapperGotTheFlagNotJustInt3
424424
EXPECT_EQ(10000000000000000000ull, UInt64::Parse("1E19", E, nullptr));
425425
}
426426

427-
TEST(IntegerAllowExponentTests, Fix2268_TheAllZeroMagnitudeIsTheOneDeliberateDeviation) {
428-
// Read literally, `Scale < DigitsCount` makes "0.0" an OverflowException in .NET: the
429-
// fractional zero takes the `Scale--` branch and leaves scale -1 against a digits count of
430-
// 0. This port keeps returning 0, because .NET's own suite pins no such row, the reading is
431-
// an unexecutable source trace, and it is a narrowing SR-AUD-177 never asked for. Ticket
432-
// #2356 holds the question; this test is what stops the answer changing by accident.
427+
TEST(IntegerAllowExponentTests, Fix2356_AnAllZeroMagnitudeNeverOverflowsAtAnyExponent) {
428+
// #2356 RESOLVED, AND IT REVERSED THE RECORDED ANSWER TWICE.
429+
//
430+
// The deferred pin this replaces asserted that an all-zero magnitude with an absurd POSITIVE
431+
// exponent overflows "exactly as .NET does". It does not. The pin's own comment named the
432+
// reason it was wrong -- "the reading is an unexecutable source trace" -- and the missing
433+
// line is `Number.Parsing.Common.cs:259-268`, which runs after the trailing-token loop:
434+
//
435+
// if ((state & StateNonZero) == 0) {
436+
// if (number.Kind != NumberBufferKind.Decimal) number.Scale = 0;
437+
// if (number.Kind == NumberBufferKind.Integer && !StateDecimal) number.IsNegative = false;
438+
// }
439+
//
440+
// `StateNonZero` is set only inside `if (ch != '0' || StateNonZero)`, so an all-zero magnitude
441+
// never sets it, and the scale it would have overflowed on IS DISCARDED BEFORE THE CHECK.
442+
//
443+
// A middle answer was considered and is also wrong: because a LEADING zero skips that same
444+
// block, it does not advance the scale either, so the count of zeros written cannot matter.
445+
// "0E-2" and "00E-2" must agree, and this test pins that they do.
446+
for (const char* zero : {"0", "00", "000"}) {
447+
SCOPED_TRACE(zero);
448+
EXPECT_EQ(0, Int32::Parse(zero, NumberStyles::Any, nullptr));
449+
}
450+
for (const char* text : {"0E-1", "0E-2", "00E-2", "000E-9", "0E1", "0E30", "0E100000000",
451+
"0E-100000000", "0.0E-5"}) {
452+
SCOPED_TRACE(text);
453+
EXPECT_EQ(0, Int32::Parse(text, NumberStyles::Any, nullptr));
454+
EXPECT_EQ(0u, UInt32::Parse(text, NumberStyles::Any, nullptr));
455+
}
456+
457+
// The decimal-point rows the old pin defended are unchanged, and now for the RIGHT reason:
458+
// not "this port declines a narrowing", but "this is what .NET computes".
433459
EXPECT_EQ(0, Int32::Parse("0.0", NumberStyles::Number, nullptr));
434460
EXPECT_EQ(0, Int32::Parse("000.000", NumberStyles::Number, nullptr));
435-
EXPECT_EQ(0, Int32::Parse("0E-2", NumberStyles::AllowExponent, nullptr));
436-
EXPECT_EQ(0, Int32::Parse("0E2", NumberStyles::AllowExponent, nullptr));
437461

438-
// The deviation is confined to a NON-POSITIVE scale. An all-zero magnitude with an absurd
439-
// positive exponent still overflows, exactly as .NET does.
440-
EXPECT_THROW((void)Int32::Parse("0E100000000", NumberStyles::AllowExponent, nullptr),
462+
// The normalisation is confined to an ALL-ZERO magnitude. One nonzero digit anywhere and the
463+
// ordinary scale rules apply again -- 6.5 is not an integer, and .NET's own Int32Tests.cs:548
464+
// pins that as an OverflowException.
465+
EXPECT_THROW((void)Int32::Parse("65E-1", NumberStyles::Any, nullptr), System::OverflowException);
466+
EXPECT_THROW((void)Int32::Parse("1E100000000", NumberStyles::Any, nullptr),
441467
System::OverflowException);
442468
}
443469

470+
TEST(IntegerAllowExponentTests, Fix2356_TheSignIsDroppedToo_ButOnlyWithoutADecimalSeparator) {
471+
// The normalisation's second half, which is easy to miss because it is guarded differently
472+
// from the first: .NET clears IsNegative only for an Integer-kind buffer that saw NO decimal
473+
// separator. So "-0" loses its sign and "-0.0" keeps it.
474+
//
475+
// HONEST LIMIT OF THESE ROWS. On the signed path both spellings are 0 either way, and on the
476+
// unsigned path a '-' never gets this far, so THE `!sawDecimal` GUARD IS CURRENTLY
477+
// UNOBSERVABLE: a mutation that drops it is not caught, by this suite or any other, and was
478+
// measured not to be (#2356 mutation M2). It is kept because it is a faithful transcription
479+
// of the reference line and becomes live the moment #2362 lets a '-' reach the unsigned
480+
// buffer -- not because a test defends it. The rows below still pin the values themselves.
481+
EXPECT_EQ(0, Int32::Parse("-0", NumberStyles::Any, nullptr));
482+
EXPECT_EQ(0, Int32::Parse("-0.0", NumberStyles::Number, nullptr));
483+
EXPECT_EQ(0, Int32::Parse("-0E-1", NumberStyles::Any, nullptr));
484+
485+
// .NET applies the normalisation AFTER its trailing-token loop, which is what makes it reach
486+
// a TRAILING sign as well as a leading one. This port transcribes that position.
487+
EXPECT_EQ(0, Int32::Parse("0-", NumberStyles::Any, nullptr));
488+
489+
// WHERE THIS PORT STILL DIVERGES, PINNED RATHER THAN QUIETLY FIXED. On the UNSIGNED path a
490+
// '-' is rejected by the grammar before any of this runs, so UInt32::Parse("-0") is a
491+
// FormatException where .NET returns 0. That is not a #2356 defect: it is the long-standing,
492+
// deliberate deviation documented on TryParseUnsignedCore, which declines to reproduce
493+
// .NET's negative-unsigned-throws-OverflowException quirk. Repairing the "-0" row alone
494+
// would align one spelling and leave "-1" diverging, which is worse than a consistent rule.
495+
// Ticket #2362 holds it.
496+
EXPECT_THROW((void)UInt32::Parse("-0", NumberStyles::Any, nullptr), System::FormatException);
497+
EXPECT_THROW((void)UInt32::Parse("-1", NumberStyles::Any, nullptr), System::FormatException);
498+
}
499+
444500
TEST(IntegerAllowExponentTests, Fix2268_TheDecimalPointRowsAreUntouched) {
445501
// The scale model replaced a `fracNonZero` flag, so every pre-existing decimal-point answer
446502
// has to be reproduced by the new code rather than merely left alone.

plan.sqlite3

0 Bytes
Binary file not shown.

0 commit comments

Comments
 (0)