Thanks David, updated in SVN:

------------------------------------------------------------------------ r3800 | lewis | 2021-04-22 17:53:06 -0400 (Thu, 22 Apr 2021) | 3 lines

Integer arithmetic fix for sqUnixUtcWithOffset by David Stes http://lists.squeakfoundation.org/pipermail/vm-dev/2021-April/036189.html

Dave

On Thu, Apr 22, 2021 at 05:35:52PM -0400, David T. Lewis wrote:

Hi David,

Sorry have not had time to respond to this yet, but I'm definitely interested in the fix.

Thanks, Dave

On Tue, Apr 20, 2021 at 07:27:27PM +0200, stes@PANDORA.BE wrote:

...

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