Problem with failing primitiveStringHash? | Cannot load metacello.sar about squeakjs

[codefrau]

So ca63f69 checks the stack after every primitive.
And 4e152a6 fixes an embarrassing number of primitives that left the stack imbalanced or at least did not check argCount.
Released as 1.0.1.

[fniephaus]

No sure what's going on, but the implementation of primitiveStringHash is generated and looks ok:

SqueakJS/plugins/MiscPrimitivePlugin.js

Lines 544 to 574 in 3538c55

	function primitiveStringHash() {
	var rcvr;
	var aByteArray;
	var speciesHash;
	var byteArraySize;
	var hash;
	var low;
	var pos;
	rcvr = interpreterProxy.stackValue(2);
	aByteArray = interpreterProxy.stackBytes(1);
	speciesHash = interpreterProxy.stackIntegerValue(0);
	if (interpreterProxy.failed()) {
	return null;
	}
	byteArraySize = aByteArray.length;
	hash = speciesHash & 268435455;
	for (pos = 1; pos <= byteArraySize; pos++) {
	/* Begin hashMultiply */
	hash += aByteArray[pos - 1];
	low = hash & 16383;
	hash = ((9741 * low) + ((((9741 * (hash >>> 14)) + (101 * low)) & 16383) * 16384)) & 268435455;
	}
	if (interpreterProxy.failed()) {
	return null;
	}
	interpreterProxy.popthenPush(3, hash);
	return null;
	}

[codefrau]

OMG this must have been wrong since 2014. I just tried it with

primitiveStringHash(argCount) { 
   ... 
   interpreterProxy.popthenPush(argCount + 1, hash);
}

and metacello.sar continues installing fine.

However, that means some other generated primitives are likely wrong too 🤔 I have not committed a fix yet.

Thank you for the test case!

[codefrau]

@LinqLover please use the vm-dev list for this kind of question/discussion. Nobody’s gonna read through closed tickets ;)

[codefrau]

That primitive looks buggy, because it is called with argCount = 1 so it should pop 2 (string+arg) but actually pops 3.

What does it do in other VMs? Are they using argCount? This was generated many years ago, but I don't think the primitive changed?

[krono]

So ca63f69 checks the stack after every primitive.
And 4e152a6 fixes an embarrassing number of primitives that left the stack imbalanced or at least did not check argCount.
Released as 1.0.1.

This is always fun :D

[LinqLover]

Great, thanks for fixing! :D

So ca63f69 checks the stack after every primitive.

Out of curiosity, how large is the performance impact of such "assertions in production"? :-)