Skip to content

Commit

Permalink
Merged r232816 - [LLInt] use loadp consistently for get_from_scope/pu…
Browse files Browse the repository at this point in the history 
…t_to_scope

https://bugs.webkit.org/show_bug.cgi?id=132333

Patch by Caitlin Potter <caitp@igalia.com> on 2018-06-13
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):
  • Loading branch information
@caitp @aperezdc
caitp authored and aperezdc committed Jun 14, 2018
1 parent f6b64e3 commit ddcb399
Show file tree
Hide file tree
Showing 8 changed files with 49 additions and 21 deletions.
27 changes: 27 additions & 0 deletions Source/JavaScriptCore/ChangeLog
View 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-04-14 Filip Pizlo <fpizlo@apple.com>

Function.prototype.caller shouldn't return generator bodies
Expand Down
4 changes: 2 additions & 2 deletions Source/JavaScriptCore/bytecode/CodeBlock.cpp
View file
Expand Up @@ -678,7 +678,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;
}

Expand Down Expand Up @@ -715,7 +715,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;
}
Expand Down
1 change: 1 addition & 0 deletions Source/JavaScriptCore/bytecode/Instruction.h
View file
Expand Up @@ -122,6 +122,7 @@ struct Instruction {
Opcode opcode;
int operand;
unsigned unsignedValue;
intptr_t operandPointer;
WriteBarrierBase<Structure> structure;
StructureID structureID;
WriteBarrierBase<SymbolTable> symbolTable;
Expand Down
2 changes: 1 addition & 1 deletion Source/JavaScriptCore/jit/JITOperations.cpp
View file
Expand Up @@ -2310,7 +2310,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;
Expand Down
2 changes: 1 addition & 1 deletion Source/JavaScriptCore/llint/LLIntSlowPaths.cpp
View file
Expand Up @@ -1722,7 +1722,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
Expand Down
10 changes: 5 additions & 5 deletions Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm
View file
Expand Up @@ -2303,7 +2303,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)
Expand All @@ -2323,7 +2323,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)
Expand Down Expand Up @@ -2398,7 +2398,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

Expand All @@ -2415,7 +2415,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
Expand All @@ -2427,7 +2427,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
Expand Down
14 changes: 7 additions & 7 deletions Source/JavaScriptCore/llint/LowLevelInterpreter64.asm
View file
Expand Up @@ -1448,7 +1448,7 @@ _llint_op_put_by_id:
bineq t1, JSCell::m_structureID[t3], .opPutByIdSlow

.opPutByIdDoneCheckingTypes:
loadisFromInstruction(6, t1)
loadpFromInstruction(6, t1)
btiz t1, .opPutByIdNotTransition

Expand Down Expand Up @@ -1478,7 +1478,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]
Expand Down Expand Up @@ -2289,7 +2289,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)
Expand All @@ -2306,7 +2306,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)
Expand Down Expand Up @@ -2379,7 +2379,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

Expand All @@ -2395,7 +2395,7 @@ end
macro putClosureVar()
loadisFromInstruction(3, t1)
loadConstantOrVariable(t1, t2)
loadisFromInstruction(6, t1)
loadpFromInstruction(6, t1)
storeq t2, JSLexicalEnvironment_variables[t0, t1, 8]
end

Expand All @@ -2406,7 +2406,7 @@ macro putLocalClosureVar()
btpz t3, .noVariableWatchpointSet
notifyWrite(t3, .pDynamic)
.noVariableWatchpointSet:
loadisFromInstruction(6, t1)
loadpFromInstruction(6, t1)
storeq t2, JSLexicalEnvironment_variables[t0, t1, 8]
end

Expand Down
10 changes: 5 additions & 5 deletions Source/JavaScriptCore/runtime/CommonSlowPaths.h
View file
Expand Up @@ -135,7 +135,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();
}
}

Expand All @@ -158,8 +158,8 @@ inline void tryCachePutToScopeGlobal(
scope->structure()->didCachePropertyReplacement(vm, slot.cachedOffset());

ConcurrentJSLocker locker(codeBlock->m_lock);
pc[5].u.structure.set(vm, codeBlock, scope->structure());
pc[6].u.operand = slot.cachedOffset();
pc[5].u.structure.set(vm, codeBlock, scope->structure(vm));
pc[6].u.operandPointer = slot.cachedOffset();
}
}

Expand All @@ -183,7 +183,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();
}
}

Expand All @@ -197,7 +197,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());
}
Expand Down

0 comments on commit ddcb399

Please sign in to comment.