[MLton] Bug in IntInf implementation

Wesley W. Terpstra wesley at terpstra.ca
Fri Dec 7 16:04:54 PST 2007


On Dec 7, 2007, at 10:30 PM, Matthew Fluet wrote:
> initIntInfRes has every reason to believe that the current heap can  
> contain the result of the operation.  (Indeed, there are numerous  
> 'assert's to that effect in the code.)  All of the IntInf primitives  
> (both internally as manipulated by the compiler and externally as  
> implemented in runtime/gc/int-inf.c) take a size_t argument which,  
> at runtime, will be the maximum size of the resulting IntInf.  The  
> limit check insertion pass integrates these arguments into the limit  
> checks to ensure that there is always sufficient space in the heap  
> to contain the result.

My mistake! I was looking through the runtime and didn't see any check  
to grow the heap. So, intInfBinary in ssa-to-rssa.fun introduces a C  
function call (complete with the size parameter). The predicted size  
comes from the basis:
>          val bigAdd = make (I.+!, Prim.+, S.max, 1)
>          val bigSub = make (I.-!, Prim.-, S.max, 1)
>          val bigMul = make (I.*!, Prim.*, S.+, 0)
This is all way earlier than I expected.

I'm unable to find where the third argument to the IntInf primitives  
is used in a limit check. I trust that it happens, though.

It still might not be a bad idea to hook the gmp memory management  
methods to an assert failure, in case the predicted size is wrong.

> In fact, the program does not hang, but is simply computing the  
> result rather slowly.  See <src>/basis-library/integer/int-inf.sml.   
> IntInf shifts are limited to 128 bits at a time.  I know I'm the one  
> who wrote that code, and I have a very vague recollection that there  
> was a reason, but I can't recall it now.  One issue is that if one  
> compiles with '-default-type word64', then the shift argument of  
> IntInf.~>> and IntInf.<< (as exported by the Basis Library) is a  
> Word64.word, but on a 32-bit platform, the GMP primitives will only  
> accept a 32-bit word as the shift amount.  And, in any case, we've  
> made the IntInf shift primitives require a 32-bit shift, so one  
> needs a cap.  But, obviously, it could be much higher than 128.

Ok. I don't know if a real application needs this operation to be  
fast. I tried it only to overflow the heap, after it seemed something  
was amiss in the int-inf.c file.




More information about the MLton mailing list