[Vm-dev] patch for subversion (classical VM) primitiveLocalMicroseconds

stes@PANDORA.BE stes at telenet.be
Tue Apr 20 17:27:27 UTC 2021

Hash: SHA256


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.

Version: GnuPG v2

-------------- next part --------------
A non-text attachment was scrubbed...
Name: primitiveLocalMicrosecondClock.patch
Type: text/x-patch
Size: 615 bytes
Desc: not available
URL: <http://lists.squeakfoundation.org/pipermail/vm-dev/attachments/20210420/6d774dd8/attachment.bin>

More information about the Vm-dev mailing list