Skip to content

Commit cf951df

Browse files
mi-acCommit bot
mi-ac
authored and
Commit bot
committed
Revert of Correctly annotate eval origin. (patchset v8#4 id:60001 of https://codereview.chromium.org/1854713002/ )
Reason for revert: [Sheriff] Crashes a layout test: https://build.chromium.org/p/client.v8.fyi/builders/V8-Blink%20Linux%2064/builds/5855 Original issue's description: > Correctly annotate eval origin. > > There were a couple of issues with it: > - interpreter is not supported > - the source position was just accidentally correct for full-codegen > - the eval origin could have been cached > > Also fixes a few other places to use AbstractCode. > > R=mstarzinger@chromium.org > > Committed: https://crrev.com/2f3a171adc9e620c2235bf0562145b9d4eaba66d > Cr-Commit-Position: refs/heads/master@{#35257} TBR=mstarzinger@chromium.org,yangguo@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true Review URL: https://codereview.chromium.org/1858773004 Cr-Commit-Position: refs/heads/master@{#35260}
1 parent 4142bc6 commit cf951df

36 files changed

Lines changed: 263 additions & 174 deletions

‎src/accessors.cc‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -661,8 +661,11 @@ void Accessors::ScriptEvalFromScriptPositionGetter(
661661
Script::cast(Handle<JSValue>::cast(object)->value()), isolate);
662662
Handle<Object> result = isolate->factory()->undefined_value();
663663
if (script->compilation_type() == Script::COMPILATION_TYPE_EVAL) {
664-
result =
665-
Handle<Object>(Smi::FromInt(script->eval_from_position()), isolate);
664+
Handle<Code> code(SharedFunctionInfo::cast(
665+
script->eval_from_shared())->code());
666+
result = Handle<Object>(Smi::FromInt(code->SourcePosition(
667+
script->eval_from_instructions_offset())),
668+
isolate);
666669
}
667670
info.GetReturnValue().Set(Utils::ToLocal(result));
668671
}

‎src/api.cc‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2032,7 +2032,6 @@ MaybeLocal<Function> ScriptCompiler::CompileFunctionInContext(
20322032
}
20332033

20342034
i::Handle<i::Object> name_obj;
2035-
int eval_position = 0;
20362035
int line_offset = 0;
20372036
int column_offset = 0;
20382037
if (!source->resource_name.IsEmpty()) {
@@ -2045,12 +2044,11 @@ MaybeLocal<Function> ScriptCompiler::CompileFunctionInContext(
20452044
column_offset = static_cast<int>(source->resource_column_offset->Value());
20462045
}
20472046
i::Handle<i::JSFunction> fun;
2048-
has_pending_exception =
2049-
!i::Compiler::GetFunctionFromEval(
2050-
source_string, outer_info, context, i::SLOPPY,
2051-
i::ONLY_SINGLE_FUNCTION_LITERAL, eval_position, line_offset,
2052-
column_offset - scope_position, name_obj, source->resource_options)
2053-
.ToHandle(&fun);
2047+
has_pending_exception = !i::Compiler::GetFunctionFromEval(
2048+
source_string, outer_info, context, i::SLOPPY,
2049+
i::ONLY_SINGLE_FUNCTION_LITERAL, line_offset,
2050+
column_offset - scope_position, name_obj,
2051+
source->resource_options).ToHandle(&fun);
20542052
if (has_pending_exception) {
20552053
isolate->ReportPendingMessages();
20562054
}

‎src/builtins.cc‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2055,12 +2055,11 @@ MaybeHandle<JSFunction> CompileString(Handle<Context> context,
20552055
}
20562056

20572057
// Compile source string in the native context.
2058-
StackTraceFrameIterator it(isolate);
2059-
FrameSummary summary = FrameSummary::GetFirst(it.frame());
2060-
Handle<SharedFunctionInfo> outer_info(summary.function()->shared());
2061-
int pos = summary.abstract_code()->SourcePosition(summary.code_offset());
2058+
Handle<SharedFunctionInfo> outer_info(native_context->closure()->shared(),
2059+
isolate);
20622060
return Compiler::GetFunctionFromEval(source, outer_info, native_context,
2063-
SLOPPY, restriction, pos);
2061+
SLOPPY, restriction,
2062+
RelocInfo::kNoPosition);
20642063
}
20652064

