From 73ce01231ca8e7f944b577242e119ef4234c27a1 Mon Sep 17 00:00:00 2001 From: Pablo Tesone Date: Mon, 25 Nov 2024 15:06:35 +0100 Subject: [PATCH 1/3] The classIndex that we read from the JITed code might be sign extended when encoding, so we need to guarantee that we bit mask it before using it and the correct types of the variables --- smalltalksrc/VMMaker/SpurMemoryManager.class.st | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/smalltalksrc/VMMaker/SpurMemoryManager.class.st b/smalltalksrc/VMMaker/SpurMemoryManager.class.st index df40628176..b959f4cc3c 100644 --- a/smalltalksrc/VMMaker/SpurMemoryManager.class.st +++ b/smalltalksrc/VMMaker/SpurMemoryManager.class.st @@ -8163,9 +8163,15 @@ SpurMemoryManager >> isValidClassIndex: classIndex [ ] { #category : 'class table' } -SpurMemoryManager >> isValidClassTag: classIndex [ +SpurMemoryManager >> isValidClassTag: classIndexToMask [ - | classOrNil | + | classOrNil classIndex | + + + + + classIndex := classIndexToMask bitAnd: 1 << self classIndexFieldWidth - 1. + self assert: (classIndex between: 0 and: 1 << self classIndexFieldWidth - 1). classOrNil := self classOrNilAtIndex: classIndex. ^classOrNil ~= nilObj From 3ebecd9f7e37aa22dfce4d652ca954fc63db6b41 Mon Sep 17 00:00:00 2001 From: Pablo Tesone Date: Mon, 25 Nov 2024 17:38:30 +0100 Subject: [PATCH 2/3] Changing the masking to the inlineCacheTagAt: method and adding tests --- .../VMMaker/CogAbstractInstruction.class.st | 2 +- smalltalksrc/VMMaker/CogIA32Compiler.class.st | 6 +- .../CogInLineLiteralsARMCompiler.class.st | 11 +++- .../CogInLineLiteralsX64Compiler.class.st | 6 +- .../VMMaker/CogMIPSELCompiler.class.st | 5 +- .../CogOutOfLineLiteralsARMCompiler.class.st | 7 ++- ...CogOutOfLineLiteralsARMv8Compiler.class.st | 8 ++- .../VMMaker/SpurMemoryManager.class.st | 9 +-- .../VMClassTagInlineReadTest.class.st | 62 +++++++++++++++++++ 9 files changed, 99 insertions(+), 17 deletions(-) create mode 100644 smalltalksrc/VMMakerTests/VMClassTagInlineReadTest.class.st diff --git a/smalltalksrc/VMMaker/CogAbstractInstruction.class.st b/smalltalksrc/VMMaker/CogAbstractInstruction.class.st index 16c1bba7cd..468899b9c2 100644 --- a/smalltalksrc/VMMaker/CogAbstractInstruction.class.st +++ b/smalltalksrc/VMMaker/CogAbstractInstruction.class.st @@ -1057,7 +1057,7 @@ CogAbstractInstruction >> getOperandsWithFormat: format [ ifTrue: [ (operand > 16 and: [ opcode ~= Label ]) ifTrue: [ (operand allMask: 16r80000000) - ifTrue: [ strOperands add: operand, '/', operand signedIntFromLong ]. + ifTrue: [ strOperands add: operand printString, '/', operand signedIntFromLong printString ]. strOperands add: operand asString, '/', (operand hex)] ifFalse: [ strOperands add: operand. diff --git a/smalltalksrc/VMMaker/CogIA32Compiler.class.st b/smalltalksrc/VMMaker/CogIA32Compiler.class.st index f059f78f96..9bde3e2b60 100644 --- a/smalltalksrc/VMMaker/CogIA32Compiler.class.st +++ b/smalltalksrc/VMMaker/CogIA32Compiler.class.st @@ -3646,7 +3646,11 @@ CogIA32Compiler >> hasThreeAddressArithmetic [ { #category : 'inline cacheing' } CogIA32Compiler >> inlineCacheTagAt: callSiteReturnAddress [ "Answer the inline cache tag for the return address of a send." - ^self literalBeforeFollowingAddress: callSiteReturnAddress - 5 + + + + ^ (self literalBeforeFollowingAddress: callSiteReturnAddress - 5) + bitAnd: 1 << objectMemory classIndexFieldWidth - 1 ] { #category : 'disassembly' } diff --git a/smalltalksrc/VMMaker/CogInLineLiteralsARMCompiler.class.st b/smalltalksrc/VMMaker/CogInLineLiteralsARMCompiler.class.st index 24131a9463..66a5b708e5 100644 --- a/smalltalksrc/VMMaker/CogInLineLiteralsARMCompiler.class.st +++ b/smalltalksrc/VMMaker/CogInLineLiteralsARMCompiler.class.st @@ -50,8 +50,15 @@ CogInLineLiteralsARMCompiler >> getDefaultCogCodeSize [ { #category : 'inline cacheing' } CogInLineLiteralsARMCompiler >> inlineCacheTagAt: callSiteReturnAddress [ "Answer the inline cache tag for the return address of a send." - self assert: (self instructionIsBL: (self instructionBeforeAddress: callSiteReturnAddress)). - ^self extract32BitOperandFrom4InstructionsPreceding: callSiteReturnAddress - 4 + + + + self assert: (self instructionIsBL: + (self instructionBeforeAddress: callSiteReturnAddress)). + + ^ (self extract32BitOperandFrom4InstructionsPreceding: + callSiteReturnAddress - 4) bitAnd: + 1 << objectMemory classIndexFieldWidth - 1 ] { #category : 'testing' } diff --git a/smalltalksrc/VMMaker/CogInLineLiteralsX64Compiler.class.st b/smalltalksrc/VMMaker/CogInLineLiteralsX64Compiler.class.st index 922e17b0a7..df831c2eca 100644 --- a/smalltalksrc/VMMaker/CogInLineLiteralsX64Compiler.class.st +++ b/smalltalksrc/VMMaker/CogInLineLiteralsX64Compiler.class.st @@ -164,7 +164,11 @@ CogInLineLiteralsX64Compiler >> getDefaultCogCodeSize [ { #category : 'inline cacheing' } CogInLineLiteralsX64Compiler >> inlineCacheTagAt: callSiteReturnAddress [ "Answer the inline cache tag for the return address of a send." - ^self literal32BeforeFollowingAddress: callSiteReturnAddress - 5 + + + + ^ (self literal32BeforeFollowingAddress: callSiteReturnAddress - 5) + bitAnd: 1 << objectMemory classIndexFieldWidth - 1 ] { #category : 'testing' } diff --git a/smalltalksrc/VMMaker/CogMIPSELCompiler.class.st b/smalltalksrc/VMMaker/CogMIPSELCompiler.class.st index c29ae150b2..03d13c0f8f 100644 --- a/smalltalksrc/VMMaker/CogMIPSELCompiler.class.st +++ b/smalltalksrc/VMMaker/CogMIPSELCompiler.class.st @@ -1908,6 +1908,8 @@ CogMIPSELCompiler >> inlineCacheTagAt: callSiteReturnAddress [ ... <-- callSiteReturnAddress" + + self assert: (self opcodeAtAddress: callSiteReturnAddress - 24) = LUI. self assert: (self opcodeAtAddress: callSiteReturnAddress - 20) = ORI. self assert: (self opcodeAtAddress: callSiteReturnAddress - 16) = LUI. @@ -1916,7 +1918,8 @@ CogMIPSELCompiler >> inlineCacheTagAt: callSiteReturnAddress [ self assert: (self functionAtAddress: callSiteReturnAddress - 8) = JALR. self assert: (objectMemory longAt: callSiteReturnAddress - 4) = self nop. - ^self literalAtAddress: callSiteReturnAddress - 20 + ^(self literalAtAddress: callSiteReturnAddress - 20) bitAnd: + 1 << objectMemory classIndexFieldWidth - 1 ] { #category : 'disassembly' } diff --git a/smalltalksrc/VMMaker/CogOutOfLineLiteralsARMCompiler.class.st b/smalltalksrc/VMMaker/CogOutOfLineLiteralsARMCompiler.class.st index 377668bb6c..e28459b738 100644 --- a/smalltalksrc/VMMaker/CogOutOfLineLiteralsARMCompiler.class.st +++ b/smalltalksrc/VMMaker/CogOutOfLineLiteralsARMCompiler.class.st @@ -51,8 +51,11 @@ CogOutOfLineLiteralsARMCompiler >> getDefaultCogCodeSize [ { #category : 'inline cacheing' } CogOutOfLineLiteralsARMCompiler >> inlineCacheTagAt: callSiteReturnAddress [ - - ^objectMemory uint32AtPointer: (self pcRelativeAddressAt: (callSiteReturnAddress - 8) asUnsignedInteger) + + + + ^(objectMemory uint32AtPointer: (self pcRelativeAddressAt: (callSiteReturnAddress - 8) asUnsignedInteger)) bitAnd: + 1 << objectMemory classIndexFieldWidth - 1 ] { #category : 'testing' } diff --git a/smalltalksrc/VMMaker/CogOutOfLineLiteralsARMv8Compiler.class.st b/smalltalksrc/VMMaker/CogOutOfLineLiteralsARMv8Compiler.class.st index de8ad78f83..a0f2232850 100644 --- a/smalltalksrc/VMMaker/CogOutOfLineLiteralsARMv8Compiler.class.st +++ b/smalltalksrc/VMMaker/CogOutOfLineLiteralsARMv8Compiler.class.st @@ -207,8 +207,12 @@ CogOutOfLineLiteralsARMv8Compiler >> getDefaultCogCodeSize [ { #category : 'inline cacheing' } CogOutOfLineLiteralsARMv8Compiler >> inlineCacheTagAt: callSiteReturnAddress [ - - ^objectMemory unsignedLongAt: (self pcRelativeAddressAt: (callSiteReturnAddress - 8) asUnsignedInteger) + + + + ^ (objectMemory unsignedLongAt: (self pcRelativeAddressAt: + (callSiteReturnAddress - 8) asUnsignedInteger)) bitAnd: + 1 << objectMemory classIndexFieldWidth - 1 ] { #category : 'testing' } diff --git a/smalltalksrc/VMMaker/SpurMemoryManager.class.st b/smalltalksrc/VMMaker/SpurMemoryManager.class.st index b959f4cc3c..5c83ec9197 100644 --- a/smalltalksrc/VMMaker/SpurMemoryManager.class.st +++ b/smalltalksrc/VMMaker/SpurMemoryManager.class.st @@ -8163,14 +8163,9 @@ SpurMemoryManager >> isValidClassIndex: classIndex [ ] { #category : 'class table' } -SpurMemoryManager >> isValidClassTag: classIndexToMask [ +SpurMemoryManager >> isValidClassTag: classIndex [ - | classOrNil classIndex | - - - - - classIndex := classIndexToMask bitAnd: 1 << self classIndexFieldWidth - 1. + | classOrNil | self assert: (classIndex between: 0 and: 1 << self classIndexFieldWidth - 1). classOrNil := self classOrNilAtIndex: classIndex. diff --git a/smalltalksrc/VMMakerTests/VMClassTagInlineReadTest.class.st b/smalltalksrc/VMMakerTests/VMClassTagInlineReadTest.class.st new file mode 100644 index 0000000000..1980f86f87 --- /dev/null +++ b/smalltalksrc/VMMakerTests/VMClassTagInlineReadTest.class.st @@ -0,0 +1,62 @@ +Class { + #name : 'VMClassTagInlineReadTest', + #superclass : 'VMPrimitiveCallAbstractTest', + #pools : [ + 'CogRTLOpcodes' + ], + #category : 'VMMakerTests-JitTests', + #package : 'VMMakerTests', + #tag : 'JitTests' +} + +{ #category : 'tests' } +VMClassTagInlineReadTest >> testLinkingWithEntryOffset [ + + | sendingMethod targetMethod callSiteReturn | + sendingMethod := self + jitMethod: (self findMethod: #methodWithSend) + selector: memory nilObject. + + targetMethod := self + jitMethod: (self findMethod: #yourself) + selector: memory trueObject. + + callSiteReturn := sendingMethod address + 16r98. + + cogit + linkSendAt: callSiteReturn + in: sendingMethod + to: targetMethod + offset: cogit entryOffset + receiver: memory falseObject. + + self assert: (cogit backend inlineCacheTagAt: callSiteReturn) equals:(memory classIndexOf: memory falseObject) +] + +{ #category : 'tests' } +VMClassTagInlineReadTest >> testLinkingWithEntryOffsetLargeClassIndex [ + + | sendingMethod targetMethod callSiteReturn | + sendingMethod := self + jitMethod: (self findMethod: #methodWithSend) + selector: memory nilObject. + + targetMethod := self + jitMethod: (self findMethod: #yourself) + selector: memory trueObject. + + callSiteReturn := sendingMethod address + 16r98. + + obj := self newZeroSizedObject. + memory setClassIndexOf: obj to: (1 << memory classIndexFieldWidth - 5). + + cogit + linkSendAt: callSiteReturn + in: sendingMethod + to: targetMethod + offset: cogit entryOffset + receiver: obj. + + + self assert: (cogit backend inlineCacheTagAt: callSiteReturn) equals: (1 << memory classIndexFieldWidth - 5) +] From f572ef1d148f236c96f9cd78d2996078fe69d2b0 Mon Sep 17 00:00:00 2001 From: Pablo Tesone Date: Mon, 25 Nov 2024 18:00:07 +0100 Subject: [PATCH 3/3] Adding the API tag --- smalltalksrc/VMMaker/SpurMemoryManager.class.st | 1 + 1 file changed, 1 insertion(+) diff --git a/smalltalksrc/VMMaker/SpurMemoryManager.class.st b/smalltalksrc/VMMaker/SpurMemoryManager.class.st index 5c83ec9197..77e51650a5 100644 --- a/smalltalksrc/VMMaker/SpurMemoryManager.class.st +++ b/smalltalksrc/VMMaker/SpurMemoryManager.class.st @@ -3697,6 +3697,7 @@ SpurMemoryManager >> classFormatFromInstFormat: instFormat [ { #category : 'header format' } SpurMemoryManager >> classIndexFieldWidth [ + "22-bit class mask => ~ 4M classes" ^22 ]