-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256
Hi,
This is a patch for subversion squeakvm 4.19.6.
It relates to the primitiveLocalMicrosecondClock.
It does not affect OpenSmalltalk (I think the code in OpenSmalltalk is different and is not affected by the following problem).
I'm not sure it is actually desired at all to implement primitiveLocalMicrosecondClock in the 32bit case in the classical SqueakVM, but if it is, there is an arithmetical overflow on my platform (Solaris 32bit).
I don't think it affects the classical SqueakVM Solaris 64bit by the way, (for the 68000 type of Smalltalk images).
The patch is for the file platforms/unix/vm/sqUnixMain.c function sqUnixUtcWithOffset().
This function is doing:
time_t seconds= timeval.tv_sec; suseconds_t usec= timeval.tv_usec; *microSeconds= seconds * 1000000 + usec;
unfortunately the multiplication (seconds * 1000000) is triggering overflow.
The type time_t is defined for me as: typedef long time_t; typedef long suseconds_t; /* signed # of microseconds */
Note that I believe that in the 32bit case this has size 4 bytes and in the 64bit case this has size 8 bytes (for me on Solaris).
So Solaris 64bit is not affected by the multiplication error, but 32bit is.
I propose the following patch:
Index: vm/sqUnixMain.c =================================================================== - --- vm/sqUnixMain.c (revision 3799) +++ vm/sqUnixMain.c (working copy) @@ -213,11 +213,13 @@ */ sqInt sqUnixUtcWithOffset(sqLong *microSeconds, int *offset) { + sqLong theMicroSeconds; struct timeval timeval; if (gettimeofday(&timeval, NULL) == -1) return -1; time_t seconds= timeval.tv_sec; suseconds_t usec= timeval.tv_usec; - - *microSeconds= seconds * 1000000 + usec; + theMicroSeconds = seconds; + *microSeconds= theMicroSeconds * 1000000 + usec;
The rationale is that the function seems to assume anyway that it is possible to store the result in *microSeconds which is of type sqLong.
So by defining a local variable theMicroSeconds and casting seconds to that type, and then multiplying (instead of 32bit multiply, on the 64bit), it may work ... at least it seems to work for me.
Maybe an alternative could be simply NOT to implement primitiveLocalMicrosecondClock in the 32bit case ...
If primitiveLocalMicrosecondClock would return nil (primitiveFail) in the 32bit case, it would also be a possible solution.
The primitive assumes anyway that it returns the microseconds as LargeInteger.
David Stes
PS1: in attach the patch to apply if desired
PS2: I've tested this on Solaris - all platforms are affected; it is hard to say for me how other platforms are affected, given their time_t definition and their sqLong definition.