[LLInt] use loadp consistently for get_from_scope/put_to_scope
authorcaitp@igalia.com <caitp@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 13 Jun 2018 22:51:29 +0000 (22:51 +0000)
committercaitp@igalia.com <caitp@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 13 Jun 2018 22:51:29 +0000 (22:51 +0000)
https://bugs.webkit.org/show_bug.cgi?id=132333

Reviewed by Mark Lam.

Using `loadis` for register indexes and `loadp` for constant scopes /
symboltables makes sense, but is problematic for big-endian
architectures.

Consistently treating the operand as a pointer simplifies determining
how to access the operand, and helps avoid bad accesses and crashes on
big-endian ports.

* bytecode/CodeBlock.cpp:
(JSC::CodeBlock::finishCreation):
* bytecode/Instruction.h:
* jit/JITOperations.cpp:
* llint/LLIntSlowPaths.cpp:
(JSC::LLInt::LLINT_SLOW_PATH_DECL):
* llint/LowLevelInterpreter32_64.asm:
* llint/LowLevelInterpreter64.asm:
* runtime/CommonSlowPaths.h:
(JSC::CommonSlowPaths::tryCachePutToScopeGlobal):
(JSC::CommonSlowPaths::tryCacheGetFromScopeGlobal):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@232816 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/bytecode/CodeBlock.cpp
Source/JavaScriptCore/bytecode/Instruction.h
Source/JavaScriptCore/jit/JITOperations.cpp
Source/JavaScriptCore/llint/LLIntSlowPaths.cpp
Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm
Source/JavaScriptCore/llint/LowLevelInterpreter64.asm
Source/JavaScriptCore/runtime/CommonSlowPaths.h

index 206fcc3..99f3f80 100644 (file)
@@ -1,3 +1,30 @@
+2018-06-13  Caitlin Potter  <caitp@igalia.com>
+
+        [LLInt] use loadp consistently for get_from_scope/put_to_scope
+        https://bugs.webkit.org/show_bug.cgi?id=132333
+
+        Reviewed by Mark Lam.
+
+        Using `loadis` for register indexes and `loadp` for constant scopes /
+        symboltables makes sense, but is problematic for big-endian
+        architectures.
+
+        Consistently treating the operand as a pointer simplifies determining
+        how to access the operand, and helps avoid bad accesses and crashes on
+        big-endian ports.
+
+        * bytecode/CodeBlock.cpp:
+        (JSC::CodeBlock::finishCreation):
+        * bytecode/Instruction.h:
+        * jit/JITOperations.cpp:
+        * llint/LLIntSlowPaths.cpp:
+        (JSC::LLInt::LLINT_SLOW_PATH_DECL):
+        * llint/LowLevelInterpreter32_64.asm:
+        * llint/LowLevelInterpreter64.asm:
+        * runtime/CommonSlowPaths.h:
+        (JSC::CommonSlowPaths::tryCachePutToScopeGlobal):
+        (JSC::CommonSlowPaths::tryCacheGetFromScopeGlobal):
+
 2018-06-13  Keith Miller  <keith_miller@apple.com>
 
         AutomaticThread should have a way to provide a thread name
index 98ef06f..f656d76 100644 (file)
@@ -694,7 +694,7 @@ bool CodeBlock::finishCreation(VM& vm, ScriptExecutable* ownerExecutable, Unlink
                 instructions[i + 5].u.watchpointSet = op.watchpointSet;
             else if (op.structure)
                 instructions[i + 5].u.structure.set(vm, this, op.structure);
-            instructions[i + 6].u.pointer = reinterpret_cast<void*>(op.operand);
+            instructions[i + 6].u.operandPointer = op.operand;
             break;
         }
 
@@ -731,7 +731,7 @@ bool CodeBlock::finishCreation(VM& vm, ScriptExecutable* ownerExecutable, Unlink
                     op.watchpointSet->invalidate(vm, PutToScopeFireDetail(this, ident));
             } else if (op.structure)
                 instructions[i + 5].u.structure.set(vm, this, op.structure);
-            instructions[i + 6].u.pointer = reinterpret_cast<void*>(op.operand);
+            instructions[i + 6].u.operandPointer = op.operand;
 
             break;
         }
