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

[stes@PANDORA.BE]

-----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.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQEcBAEBCAAGBQJgfw4nAAoJENdFDkXGicizFVQH/3GhR/K0K9H9AyS4uBCeEIxg
lv4rCq2w9tXWeLP8eUc50sJnAsK7UVnO6FUpdFmF0qauN0414C5KmrQcIivNoEvH
kMiWduIWXfzX7VlHj56pyuYym73qOZDZowFOvnPjaVQfKcqC9yyabYrEF7THc8j3
F8bVwCy/oz1h9/Mi6VbCrjz1G+ePmowXiv5vkJFAwQDNGrc53OyhQm6Wfibdwb/8
6C2C/loPeStDf9f3Y3zhIkelp0RI4TL+h+D+REaf0kWV/TubIx648hcP4uvBhyQk
6hwD7OHHechkJLgYAni1ipGXfX03cAM2plLIgKHahypNz1mVcZhu/6SSMTQK1ik=
=gf1g
-----END PGP SIGNATURE-----
-------------- 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>