Commit 2de8b680 authored by Jakob Gruber's avatar Jakob Gruber Committed by Commit Bot

[debug-evaluate] Deserialize builtins before check

Instead of bailing out and assuming everything will be fine if a builtin
hasn't been deserialized yet, deserialize eagerly and perform the full
check.

Change-Id: I60b0d33786a266e124358e2eebe926d8f785881d
Reviewed-on: https://chromium-review.googlesource.com/859998
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: 's avatarYang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50529}
parent 9b2a15b7
...@@ -16,6 +16,7 @@ ...@@ -16,6 +16,7 @@
#include "src/interpreter/bytecode-array-iterator.h" #include "src/interpreter/bytecode-array-iterator.h"
#include "src/interpreter/bytecodes.h" #include "src/interpreter/bytecodes.h"
#include "src/isolate-inl.h" #include "src/isolate-inl.h"
#include "src/snapshot/snapshot.h"
namespace v8 { namespace v8 {
namespace internal { namespace internal {
...@@ -750,16 +751,29 @@ bool DebugEvaluate::FunctionHasNoSideEffect(Handle<SharedFunctionInfo> info) { ...@@ -750,16 +751,29 @@ bool DebugEvaluate::FunctionHasNoSideEffect(Handle<SharedFunctionInfo> info) {
? info->lazy_deserialization_builtin_id() ? info->lazy_deserialization_builtin_id()
: info->code()->builtin_index(); : info->code()->builtin_index();
DCHECK_NE(Builtins::kDeserializeLazy, builtin_index); DCHECK_NE(Builtins::kDeserializeLazy, builtin_index);
if (builtin_index >= 0 && builtin_index < Builtins::builtin_count && if (Builtins::IsBuiltinId(builtin_index) &&
BuiltinHasNoSideEffect(static_cast<Builtins::Name>(builtin_index))) { BuiltinHasNoSideEffect(static_cast<Builtins::Name>(builtin_index))) {
#ifdef DEBUG #ifdef DEBUG
if (info->code()->builtin_index() == Builtins::kDeserializeLazy) { Isolate* isolate = info->GetIsolate();
return true; // Target builtin is not yet deserialized. Code* code = isolate->builtins()->builtin(builtin_index);
if (code->builtin_index() == Builtins::kDeserializeLazy) {
// Target builtin is not yet deserialized. Deserialize it now.
DCHECK(Builtins::IsLazy(builtin_index));
DCHECK_EQ(Builtins::TFJ, Builtins::KindOf(builtin_index));
if (FLAG_trace_lazy_deserialization) {
PrintF("Lazy-deserializing builtin %s\n",
Builtins::name(builtin_index));
}
code = Snapshot::DeserializeBuiltin(isolate, builtin_index);
DCHECK_NE(Builtins::kDeserializeLazy, code->builtin_index());
} }
// TODO(yangguo): Check builtin-to-builtin calls too. // TODO(yangguo): Check builtin-to-builtin calls too.
int mode = RelocInfo::ModeMask(RelocInfo::EXTERNAL_REFERENCE); int mode = RelocInfo::ModeMask(RelocInfo::EXTERNAL_REFERENCE);
bool failed = false; bool failed = false;
for (RelocIterator it(info->code(), mode); !it.done(); it.next()) { for (RelocIterator it(code, mode); !it.done(); it.next()) {
RelocInfo* rinfo = it.rinfo(); RelocInfo* rinfo = it.rinfo();
Address address = rinfo->target_external_reference(); Address address = rinfo->target_external_reference();
const Runtime::Function* function = Runtime::FunctionForEntry(address); const Runtime::Function* function = Runtime::FunctionForEntry(address);
......
...@@ -2147,7 +2147,7 @@ bool Debug::PerformSideEffectCheck(Handle<JSFunction> function) { ...@@ -2147,7 +2147,7 @@ bool Debug::PerformSideEffectCheck(Handle<JSFunction> function) {
return false; return false;
} }
Deoptimizer::DeoptimizeFunction(*function); Deoptimizer::DeoptimizeFunction(*function);
if (!function->shared()->HasNoSideEffect()) { if (!SharedFunctionInfo::HasNoSideEffect(handle(function->shared()))) {
if (FLAG_trace_side_effect_free_debug_evaluate) { if (FLAG_trace_side_effect_free_debug_evaluate) {
PrintF("[debug-evaluate] Function %s failed side effect check.\n", PrintF("[debug-evaluate] Function %s failed side effect check.\n",
function->shared()->DebugName()->ToCString().get()); function->shared()->DebugName()->ToCString().get());
......
...@@ -13654,14 +13654,14 @@ String* SharedFunctionInfo::DebugName() { ...@@ -13654,14 +13654,14 @@ String* SharedFunctionInfo::DebugName() {
return name(); return name();
} }
bool SharedFunctionInfo::HasNoSideEffect() { // static
if (!computed_has_no_side_effect()) { bool SharedFunctionInfo::HasNoSideEffect(Handle<SharedFunctionInfo> info) {
DisallowHeapAllocation not_handlified; if (!info->computed_has_no_side_effect()) {
Handle<SharedFunctionInfo> info(this); bool has_no_side_effect = DebugEvaluate::FunctionHasNoSideEffect(info);
set_has_no_side_effect(DebugEvaluate::FunctionHasNoSideEffect(info)); info->set_has_no_side_effect(has_no_side_effect);
set_computed_has_no_side_effect(true); info->set_computed_has_no_side_effect(true);
} }
return has_no_side_effect(); return info->has_no_side_effect();
} }
// The filter is a pattern that matches function names in this way: // The filter is a pattern that matches function names in this way:
......
...@@ -254,7 +254,7 @@ class SharedFunctionInfo : public HeapObject { ...@@ -254,7 +254,7 @@ class SharedFunctionInfo : public HeapObject {
String* DebugName(); String* DebugName();
// The function cannot cause any side effects. // The function cannot cause any side effects.
bool HasNoSideEffect(); static bool HasNoSideEffect(Handle<SharedFunctionInfo> info);
// Used for flags such as --turbo-filter. // Used for flags such as --turbo-filter.
bool PassesFilter(const char* raw_filter); bool PassesFilter(const char* raw_filter);
......
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