range-check int and uint widths in unifyInt

[sahvx655-wq]

unifyInt only range-checks the fixed-width int and uint kinds; reflect.Int and reflect.Uint fall through unchecked, so on a 32-bit build a value above 2^31-1 decoded into an int or uint field is silently truncated instead of rejected. Spotted it reading the range checks next to the uint fix. Swapped the hand-rolled per-width comparisons for rv.OverflowInt and rv.OverflowUint, which are width-aware and also cover the platform-sized kinds. The num < 0 guard stays so negatives into unsigned still error.

[arp242]

Tests seem to fail on 32bit CI?

32bit systems are not all that common any more, but added that some years ago as people reported problems on 32bit systems, so it seems some people are still using it.

[sahvx655-wq]

Good catch. The decode.go change itself is fine, it was the new test that was wrong. I'd reused MaxInt32+1 for both the int and the uint case, but that value (2147483648) still fits inside a 32-bit uint, whose ceiling is MaxUint32 (4294967295). So OverflowUint correctly returns false and the uint assertion tripped, which is exactly the line that failed in the 386 log. The signed int case was right because MaxInt32+1 does cross the int32 boundary.

Pushed a fix that uses MaxUint32+1 for the uint case, so it actually overflows a 32-bit uint while staying within int64. I can't run 386 locally (no qemu on this machine), but the test binary cross-compiles clean under GOARCH=386 GOOS=linux and the threshold is now correct per kind, so the 32bit job should go green.

[arp242]

I copy/pasted your test to the current master branch, without the fix, and it passes there:

% go test
PASS
ok      github.com/BurntSushi/toml      0.003s

% git diff
diff --git i/decode_test.go w/decode_test.go
index 28019d9..2dd84b4 100644
--- i/decode_test.go
+++ w/decode_test.go
@@ -1233,3 +1233,27 @@ func TestMaxTableNesting(t *testing.T) {
 		})
 	}
 }
+
+// A value that fits int64 but not a 32-bit int/uint must still be rejected when
+// decoding into the platform-sized int/uint kinds, not silently truncated. The
+// thresholds differ per kind: int wraps past MaxInt32, uint past MaxUint32.
+func TestDecodeIntPlatformOverflow(t *testing.T) {
+	var i struct{ Value int }
+	_, err := Decode(fmt.Sprintf(`value = %d`, math.MaxInt32+int64(1)), &i)
+	if strconv.IntSize == 32 {
+		if err == nil {
+			t.Fatalf("int: expected out-of-range error on %d-bit int, got value %d", strconv.IntSize, i.Value)
+		}
+	} else if err != nil {
+		t.Fatalf("int: unexpected error on %d-bit int: %v", strconv.IntSize, err)
+	}
+	var u struct{ Value uint }
+	_, err = Decode(fmt.Sprintf(`value = %d`, math.MaxUint32+int64(1)), &u)
+	if strconv.IntSize == 32 {
+		if err == nil {
+			t.Fatalf("uint: expected out-of-range error on %d-bit uint, got value %d", strconv.IntSize, u.Value)
+		}
+	} else if err != nil {
+		t.Fatalf("uint: unexpected error on %d-bit uint: %v", strconv.IntSize, err)
+	}
+}

[sahvx655-wq]

Right, that's the gate doing its job rather than the test being wrong. The only path this PR changes behaviour on is the platform-sized reflect.Int/reflect.Uint kinds, and those are only narrower than int64 on a 32-bit build. On a 64-bit host int/uint are 64 bits wide, so MaxInt32+1 and MaxUint32+1 both fit, master and the branch decode them identically with no error, and the two assertions sit behind the strconv.IntSize == 32 gate so they're a no-op for you. The root cause is that master's hand-rolled checks only named Int8/16/32 and Uint8/16/32 explicitly; reflect.Int and reflect.Uint fell straight through to SetInt/SetUint, which don't range-check at all.

To see it actually go red on master, run it under the 386 job, the same one that flagged my earlier MaxInt32-for-uint slip: GOARCH=386 go test -run TestDecodeIntPlatformOverflow. On master that returns no error and silently truncates (SetInt stores -2147483648 for the int case, SetUint stores 0 for the uint), so the test fails; with OverflowInt/OverflowUint in place both are rejected and it passes. If you haven't got a 32-bit runner to hand, the mechanism shows on any arch with int32/uint32 standing in for the 386 width: OverflowInt(2147483648) is true while SetInt quietly stores -2147483648. The 386 CI job on the branch is green now, and I've tightened the test comment to spell out the gating so it reads less like a no-op.

[arp242]

Cheers, thanks