Commit 09e921d4 authored by rmcilroy's avatar rmcilroy Committed by Commit bot

[Interpreter] Avoid dereferencing handles on BytecodeGenerator for AST operations.

Updates a number of AST operations to avoid dereferencing handles
such that they can safely be called off-thread. Also adds a
HandleDereferenceMode argument to some operations where handles are
compared. If handle dereferencing is allowed, the handles are compared
directly, if not then their locations are compared (which relies on the
handles being created in a CanonicalHandleScope).

BUG=v8:5203
TBR=adamk@chromium.org

Review-Url: https://codereview.chromium.org/2223523002
Cr-Commit-Position: refs/heads/master@{#38526}
parent 9e14155d
......@@ -2170,6 +2170,11 @@ MaybeLocal<Script> ScriptCompiler::Compile(Local<Context> context,
source->info->set_script(script);
source->info->set_context(isolate->native_context());
// Create a canonical handle scope before internalizing parsed values if
// compiling bytecode. This is required for off-thread bytecode generation.
std::unique_ptr<i::CanonicalHandleScope> canonical;
if (i::FLAG_ignition) canonical.reset(new i::CanonicalHandleScope(isolate));
// Do the parsing tasks which need to be done on the main thread. This will
// also handle parse errors.
source->parser->Internalize(isolate, script,
......
......@@ -106,9 +106,9 @@ void AstRawString::Internalize(Isolate* isolate) {
}
}
bool AstRawString::AsArrayIndex(uint32_t* index) const {
if (!string_.is_null())
bool AstRawString::AsArrayIndex(uint32_t* index,
HandleDereferenceMode deref_mode) const {
if (deref_mode == HandleDereferenceMode::kAllowed && !string_.is_null())
return string_->AsArrayIndex(index);
if (!is_one_byte() || literal_bytes_.length() == 0 ||
literal_bytes_.length() > String::kMaxArrayIndexSize)
......@@ -136,11 +136,10 @@ void AstConsString::Internalize(Isolate* isolate) {
.ToHandleChecked();
}
bool AstValue::IsPropertyName() const {
bool AstValue::IsPropertyName(HandleDereferenceMode deref_mode) const {
if (type_ == STRING) {
uint32_t index;
return !string_->AsArrayIndex(&index);
return !string_->AsArrayIndex(&index, deref_mode);
}
return false;
}
......@@ -276,6 +275,7 @@ void AstValueFactory::Internalize(Isolate* isolate) {
// Everything is already internalized.
return;
}
// Strings need to be internalized before values, because values refer to
// strings.
for (int i = 0; i < strings_.length(); ++i) {
......
......@@ -80,7 +80,8 @@ class AstRawString final : public AstString {
void Internalize(Isolate* isolate);
bool AsArrayIndex(uint32_t* index) const;
bool AsArrayIndex(uint32_t* index, HandleDereferenceMode deref_mode =
HandleDereferenceMode::kAllowed) const;
// The string is not null-terminated, use length() to find out the length.
const unsigned char* raw_data() const {
......@@ -180,7 +181,8 @@ class AstValue : public ZoneObject {
return type_ == STRING && string_ == string;
}
bool IsPropertyName() const;
bool IsPropertyName(
HandleDereferenceMode deref_mode = HandleDereferenceMode::kAllowed) const;
bool BooleanValue() const;
......
......@@ -74,22 +74,18 @@ bool Expression::IsStringLiteral() const {
return IsLiteral() && AsLiteral()->value()->IsString();
}
bool Expression::IsPropertyName() const {
return IsLiteral() && AsLiteral()->IsPropertyName();
bool Expression::IsPropertyName(HandleDereferenceMode deref_mode) const {
return IsLiteral() && AsLiteral()->IsPropertyName(deref_mode);
}
bool Expression::IsNullLiteral() const {
if (!IsLiteral()) return false;
Handle<Object> value = AsLiteral()->value();
return !value->IsSmi() &&
value->IsNull(HeapObject::cast(*value)->GetIsolate());
return AsLiteral()->raw_value()->IsNull();
}
bool Expression::IsUndefinedLiteral() const {
if (IsLiteral()) {
Handle<Object> value = AsLiteral()->value();
if (!value->IsSmi() &&
value->IsUndefined(HeapObject::cast(*value)->GetIsolate())) {
if (AsLiteral()->raw_value()->IsUndefined()) {
return true;
}
}
......@@ -892,19 +888,20 @@ bool Expression::IsMonomorphic() const {
}
}
bool Call::IsUsingCallFeedbackICSlot(Isolate* isolate) const {
CallType call_type = GetCallType(isolate);
bool Call::IsUsingCallFeedbackICSlot(
Isolate* isolate, HandleDereferenceMode dereference_mode) const {
CallType call_type = GetCallType(isolate, dereference_mode);
if (call_type == POSSIBLY_EVAL_CALL) {
return false;
}
return true;
}
bool Call::IsUsingCallFeedbackSlot(Isolate* isolate) const {
bool Call::IsUsingCallFeedbackSlot(
Isolate* isolate, HandleDereferenceMode dereference_mode) const {
// SuperConstructorCall uses a CallConstructStub, which wants
// a Slot, in addition to any IC slots requested elsewhere.
return GetCallType(isolate) == SUPER_CALL;
return GetCallType(isolate, dereference_mode) == SUPER_CALL;
}
......@@ -918,11 +915,11 @@ void Call::AssignFeedbackVectorSlots(Isolate* isolate, FeedbackVectorSpec* spec,
}
}
Call::CallType Call::GetCallType(Isolate* isolate) const {
Call::CallType Call::GetCallType(Isolate* isolate,
HandleDereferenceMode deref_mode) const {
VariableProxy* proxy = expression()->AsVariableProxy();
if (proxy != NULL) {
if (proxy->var()->is_possibly_eval(isolate)) {
if (proxy->var()->is_possibly_eval(isolate, deref_mode)) {
return POSSIBLY_EVAL_CALL;
} else if (proxy->var()->IsUnallocatedOrGlobalSlot()) {
return GLOBAL_CALL;
......@@ -936,7 +933,7 @@ Call::CallType Call::GetCallType(Isolate* isolate) const {
Property* property = expression()->AsProperty();
if (property != nullptr) {
bool is_super = property->IsSuperAccess();
if (property->key()->IsPropertyName()) {
if (property->key()->IsPropertyName(deref_mode)) {
return is_super ? NAMED_SUPER_PROPERTY_CALL : NAMED_PROPERTY_CALL;
} else {
return is_super ? KEYED_SUPER_PROPERTY_CALL : KEYED_PROPERTY_CALL;
......@@ -969,6 +966,5 @@ bool Literal::Match(void* literal1, void* literal2) {
(x->IsNumber() && y->IsNumber() && x->AsNumber() == y->AsNumber());
}
} // namespace internal
} // namespace v8
......@@ -314,7 +314,10 @@ class Expression : public AstNode {
// Symbols that cannot be parsed as array indices are considered property
// names. We do not treat symbols that can be array indexes as property
// names because [] for string objects is handled only by keyed ICs.
bool IsPropertyName() const;
// Produces the same result whether |deref_mode| is kAllowed or kDisallowed,
// although may be faster with kAllowed.
bool IsPropertyName(
HandleDereferenceMode deref_mode = HandleDereferenceMode::kAllowed) const;
// True iff the expression is a class or function expression without
// a syntactic name.
......@@ -1192,10 +1195,16 @@ class SloppyBlockFunctionStatement final : public Statement {
class Literal final : public Expression {
public:
bool IsPropertyName() const { return value_->IsPropertyName(); }
// Returns true if literal represents a property name (i.e. cannot be parsed
// as array indices). Produces the same result whether |deref_mode| is
// kAllowed or kDisallowed, although may be faster with kAllowed.
bool IsPropertyName(HandleDereferenceMode deref_mode =
HandleDereferenceMode::kAllowed) const {
return value_->IsPropertyName(deref_mode);
}
Handle<String> AsPropertyName() {
DCHECK(IsPropertyName());
DCHECK(IsPropertyName(HandleDereferenceMode::kDisallowed));
return Handle<String>::cast(value());
}
......@@ -1204,8 +1213,8 @@ class Literal final : public Expression {
return value_->AsString();
}
bool ToBooleanIsTrue() const { return value()->BooleanValue(); }
bool ToBooleanIsFalse() const { return !value()->BooleanValue(); }
bool ToBooleanIsTrue() const { return raw_value()->BooleanValue(); }
bool ToBooleanIsFalse() const { return !raw_value()->BooleanValue(); }
Handle<Object> value() const { return value_->value(); }
const AstValue* raw_value() const { return value_; }
......@@ -1360,7 +1369,7 @@ class ObjectLiteral final : public MaterializedLiteral {
Handle<FixedArray> constant_properties() const {
return constant_properties_;
}
int properties_count() const { return constant_properties_->length() / 2; }
int properties_count() const { return boilerplate_properties_; }
ZoneList<Property*>* properties() const { return properties_; }
bool fast_elements() const { return fast_elements_; }
bool may_store_doubles() const { return may_store_doubles_; }
......@@ -1735,10 +1744,15 @@ class Property final : public Expression {
return property_feedback_slot_;
}
static LhsKind GetAssignType(Property* property) {
// Returns the properties assign type. Produces the same result whether
// |deref_mode| is kAllowed or kDisallowed, although may be faster with
// kAllowed.
static LhsKind GetAssignType(
Property* property,
HandleDereferenceMode deref_mode = HandleDereferenceMode::kAllowed) {
if (property == NULL) return VARIABLE;
bool super_access = property->IsSuperAccess();
return (property->key()->IsPropertyName())
return (property->key()->IsPropertyName(deref_mode))
? (super_access ? NAMED_SUPER_PROPERTY : NAMED_PROPERTY)
: (super_access ? KEYED_SUPER_PROPERTY : KEYED_PROPERTY);
}
......@@ -1853,9 +1867,17 @@ class Call final : public Expression {
};
// Helpers to determine how to handle the call.
CallType GetCallType(Isolate* isolate) const;
bool IsUsingCallFeedbackSlot(Isolate* isolate) const;
bool IsUsingCallFeedbackICSlot(Isolate* isolate) const;
// If called with |deref_mode| of kDisallowed, then the AST nodes must have
// been internalized within a CanonicalHandleScope.
CallType GetCallType(
Isolate* isolate,
HandleDereferenceMode deref_mode = HandleDereferenceMode::kAllowed) const;
bool IsUsingCallFeedbackSlot(
Isolate* isolate,
HandleDereferenceMode deref_mode = HandleDereferenceMode::kAllowed) const;
bool IsUsingCallFeedbackICSlot(
Isolate* isolate,
HandleDereferenceMode deref_mode = HandleDereferenceMode::kAllowed) const;
#ifdef DEBUG
// Used to assert that the FullCodeGenerator records the return site.
......
......@@ -55,10 +55,6 @@ class Variable: public ZoneObject {
int initializer_position() { return initializer_position_; }
void set_initializer_position(int pos) { initializer_position_ = pos; }
bool IsVariable(Handle<String> n) const {
return !is_this() && name().is_identical_to(n);
}
bool IsUnallocated() const {
return location_ == VariableLocation::UNALLOCATED;
}
......@@ -90,13 +86,25 @@ class Variable: public ZoneObject {
// any variable named "this" does indeed refer to a Variable::THIS binding;
// the grammar ensures this to be the case. So wherever a "this" binding
// might be provided by the global, use HasThisName instead of is_this().
bool HasThisName(Isolate* isolate) const {
return is_this() || *name() == *isolate->factory()->this_string();
bool HasThisName(Isolate* isolate,
HandleDereferenceMode deref_mode =
HandleDereferenceMode::kAllowed) const {
// Note: it is safe to dereference isolate->factory()->this_string() here
// regardless of |deref_mode| because it is a constant root and so will
// never be updated or moved.
return is_this() ||
name_is_identical_to(isolate->factory()->this_string(), deref_mode);
}
// True if the variable is named eval and not known to be shadowed.
bool is_possibly_eval(Isolate* isolate) const {
return IsVariable(isolate->factory()->eval_string());
bool is_possibly_eval(Isolate* isolate,
HandleDereferenceMode deref_mode =
HandleDereferenceMode::kAllowed) const {
// Note: it is safe to dereference isolate->factory()->eval_string() here
// regardless of |deref_mode| because it is a constant root and so will
// never be updated or moved.
return !is_this() &&
name_is_identical_to(isolate->factory()->eval_string(), deref_mode);
}
Variable* local_if_not_shadowed() const {
......@@ -122,6 +130,20 @@ class Variable: public ZoneObject {
static int CompareIndex(Variable* const* v, Variable* const* w);
private:
bool name_is_identical_to(Handle<Object> object,
HandleDereferenceMode deref_mode) const {
if (deref_mode == HandleDereferenceMode::kAllowed) {
return *name() == *object;
} else {
// If handle dereference isn't allowed use the handle address for
// identity. This depends on the variable name being internalized in a
// CanonicalHandleScope, so that all handles created during the
// internalization with identical values have identical locations, and any
// handles created which point to roots have the root handle's location.
return name().address() == object.address();
}
}
Scope* scope_;
const AstRawString* name_;
VariableMode mode_;
......
......@@ -170,6 +170,12 @@ void CompilerDispatcherJob::InternalizeParsingResult() {
status() == CompileJobStatus::kFailed);
HandleScope scope(isolate_);
// Create a canonical handle scope before internalizing parsed values if
// compiling bytecode. This is required for off-thread bytecode generation.
std::unique_ptr<CanonicalHandleScope> canonical;
if (FLAG_ignition) canonical.reset(new CanonicalHandleScope(isolate_));
Handle<SharedFunctionInfo> shared(function_->shared(), isolate_);
Handle<Script> script(Script::cast(shared->script()), isolate_);
......
......@@ -540,9 +540,8 @@ MUST_USE_RESULT MaybeHandle<Code> GetUnoptimizedCode(CompilationInfo* info) {
VMState<COMPILER> state(info->isolate());
PostponeInterruptsScope postpone(info->isolate());
// Create a canonical handle scope if compiling ignition bytecode. This is
// required by the constant array builder to de-duplicate common objects
// without dereferencing handles.
// Create a canonical handle scope before internalizing parsed values if
// compiling bytecode. This is required for off-thread bytecode generation.
std::unique_ptr<CanonicalHandleScope> canonical;
if (FLAG_ignition) canonical.reset(new CanonicalHandleScope(info->isolate()));
......@@ -1077,9 +1076,8 @@ Handle<SharedFunctionInfo> CompileToplevel(CompilationInfo* info) {
ParseInfo* parse_info = info->parse_info();
Handle<Script> script = parse_info->script();
// Create a canonical handle scope if compiling ignition bytecode. This is
// required by the constant array builder to de-duplicate common objects
// without dereferencing handles.
// Create a canonical handle scope before internalizing parsed values if
// compiling bytecode. This is required for off-thread bytecode generation.
std::unique_ptr<CanonicalHandleScope> canonical;
if (FLAG_ignition) canonical.reset(new CanonicalHandleScope(isolate));
......
......@@ -816,6 +816,8 @@ enum class CreateArgumentsType : uint8_t {
kRestParameter
};
enum class HandleDereferenceMode { kAllowed, kDisallowed };
inline size_t hash_value(CreateArgumentsType type) {
return bit_cast<uint8_t>(type);
}
......
......@@ -254,6 +254,8 @@ void BytecodeArrayWriter::PatchJumpWith8BitOperand(size_t jump_location,
constant_array_builder()->DiscardReservedEntry(OperandSize::kByte);
bytecodes()->at(operand_location) = static_cast<uint8_t>(delta);
} else {
// TODO(5203): Remove this temporary exception.
AllowHandleAllocation allow_handles;
// The jump does not fit within the range of an Imm operand, so
// commit reservation putting the offset into the constant pool,
// and update the jump instruction and operand.
......@@ -278,6 +280,8 @@ void BytecodeArrayWriter::PatchJumpWith16BitOperand(size_t jump_location,
constant_array_builder()->DiscardReservedEntry(OperandSize::kShort);
WriteUnalignedUInt16(operand_bytes, static_cast<uint16_t>(delta));
} else {
// TODO(5203): Remove this temporary exception.
AllowHandleAllocation allow_handles;
jump_bytecode = GetJumpWithConstantOperand(jump_bytecode);
bytecodes()->at(jump_location) = Bytecodes::ToByte(jump_bytecode);
size_t entry = constant_array_builder()->CommitReservedEntry(
......
......@@ -667,6 +667,10 @@ void BytecodeGenerator::FinalizeBytecode() {
}
void BytecodeGenerator::GenerateBytecode() {
DisallowHeapAllocation no_allocation;
DisallowHandleAllocation no_handles;
DisallowHandleDereference no_deref;
// Initialize the incoming context.
ContextScope incoming_context(this, scope(), false);
......@@ -1162,7 +1166,8 @@ void BytecodeGenerator::VisitForInAssignment(Expression* expr,
// Evaluate assignment starting with the value to be stored in the
// accumulator.
Property* property = expr->AsProperty();
LhsKind assign_type = Property::GetAssignType(property);
LhsKind assign_type =
Property::GetAssignType(property, HandleDereferenceMode::kDisallowed);
switch (assign_type) {
case VARIABLE: {
Variable* variable = expr->AsVariableProxy()->var();
......@@ -1639,6 +1644,10 @@ void BytecodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) {
case ObjectLiteral::Property::COMPUTED: {
// It is safe to use [[Put]] here because the boilerplate already
// contains computed properties with an uninitialized value.
// TODO(5203): Remove this temporary exception.
AllowHandleDereference allow_deref;
if (literal_key->value()->IsInternalizedString()) {
if (property->emit_store()) {
VisitForAccumulatorValue(property->value());
......@@ -2149,7 +2158,8 @@ void BytecodeGenerator::VisitAssignment(Assignment* expr) {
// Left-hand side can only be a property, a global or a variable slot.
Property* property = expr->target()->AsProperty();
LhsKind assign_type = Property::GetAssignType(property);
LhsKind assign_type =
Property::GetAssignType(property, HandleDereferenceMode::kDisallowed);
// Evaluate LHS expression.
switch (assign_type) {
......@@ -2379,7 +2389,8 @@ void BytecodeGenerator::VisitThrow(Throw* expr) {
}
void BytecodeGenerator::VisitPropertyLoad(Register obj, Property* expr) {
LhsKind property_kind = Property::GetAssignType(expr);
LhsKind property_kind =
Property::GetAssignType(expr, HandleDereferenceMode::kDisallowed);
FeedbackVectorSlot slot = expr->PropertyFeedbackSlot();
builder()->SetExpressionPosition(expr);
switch (property_kind) {
......@@ -2457,7 +2468,8 @@ void BytecodeGenerator::VisitKeyedSuperPropertyLoad(Property* property,
}
void BytecodeGenerator::VisitProperty(Property* expr) {
LhsKind property_kind = Property::GetAssignType(expr);
LhsKind property_kind =
Property::GetAssignType(expr, HandleDereferenceMode::kDisallowed);
if (property_kind != NAMED_SUPER_PROPERTY &&
property_kind != KEYED_SUPER_PROPERTY) {
Register obj = VisitForRegisterValue(expr->obj());
......@@ -2501,7 +2513,8 @@ Register BytecodeGenerator::VisitArguments(ZoneList<Expression*>* args) {
void BytecodeGenerator::VisitCall(Call* expr) {
Expression* callee_expr = expr->expression();
Call::CallType call_type = expr->GetCallType(isolate());
Call::CallType call_type =
expr->GetCallType(isolate(), HandleDereferenceMode::kDisallowed);
if (call_type == Call::SUPER_CALL) {
return VisitCallSuper(expr);
......@@ -2766,7 +2779,9 @@ void BytecodeGenerator::VisitDelete(UnaryOperation* expr) {
// not allowed in strict mode. Deleting 'this' is allowed in both modes.
VariableProxy* proxy = expr->expression()->AsVariableProxy();
Variable* variable = proxy->var();
DCHECK(is_sloppy(language_mode()) || variable->HasThisName(isolate()));
DCHECK(
is_sloppy(language_mode()) ||
variable->HasThisName(isolate(), HandleDereferenceMode::kDisallowed));
switch (variable->location()) {
case VariableLocation::GLOBAL:
case VariableLocation::UNALLOCATED: {
......@@ -2788,7 +2803,8 @@ void BytecodeGenerator::VisitDelete(UnaryOperation* expr) {
case VariableLocation::CONTEXT: {
// Deleting local var/let/const, context variables, and arguments
// does not have any effect.
if (variable->HasThisName(isolate())) {
if (variable->HasThisName(isolate(),
HandleDereferenceMode::kDisallowed)) {
builder()->LoadTrue();
} else {
builder()->LoadFalse();
......@@ -2819,7 +2835,8 @@ void BytecodeGenerator::VisitCountOperation(CountOperation* expr) {
// Left-hand side can only be a property, a global or a variable slot.
Property* property = expr->expression()->AsProperty();
LhsKind assign_type = Property::GetAssignType(property);
LhsKind assign_type =
Property::GetAssignType(property, HandleDereferenceMode::kDisallowed);
bool is_postfix = expr->is_postfix() && !execution_result()->IsEffect();
......@@ -3036,6 +3053,18 @@ void BytecodeGenerator::VisitRewritableExpression(RewritableExpression* expr) {
Visit(expr->expression());
}
namespace {
Handle<ScopeInfo> GetScopeInfo(Scope* scope, Isolate* isolate) {
// TODO(5203): Remove this temporary exception.
AllowHeapAllocation allow_allocation;
AllowHandleAllocation allow_handles;
AllowHandleDereference allow_deref;
return scope->GetScopeInfo(isolate);
}
} // namespace
void BytecodeGenerator::VisitNewLocalFunctionContext() {
AccumulatorResultScope accumulator_execution_result(this);
Scope* scope = this->scope();
......@@ -3049,7 +3078,7 @@ void BytecodeGenerator::VisitNewLocalFunctionContext() {
builder()
->LoadAccumulatorWithRegister(Register::function_closure())
.StoreAccumulatorInRegister(closure)
.LoadLiteral(scope->GetScopeInfo(isolate()))
.LoadLiteral(GetScopeInfo(scope, isolate()))
.StoreAccumulatorInRegister(scope_info)
.CallRuntime(Runtime::kNewScriptContext, closure, 2);
} else {
......@@ -3097,7 +3126,7 @@ void BytecodeGenerator::VisitNewLocalBlockContext(Scope* scope) {
Register closure = register_allocator()->NextConsecutiveRegister();
builder()
->LoadLiteral(scope->GetScopeInfo(isolate()))
->LoadLiteral(GetScopeInfo(scope, isolate()))
.StoreAccumulatorInRegister(scope_info);
VisitFunctionClosureForContext();
builder()
......
......@@ -294,6 +294,8 @@ void BytecodePeepholeOptimizer::TransformToStarIfLoadingNameConstantAction(
DCHECK_EQ(last()->bytecode(), Bytecode::kLdaConstant);
DCHECK(!Bytecodes::IsJump(node->bytecode()));
// TODO(5203): Remove this temporary exception.
AllowHandleDereference allow_deref;
if (GetConstantForIndexOperand(last(), 0)->IsName()) {
node->replace_bytecode(Bytecode::kStar);
}
......
......@@ -23959,6 +23959,24 @@ TEST(StreamingScriptConstantArray) {
RunStreamingTest(chunks);
}
TEST(StreamingScriptEvalShadowing) {
// When run with Ignition, tests that the streaming parser canonicalizes
// handles so the Variable::is_possibly_eval() is correct.
const char* chunk1 =
"(function() {\n"
" var y = 2;\n"
" return (function() {\n"
" eval('var y = 13;');\n"
" function g() {\n"
" return y\n"
" }\n"
" return g();\n"
" })()\n"
"})()\n";
const char* chunks[] = {chunk1, NULL};
RunStreamingTest(chunks);
}
TEST(StreamingBiggerScript) {
const char* chunk1 =
"function foo() {\n"
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment