Repository navigation
Int32 operations do not properly raise Overflow #370
Description
Version
110.99.8 (Latest)
Operating System
- Any
- Linux
- macOS
- Windows
- Other Unix
OS Version
Ubuntu 25.04
Processor
- Any
- Arm (using Rosetta)
- PowerPC
- Sparc
- x86 (32-bit)
- x86-64 (64-bit)
- Other
System Component
Basis Library
Severity
Minor
Description
Int32 operations that should raise Overflow in some circumstances (Int32.*, Int32.div, Int32.quot) do not raise the exception, instead returning an incorrect value.
Transcript
- Int32.* (~0x80000000, ~1);
val it = ~2147483648 : Int32.intExpected Behavior
- Int32.* (~0x80000000, ~1);
uncaught exception Overflow [overflow]
raised at: <file stdIn>Steps to Reproduce
See the transcript
Additional Information
This same issue happens with other values, and can result in Int32.int values that are out-of-bounds.
- Control.CG.printit := true;
val it = () : unit
- Int32.* (~0x80000000, ~8);
[omitted...]
[after closure ...]
std v52(v53[PV],v31[PV],v44[C],v45[PV],v46[PV],v47[PV],v33[PR2]) =
v33.0 -> v54[FN]
v48(v59[PV],v58[PV],v57[PV],v56[PV],v55[I]) =
{v55} -> v60
v58.1 -> v61[PV]
v58.0 -> v62[C]
v62(v62,v61,v57,v56,v60)
{RK_CONT 2,v44,v45} -> v63
v54.0 -> v64[FN]
v64(v64,v54,(L)v48,v63,v46,v47,(I63t)~2147483648,(I63t)~8)
Starting feedback...Finished
v52 E 4 3
v48 C 3 3
***********************************************
std v52(v53[PV],v31[PV],v44[C],v45[PV],v46[PV],v47[PV],v33[PR2]) =
v33.0 -> v54[FN]
{RK_CONT 2,v44,v45} -> v63
v54.0 -> v64[FN]
v64(v64,v54,(L)v48,v63,v46,v47,(I63t)~2147483648,(I63t)~8)
***********************************************
***********************************************
std_cont v48(v59[PV],v58[PV],v57[PV],v56[PV],v55[I]) =
{v55} -> v60
v58.1 -> v61[PV]
v58.0 -> v62[C]
v62(v62,v61,v57,v56,v60)
***********************************************
val it = 17179869184 : Int32.int
- 17179869184 : Int32.int;
stdIn:6.1-6.12 Error: literal '17179869184' is too large for type ?.int32
From this transcript you can also see that the issue is not in the cps optimizer
Email address
you know my email by now
Activity
JohnReppy commentedon Oct 5, 2025
This is quite strange, since the Int32 operations are wrapped with explicit overflow checks in
system/smlnj/init/target64-init.sml.Skyb0rg007 commentedon Oct 6, 2025
I would need to check, but the
toIntfunction is bound toInLine.int32_to_int, which may be silently masking the upper bits off the value. If that is the case, then those conditions could never be true.JohnReppy commentedon Oct 6, 2025
Good suggestion. The 32->63 bit conversion assumes that 32-bit ints are not sign extended, so it does a left shift followed by an arithmetic right shift to get the sign bits right. I think that the fix is to use the
COPYprimop when converting smaller signed integers to bigger ones (instead of theEXTENDprimop). Then, as long as smaller signed integers are always represented internally as 63-bit signed values, things should work okay. In the development branch, we will have to revert this change if we switch to using native 32-bit operations.[-]Int32 operations do not properly Overflow[/-][+]`Int32` operations do not properly raise `Overflow`[/+]JohnReppy commentedon Oct 6, 2025
It turns out that switching to
COPYdoes not quite work, since it breaks some of the conversions. The problem is thatEXTENDis used for bothWordN.toIntXandIntN.toIntconversions and we cannot always tell the difference. Instead, I have fixed this bug by using acastin thei32chkfunction (instead of thetoIntconversion).Fix for issue #370 (`Int32` operations do not properly raise `Overflow`)
Skyb0rg007 commentedon Oct 7, 2025
I built the current HEAD, and the issue is fixed on my machine. Might be worth adding a test to the regressions-tests repo to make sure it doesn't come back.
Skyb0rg007 commentedon Oct 7, 2025
Skyb0rg007 commentedon Oct 7, 2025
JohnReppy commentedon Oct 7, 2025
This bug is different and should have its own issue. I believe that we are seeing a problem in the string conversion code that was previously masked by the fact that
Overflowwas not being raised when it should have been.