And thanks to Esteban for pointing me to the bug.
2016-04-25 21:22 GMT+02:00 commits@source.squeak.org:
Nicolas Cellier uploaded a new version of VMMaker to project VM Maker: http://source.squeak.org/VMMaker/VMMaker.oscog-nice.1843.mcz
==================== Summary ====================
Name: VMMaker.oscog-nice.1843 Author: nice Time: 25 April 2016, 8:24:03.731 pm UUID: 9ef5d374-ae5e-437a-9116-cde9d708c0d7 Ancestors: VMMaker.oscog-cb.1842
Fix my recent bug for signed 32bit long access (introduced beginning of April).
One symptom is: (Alien newGC: 4) signedLongAt: 1 put: -16r7287E552; signedLongAt: 1
I incorrecly used the signed value (integerValue) in one branch, and the magnitude (value) in the other branch... This is an incorrect copy/paste from maybeInlinePositive32BitIntegerFor: in my ultimate #if SPURVM refactoring.
While at it, avoid the confusion by using a better variable name (magnitude), and eliminate UB related to computing opposite of INT_MIN by explicitely using unsigned type.
=============== Diff against VMMaker.oscog-cb.1842 ===============
Item was changed: ----- Method: StackInterpreter>>noInlineSigned32BitIntegerFor: (in category 'primitive support') ----- noInlineSigned32BitIntegerFor: integerValue "Answer a full 32 bit integer object for the given integer value." <notOption: #Spur64BitMemoryManager>
| newLargeInteger magnitude largeClass |
| newLargeInteger value largeClass | <inline: false>
<var: 'magnitude' type: 'unsigned int'> (objectMemory isIntegerValue: integerValue) ifTrue: [^objectMemory integerObjectOf: integerValue]. self deny: objectMemory hasSixtyFourBitImmediates. integerValue < 0 ifTrue: [largeClass :=
ClassLargeNegativeIntegerCompactIndex.
magnitude := 0 asUnsignedInteger -
integerValue]
value := 0 - integerValue] ifFalse: [largeClass :=
ClassLargePositiveIntegerCompactIndex.
magnitude := integerValue].
value := integerValue]. newLargeInteger := objectMemory
eeInstantiateSmallClassIndex: largeClass format: (objectMemory byteFormatForNumBytes: 4) numSlots: 1. self cppIf: SPURVM ifTrue: ["Memory is 8 byte aligned in Spur, make sure that oversized bytes are set to zero"
objectMemory storeLong32: 0 ofObject:
newLargeInteger withValue: (objectMemory byteSwapped32IfBigEndian: magnitude).
objectMemory storeLong32: 0 ofObject:
newLargeInteger withValue: (objectMemory byteSwapped32IfBigEndian: integerValue). objectMemory storeLong32: 1 ofObject: newLargeInteger withValue: 0] ifFalse:
[objectMemory storeLong32: 0 ofObject:
newLargeInteger withValue: (objectMemory byteSwapped32IfBigEndian: magnitude)].
[objectMemory storeLong32: 0 ofObject:
newLargeInteger withValue: (objectMemory byteSwapped32IfBigEndian: value)]. ^newLargeInteger!
and I confirm it works :)
Esteban
On 25 Apr 2016, at 21:41, Nicolas Cellier nicolas.cellier.aka.nice@gmail.com wrote:
And thanks to Esteban for pointing me to the bug.
2016-04-25 21:22 GMT+02:00 <commits@source.squeak.org mailto:commits@source.squeak.org>:
Nicolas Cellier uploaded a new version of VMMaker to project VM Maker: http://source.squeak.org/VMMaker/VMMaker.oscog-nice.1843.mcz http://source.squeak.org/VMMaker/VMMaker.oscog-nice.1843.mcz
==================== Summary ====================
Name: VMMaker.oscog-nice.1843 Author: nice Time: 25 April 2016, 8:24:03.731 pm UUID: 9ef5d374-ae5e-437a-9116-cde9d708c0d7 Ancestors: VMMaker.oscog-cb.1842
Fix my recent bug for signed 32bit long access (introduced beginning of April).
One symptom is: (Alien newGC: 4) signedLongAt: 1 put: -16r7287E552; signedLongAt: 1
I incorrecly used the signed value (integerValue) in one branch, and the magnitude (value) in the other branch... This is an incorrect copy/paste from maybeInlinePositive32BitIntegerFor: in my ultimate #if SPURVM refactoring.
While at it, avoid the confusion by using a better variable name (magnitude), and eliminate UB related to computing opposite of INT_MIN by explicitely using unsigned type.
=============== Diff against VMMaker.oscog-cb.1842 ===============
Item was changed: ----- Method: StackInterpreter>>noInlineSigned32BitIntegerFor: (in category 'primitive support') ----- noInlineSigned32BitIntegerFor: integerValue "Answer a full 32 bit integer object for the given integer value." <notOption: #Spur64BitMemoryManager>
| newLargeInteger magnitude largeClass |
| newLargeInteger value largeClass | <inline: false>
<var: 'magnitude' type: 'unsigned int'> (objectMemory isIntegerValue: integerValue) ifTrue: [^objectMemory integerObjectOf: integerValue]. self deny: objectMemory hasSixtyFourBitImmediates. integerValue < 0 ifTrue: [largeClass := ClassLargeNegativeIntegerCompactIndex.
magnitude := 0 asUnsignedInteger - integerValue]
value := 0 - integerValue] ifFalse: [largeClass := ClassLargePositiveIntegerCompactIndex.
magnitude := integerValue].
value := integerValue]. newLargeInteger := objectMemory eeInstantiateSmallClassIndex: largeClass format: (objectMemory byteFormatForNumBytes: 4) numSlots: 1. self cppIf: SPURVM ifTrue: ["Memory is 8 byte aligned in Spur, make sure that oversized bytes are set to zero"
objectMemory storeLong32: 0 ofObject: newLargeInteger withValue: (objectMemory byteSwapped32IfBigEndian: magnitude).
objectMemory storeLong32: 0 ofObject: newLargeInteger withValue: (objectMemory byteSwapped32IfBigEndian: integerValue). objectMemory storeLong32: 1 ofObject: newLargeInteger withValue: 0] ifFalse:
[objectMemory storeLong32: 0 ofObject: newLargeInteger withValue: (objectMemory byteSwapped32IfBigEndian: magnitude)].
[objectMemory storeLong32: 0 ofObject: newLargeInteger withValue: (objectMemory byteSwapped32IfBigEndian: value)]. ^newLargeInteger!
vm-dev@lists.squeakfoundation.org