20662065
} // namespace

‎src/compiler.cc‎

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1509,9 +1509,8 @@ void Compiler::CompileForLiveEdit(Handle<Script> script) {
15091509
MaybeHandle<JSFunction> Compiler::GetFunctionFromEval(
15101510
Handle<String> source, Handle<SharedFunctionInfo> outer_info,
15111511
Handle<Context> context, LanguageMode language_mode,
1512-
ParseRestriction restriction, int eval_position, int line_offset,
1513-
int column_offset, Handle<Object> script_name,
1514-
ScriptOriginOptions options) {
1512+
ParseRestriction restriction, int line_offset, int column_offset,
1513+
Handle<Object> script_name, ScriptOriginOptions options) {
15151514
Isolate* isolate = source->GetIsolate();
15161515
int source_length = source->length();
15171516
isolate->counters()->total_eval_size()->Increment(source_length);
@@ -1520,7 +1519,7 @@ MaybeHandle<JSFunction> Compiler::GetFunctionFromEval(
15201519
CompilationCache* compilation_cache = isolate->compilation_cache();
15211520
MaybeHandle<SharedFunctionInfo> maybe_shared_info =
15221521
compilation_cache->LookupEval(source, outer_info, context, language_mode,
1523-
eval_position);
1522+
line_offset);
15241523
Handle<SharedFunctionInfo> shared_info;
15251524

15261525
Handle<Script> script;
@@ -1532,10 +1531,6 @@ MaybeHandle<JSFunction> Compiler::GetFunctionFromEval(
15321531
script->set_column_offset(column_offset);
15331532
}
15341533
script->set_origin_options(options);
1535-
script->set_compilation_type(Script::COMPILATION_TYPE_EVAL);
1536-
script->set_eval_from_shared(*outer_info);
1537-
script->set_eval_from_position(eval_position);
1538-
15391534
Zone zone(isolate->allocator());
15401535
ParseInfo parse_info(&zone, script);
15411536
CompilationInfo info(&parse_info);
@@ -1545,6 +1540,8 @@ MaybeHandle<JSFunction> Compiler::GetFunctionFromEval(
15451540
parse_info.set_parse_restriction(restriction);
15461541
parse_info.set_context(context);
15471542

1543+
Debug::RecordEvalCaller(script);
1544+
15481545
shared_info = CompileToplevel(&info);
15491546

15501547
if (shared_info.is_null()) {
@@ -1560,7 +1557,7 @@ MaybeHandle<JSFunction> Compiler::GetFunctionFromEval(
15601557
DCHECK(is_sloppy(language_mode) ||
15611558
is_strict(shared_info->language_mode()));
15621559
compilation_cache->PutEval(source, outer_info, context, shared_info,
1563-
eval_position);
1560+
line_offset);
15641561
}
15651562
}
15661563

‎src/compiler.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,8 +77,8 @@ class Compiler : public AllStatic {
7777
MUST_USE_RESULT static MaybeHandle<JSFunction> GetFunctionFromEval(
7878
Handle<String> source, Handle<SharedFunctionInfo> outer_info,
7979
Handle<Context> context, LanguageMode language_mode,
80-
ParseRestriction restriction, int eval_position, int line_offset = 0,
81-
int column_offset = 0, Handle<Object> script_name = Handle<Object>(),
80+
ParseRestriction restriction, int line_offset, int column_offset = 0,
81+
Handle<Object> script_name = Handle<Object>(),
8282
ScriptOriginOptions options = ScriptOriginOptions());
8383

8484
// Create a shared function info object for a String source within a context.

‎src/compiler/ast-graph-builder.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2456,7 +2456,7 @@ void AstGraphBuilder::VisitCall(Call* expr) {
24562456
// provide a fully resolved callee to patch into the environment.
24572457
Node* function = GetFunctionClosure();
24582458
Node* language = jsgraph()->Constant(language_mode());
2459-
Node* position = jsgraph()->Constant(expr->position());
2459+
Node* position = jsgraph()->Constant(current_scope()->start_position());
24602460
const Operator* op =
24612461
javascript()->CallRuntime(Runtime::kResolvePossiblyDirectEval);
24622462
Node* new_callee =

‎src/debug/debug-evaluate.cc‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -95,11 +95,11 @@ MaybeHandle<Object> DebugEvaluate::Evaluate(
9595
}
9696

9797
Handle<JSFunction> eval_fun;
98-
ASSIGN_RETURN_ON_EXCEPTION(
99-
isolate, eval_fun,
100-
Compiler::GetFunctionFromEval(source, outer_info, context, SLOPPY,
101-
NO_PARSE_RESTRICTION, 0),
102-
Object);
98+
ASSIGN_RETURN_ON_EXCEPTION(isolate, eval_fun,
99+
Compiler::GetFunctionFromEval(
100+
source, outer_info, context, SLOPPY,
101+
NO_PARSE_RESTRICTION, RelocInfo::kNoPosition),
102+
Object);
103103

104104
Handle<Object> result;
105105
ASSIGN_RETURN_ON_EXCEPTION(

‎src/debug/debug-frames.cc‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,8 @@ int FrameInspector::GetSourcePosition() {
7272
return deoptimized_frame_->GetSourcePosition();
7373
} else if (is_interpreted_) {
7474
InterpretedFrame* frame = reinterpret_cast<InterpretedFrame*>(frame_);
75-
BytecodeArray* bytecode_array = frame->GetBytecodeArray();
75+
BytecodeArray* bytecode_array =
76+
frame->function()->shared()->bytecode_array();
7677
return bytecode_array->SourcePosition(frame->GetBytecodeOffset());
7778
} else {
7879
Code* code = frame_->LookupCode();

‎src/debug/debug.cc‎

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,12 @@ BreakLocation BreakLocation::FromCodeOffset(Handle<DebugInfo> debug_info,
260260
return it->GetBreakLocation();
261261
}
262262

263+
FrameSummary GetFirstFrameSummary(JavaScriptFrame* frame) {
264+
List<FrameSummary> frames(FLAG_max_inlining_levels + 1);
265+
frame->Summarize(&frames);
266+
return frames.first();
267+
}
268+
263269
int CallOffsetFromCodeOffset(int code_offset, bool is_interpreted) {
264270
// Code offset points to the instruction after the call. Subtract 1 to
265271
// exclude that instruction from the search. For bytecode, the code offset
@@ -269,7 +275,7 @@ int CallOffsetFromCodeOffset(int code_offset, bool is_interpreted) {
269275

270276
BreakLocation BreakLocation::FromFrame(Handle<DebugInfo> debug_info,
271277
JavaScriptFrame* frame) {
272-
FrameSummary summary = FrameSummary::GetFirst(frame);
278+
FrameSummary summary = GetFirstFrameSummary(frame);
273279
int call_offset =
274280
CallOffsetFromCodeOffset(summary.code_offset(), frame->is_interpreted());
275281
return FromCodeOffset(debug_info, call_offset);
@@ -625,7 +631,7 @@ void Debug::Break(JavaScriptFrame* frame) {
625631
step_break = location.IsTailCall();
626632
// Fall through.
627633
case StepIn: {
628-
FrameSummary summary = FrameSummary::GetFirst(frame);
634+
FrameSummary summary = GetFirstFrameSummary(frame);
629635
int offset = summary.code_offset();
630636
step_break = step_break || location.IsReturn() ||
631637
(current_fp != last_fp) ||
@@ -1005,7 +1011,7 @@ void Debug::PrepareStep(StepAction step_action) {
10051011
}
10061012

10071013
// Get the debug info (create it if it does not exist).
1008-
FrameSummary summary = FrameSummary::GetFirst(frame);
1014+
FrameSummary summary = GetFirstFrameSummary(frame);
10091015
Handle<JSFunction> function(summary.function());
10101016
Handle<SharedFunctionInfo> shared(function->shared());
10111017
if (!EnsureDebugInfo(shared, function)) {
@@ -1016,7 +1022,7 @@ void Debug::PrepareStep(StepAction step_action) {
10161022
Handle<DebugInfo> debug_info(shared->GetDebugInfo());
10171023
// Refresh frame summary if the code has been recompiled for debugging.
10181024
if (AbstractCode::cast(shared->code()) != *summary.abstract_code()) {
1019-
summary = FrameSummary::GetFirst(frame);
1025+
summary = GetFirstFrameSummary(frame);
10201026
}
10211027

10221028
int call_offset =
@@ -1598,7 +1604,7 @@ bool Debug::IsBreakAtReturn(JavaScriptFrame* frame) {
15981604
if (!shared->HasDebugInfo()) return false;
15991605

16001606
DCHECK(!frame->is_optimized());
1601-
FrameSummary summary = FrameSummary::GetFirst(frame);
1607+
FrameSummary summary = GetFirstFrameSummary(frame);
16021608

16031609
Handle<DebugInfo> debug_info(shared->GetDebugInfo());
16041610
BreakLocation location =
@@ -1650,6 +1656,21 @@ Handle<FixedArray> Debug::GetLoadedScripts() {
16501656
}
16511657

16521658

1659+
void Debug::RecordEvalCaller(Handle<Script> script) {
1660+
script->set_compilation_type(Script::COMPILATION_TYPE_EVAL);
1661+
// For eval scripts add information on the function from which eval was
1662+
// called.
1663+
StackTraceFrameIterator it(script->GetIsolate());
1664+
if (!it.done()) {
1665+
script->set_eval_from_shared(it.frame()->function()->shared());
1666+
Code* code = it.frame()->LookupCode();
1667+
int offset = static_cast<int>(
1668+
it.frame()->pc() - code->instruction_start());
1669+
script->set_eval_from_instructions_offset(offset);
1670+
}
1671+
}
1672+
1673+
16531674
MaybeHandle<Object> Debug::MakeExecutionState() {
16541675
// Create the execution state object.
16551676
Handle<Object> argv[] = { isolate_->factory()->NewNumberFromInt(break_id()) };
@@ -2266,7 +2287,7 @@ void Debug::PrintBreakLocation() {
22662287
JavaScriptFrameIterator iterator(isolate_);
22672288
if (iterator.done()) return;
22682289
JavaScriptFrame* frame = iterator.frame();
2269-
FrameSummary summary = FrameSummary::GetFirst(frame);
2290+
FrameSummary summary = GetFirstFrameSummary(frame);
22702291
int source_position =
22712292
summary.abstract_code()->SourcePosition(summary.code_offset());
22722293
Handle<Object> script_obj(summary.function()->shared()->script(), isolate_);

‎src/debug/debug.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -499,6 +499,9 @@ class Debug {
499499
static int ArchiveSpacePerThread();
500500
void FreeThreadResources() { }
501501

502+
// Record function from which eval was called.
503+
static void RecordEvalCaller(Handle<Script> script);
504+
502505
bool CheckExecutionState(int id) {
503506
return is_active() && !debug_context().is_null() && break_id() != 0 &&
504507
break_id() == id;

0 commit comments

Comments
 (0)