index c133578..1182b40 100644 (file)
@@ -123,6 +123,7 @@ struct Instruction {
         Opcode opcode;
         int operand;
         unsigned unsignedValue;
+        intptr_t operandPointer;
         WriteBarrierBase<Structure> structure;
         StructureID structureID;
         WriteBarrierBase<SymbolTable> symbolTable;
index 5381dd6..ccd9b2b 100644 (file)
@@ -2381,7 +2381,7 @@ void JIT_OPERATION operationPutToScope(ExecState* exec, Instruction* bytecodePC)
 
     if (getPutInfo.resolveType() == LocalClosureVar) {
         JSLexicalEnvironment* environment = jsCast<JSLexicalEnvironment*>(scope);
-        environment->variableAt(ScopeOffset(pc[6].u.operand)).set(vm, environment, value);
+        environment->variableAt(ScopeOffset(pc[6].u.operandPointer)).set(vm, environment, value);
         if (WatchpointSet* set = pc[5].u.watchpointSet)
             set->touch(vm, "Executed op_put_scope<LocalClosureVar>");
         return;
index c8d3e47..a0b6e9c 100644 (file)
@@ -1729,7 +1729,7 @@ LLINT_SLOW_PATH_DECL(slow_path_put_to_scope)
     GetPutInfo getPutInfo = GetPutInfo(pc[4].u.operand);
     if (getPutInfo.resolveType() == LocalClosureVar) {
         JSLexicalEnvironment* environment = jsCast<JSLexicalEnvironment*>(scope);
-        environment->variableAt(ScopeOffset(pc[6].u.operand)).set(vm, environment, value);
+        environment->variableAt(ScopeOffset(pc[6].u.operandPointer)).set(vm, environment, value);
         
         // Have to do this *after* the write, because if this puts the set into IsWatched, then we need
         // to have already changed the value of the variable. Otherwise we might watch and constant-fold
index 80f41d8..8a7bcf4 100644 (file)
@@ -2339,7 +2339,7 @@ macro loadWithStructureCheck(operand, slowPath)
 end
 
 macro getProperty()
-    loadisFromInstruction(6, t3)
+    loadpFromInstruction(6, t3)
     loadPropertyAtVariableOffset(t3, t0, t1, t2)
     valueProfile(t1, t2, 28, t0)
     loadisFromInstruction(1, t0)
@@ -2359,7 +2359,7 @@ macro getGlobalVar(tdzCheckIfNecessary)
 end
 
 macro getClosureVar()
-    loadisFromInstruction(6, t3)
+    loadpFromInstruction(6, t3)
     loadp JSLexicalEnvironment_variables + TagOffset[t0, t3, 8], t1
     loadp JSLexicalEnvironment_variables + PayloadOffset[t0, t3, 8], t2
     valueProfile(t1, t2, 28, t0)
@@ -2434,7 +2434,7 @@ _llint_op_get_from_scope:
 macro putProperty()
     loadisFromInstruction(3, t1)
     loadConstantOrVariable(t1, t2, t3)
-    loadisFromInstruction(6, t1)
+    loadpFromInstruction(6, t1)
     storePropertyAtVariableOffset(t1, t0, t2, t3)
 end
 
@@ -2451,7 +2451,7 @@ end
 macro putClosureVar()
     loadisFromInstruction(3, t1)
     loadConstantOrVariable(t1, t2, t3)
-    loadisFromInstruction(6, t1)
+    loadpFromInstruction(6, t1)
     storei t2, JSLexicalEnvironment_variables + TagOffset[t0, t1, 8]
     storei t3, JSLexicalEnvironment_variables + PayloadOffset[t0, t1, 8]
 end
@@ -2463,7 +2463,7 @@ macro putLocalClosureVar()
     btpz t5, .noVariableWatchpointSet
     notifyWrite(t5, .pDynamic)
 .noVariableWatchpointSet:
-    loadisFromInstruction(6, t1)
+    loadpFromInstruction(6, t1)
     storei t2, JSLexicalEnvironment_variables + TagOffset[t0, t1, 8]
     storei t3, JSLexicalEnvironment_variables + PayloadOffset[t0, t1, 8]
 end
index f867597..c39988c 100644 (file)
@@ -1500,7 +1500,7 @@ _llint_op_put_by_id:
     bineq t1, JSCell::m_structureID[t3], .opPutByIdSlow
 
 .opPutByIdDoneCheckingTypes:
-    loadisFromInstruction(6, t1)
+    loadpFromInstruction(6, t1)
     
     btiz t1, .opPutByIdNotTransition
 
@@ -1530,7 +1530,7 @@ _llint_op_put_by_id:
 
 .opPutByIdTransitionChainDone:
     # Reload the new structure, since we clobbered it above.
-    loadisFromInstruction(6, t1)
+    loadpFromInstruction(6, t1)
 
 .opPutByIdTransitionDirect:
     storei t1, JSCell::m_structureID[t0]
@@ -2355,7 +2355,7 @@ macro loadWithStructureCheck(operand, slowPath)
 end
 
 macro getProperty()
-    loadisFromInstruction(6, t1)
+    loadpFromInstruction(6, t1)
     loadPropertyAtVariableOffset(t1, t0, t2)
     valueProfile(t2, 7, t0)
     loadisFromInstruction(1, t0)
@@ -2372,7 +2372,7 @@ macro getGlobalVar(tdzCheckIfNecessary)
 end
 
 macro getClosureVar()
-    loadisFromInstruction(6, t1)
+    loadpFromInstruction(6, t1)
     loadq JSLexicalEnvironment_variables[t0, t1, 8], t0
     valueProfile(t0, 7, t1)
     loadisFromInstruction(1, t1)
@@ -2445,7 +2445,7 @@ _llint_op_get_from_scope:
 macro putProperty()
     loadisFromInstruction(3, t1)
     loadConstantOrVariable(t1, t2)
-    loadisFromInstruction(6, t1)
+    loadpFromInstruction(6, t1)
     storePropertyAtVariableOffset(t1, t0, t2)
 end
 
@@ -2461,7 +2461,7 @@ end
 macro putClosureVar()
     loadisFromInstruction(3, t1)
     loadConstantOrVariable(t1, t2)
-    loadisFromInstruction(6, t1)
+    loadpFromInstruction(6, t1)
     storeq t2, JSLexicalEnvironment_variables[t0, t1, 8]
 end
 
@@ -2472,7 +2472,7 @@ macro putLocalClosureVar()
     btpz t3, .noVariableWatchpointSet
     notifyWrite(t3, .pDynamic)
 .noVariableWatchpointSet:
-    loadisFromInstruction(6, t1)
+    loadpFromInstruction(6, t1)
     storeq t2, JSLexicalEnvironment_variables[t0, t1, 8]
 end
 
index 1ece895..e263011 100644 (file)
@@ -138,7 +138,7 @@ inline void tryCachePutToScopeGlobal(
             ASSERT(!entry.isNull());
             ConcurrentJSLocker locker(codeBlock->m_lock);
             pc[5].u.watchpointSet = entry.watchpointSet();
-            pc[6].u.pointer = static_cast<void*>(globalLexicalEnvironment->variableAt(entry.scopeOffset()).slot());
+            pc[6].u.pointer = globalLexicalEnvironment->variableAt(entry.scopeOffset()).slot();
         }
     }
     
@@ -162,7 +162,7 @@ inline void tryCachePutToScopeGlobal(
 
         ConcurrentJSLocker locker(codeBlock->m_lock);
         pc[5].u.structure.set(vm, codeBlock, scope->structure(vm));
-        pc[6].u.operand = slot.cachedOffset();
+        pc[6].u.operandPointer = slot.cachedOffset();
     }
 }
 
@@ -186,7 +186,7 @@ inline void tryCacheGetFromScopeGlobal(
             ConcurrentJSLocker locker(exec->codeBlock()->m_lock);
             pc[4].u.operand = GetPutInfo(getPutInfo.resolveMode(), newResolveType, getPutInfo.initializationMode()).operand();
             pc[5].u.watchpointSet = entry.watchpointSet();
-            pc[6].u.pointer = static_cast<void*>(globalLexicalEnvironment->variableAt(entry.scopeOffset()).slot());
+            pc[6].u.pointer = globalLexicalEnvironment->variableAt(entry.scopeOffset()).slot();
         }
     }
 
@@ -200,7 +200,7 @@ inline void tryCacheGetFromScopeGlobal(
             {
                 ConcurrentJSLocker locker(codeBlock->m_lock);
                 pc[5].u.structure.set(vm, codeBlock, structure);
-                pc[6].u.operand = slot.cachedOffset();
+                pc[6].u.operandPointer = slot.cachedOffset();
             }
             structure->startWatchingPropertyForReplacements(vm, slot.cachedOffset());
         }