From 54e81c351f2d33a625c10782b7e378a7f4be93ba Mon Sep 17 00:00:00 2001 From: "sgjesse@chromium.org" Date: Mon, 11 Jul 2011 15:20:17 +0000 Subject: [PATCH] Add source position recording for variable loads This provides more precise source to generated code mapping as variable loads can be handled using IC calls. R=kmillikin@chromium.org BUG=v8:1527 TEST=test/message/regress/regress-1527 Review URL: http://codereview.chromium.org//7327038 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@8610 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/arm/full-codegen-arm.cc | 14 +++++++++----- src/full-codegen.h | 2 +- src/ia32/full-codegen-ia32.cc | 16 ++++++++++------ src/mips/full-codegen-mips.cc | 14 +++++++++----- src/x64/full-codegen-x64.cc | 14 +++++++++----- test/cctest/test-api.cc | 10 ++++++---- test/message/regress/regress-1527.js | 33 +++++++++++++++++++++++++++++++++ test/message/regress/regress-1527.out | 32 ++++++++++++++++++++++++++++++++ test/mjsunit/debug-backtrace.js | 3 +-- 9 files changed, 110 insertions(+), 28 deletions(-) create mode 100644 test/message/regress/regress-1527.js create mode 100644 test/message/regress/regress-1527.out diff --git a/src/arm/full-codegen-arm.cc b/src/arm/full-codegen-arm.cc index 4b55915..34d4baa 100644 --- a/src/arm/full-codegen-arm.cc +++ b/src/arm/full-codegen-arm.cc @@ -776,7 +776,7 @@ void FullCodeGenerator::EmitDeclaration(Variable* variable, // IDs for bailouts from optimized code. ASSERT(prop->obj()->AsVariableProxy() != NULL); { AccumulatorValueContext for_object(this); - EmitVariableLoad(prop->obj()->AsVariableProxy()->var()); + EmitVariableLoad(prop->obj()->AsVariableProxy()); } __ push(r0); @@ -1113,7 +1113,7 @@ void FullCodeGenerator::EmitNewClosure(Handle info, void FullCodeGenerator::VisitVariableProxy(VariableProxy* expr) { Comment cmnt(masm_, "[ VariableProxy"); - EmitVariableLoad(expr->var()); + EmitVariableLoad(expr); } @@ -1262,7 +1262,11 @@ void FullCodeGenerator::EmitDynamicLoadFromSlotFastCase( } -void FullCodeGenerator::EmitVariableLoad(Variable* var) { +void FullCodeGenerator::EmitVariableLoad(VariableProxy* proxy) { + // Record position before possible IC call. + SetSourcePosition(proxy->position()); + Variable* var = proxy->var(); + // Three cases: non-this global variables, lookup slots, and all other // types of slots. Slot* slot = var->AsSlot(); @@ -1593,7 +1597,7 @@ void FullCodeGenerator::VisitAssignment(Assignment* expr) { { AccumulatorValueContext context(this); switch (assign_type) { case VARIABLE: - EmitVariableLoad(expr->target()->AsVariableProxy()->var()); + EmitVariableLoad(expr->target()->AsVariableProxy()); PrepareForBailout(expr->target(), TOS_REG); break; case NAMED_PROPERTY: @@ -3816,7 +3820,7 @@ void FullCodeGenerator::VisitCountOperation(CountOperation* expr) { if (assign_type == VARIABLE) { ASSERT(expr->expression()->AsVariableProxy()->var() != NULL); AccumulatorValueContext context(this); - EmitVariableLoad(expr->expression()->AsVariableProxy()->var()); + EmitVariableLoad(expr->expression()->AsVariableProxy()); } else { // Reserve space for result of postfix operation. if (expr->is_postfix() && !context()->IsEffect()) { diff --git a/src/full-codegen.h b/src/full-codegen.h index d25ca49..6b174f7 100644 --- a/src/full-codegen.h +++ b/src/full-codegen.h @@ -444,7 +444,7 @@ class FullCodeGenerator: public AstVisitor { TypeofState typeof_state, Label* slow, Label* done); - void EmitVariableLoad(Variable* expr); + void EmitVariableLoad(VariableProxy* proxy); enum ResolveEvalFlag { SKIP_CONTEXT_LOOKUP, diff --git a/src/ia32/full-codegen-ia32.cc b/src/ia32/full-codegen-ia32.cc index 75cc4b8..c0d7e7c 100644 --- a/src/ia32/full-codegen-ia32.cc +++ b/src/ia32/full-codegen-ia32.cc @@ -744,7 +744,7 @@ void FullCodeGenerator::EmitDeclaration(Variable* variable, // IDs for bailouts from optimized code. ASSERT(prop->obj()->AsVariableProxy() != NULL); { AccumulatorValueContext for_object(this); - EmitVariableLoad(prop->obj()->AsVariableProxy()->var()); + EmitVariableLoad(prop->obj()->AsVariableProxy()); } __ push(eax); @@ -1064,7 +1064,7 @@ void FullCodeGenerator::EmitNewClosure(Handle info, void FullCodeGenerator::VisitVariableProxy(VariableProxy* expr) { Comment cmnt(masm_, "[ VariableProxy"); - EmitVariableLoad(expr->var()); + EmitVariableLoad(expr); } @@ -1214,7 +1214,11 @@ void FullCodeGenerator::EmitDynamicLoadFromSlotFastCase( } -void FullCodeGenerator::EmitVariableLoad(Variable* var) { +void FullCodeGenerator::EmitVariableLoad(VariableProxy* proxy) { + // Record position before possible IC call. + SetSourcePosition(proxy->position()); + Variable* var = proxy->var(); + // Three cases: non-this global variables, lookup slots, and all other // types of slots. Slot* slot = var->AsSlot(); @@ -1540,7 +1544,7 @@ void FullCodeGenerator::VisitAssignment(Assignment* expr) { { AccumulatorValueContext context(this); switch (assign_type) { case VARIABLE: - EmitVariableLoad(expr->target()->AsVariableProxy()->var()); + EmitVariableLoad(expr->target()->AsVariableProxy()); PrepareForBailout(expr->target(), TOS_REG); break; case NAMED_PROPERTY: @@ -1769,7 +1773,7 @@ void FullCodeGenerator::EmitAssignment(Expression* expr, int bailout_ast_id) { ASSERT(prop->obj()->AsVariableProxy() != NULL); ASSERT(prop->key()->AsLiteral() != NULL); { AccumulatorValueContext for_object(this); - EmitVariableLoad(prop->obj()->AsVariableProxy()->var()); + EmitVariableLoad(prop->obj()->AsVariableProxy()); } __ mov(edx, eax); __ SafeSet(ecx, Immediate(prop->key()->AsLiteral()->handle())); @@ -3768,7 +3772,7 @@ void FullCodeGenerator::VisitCountOperation(CountOperation* expr) { if (assign_type == VARIABLE) { ASSERT(expr->expression()->AsVariableProxy()->var() != NULL); AccumulatorValueContext context(this); - EmitVariableLoad(expr->expression()->AsVariableProxy()->var()); + EmitVariableLoad(expr->expression()->AsVariableProxy()); } else { // Reserve space for result of postfix operation. if (expr->is_postfix() && !context()->IsEffect()) { diff --git a/src/mips/full-codegen-mips.cc b/src/mips/full-codegen-mips.cc index 2645bdd..eaf6414 100644 --- a/src/mips/full-codegen-mips.cc +++ b/src/mips/full-codegen-mips.cc @@ -783,7 +783,7 @@ void FullCodeGenerator::EmitDeclaration(Variable* variable, // IDs for bailouts from optimized code. ASSERT(prop->obj()->AsVariableProxy() != NULL); { AccumulatorValueContext for_object(this); - EmitVariableLoad(prop->obj()->AsVariableProxy()->var()); + EmitVariableLoad(prop->obj()->AsVariableProxy()); } __ push(result_register()); @@ -1117,7 +1117,7 @@ void FullCodeGenerator::EmitNewClosure(Handle info, void FullCodeGenerator::VisitVariableProxy(VariableProxy* expr) { Comment cmnt(masm_, "[ VariableProxy"); - EmitVariableLoad(expr->var()); + EmitVariableLoad(expr); } @@ -1262,7 +1262,11 @@ void FullCodeGenerator::EmitDynamicLoadFromSlotFastCase( } -void FullCodeGenerator::EmitVariableLoad(Variable* var) { +void FullCodeGenerator::EmitVariableLoad(VariableProxy* proxy) { + // Record position before possible IC call. + SetSourcePosition(proxy->position()); + Variable* var = proxy->var(); + // Three cases: non-this global variables, lookup slots, and all other // types of slots. Slot* slot = var->AsSlot(); @@ -1598,7 +1602,7 @@ void FullCodeGenerator::VisitAssignment(Assignment* expr) { { AccumulatorValueContext context(this); switch (assign_type) { case VARIABLE: - EmitVariableLoad(expr->target()->AsVariableProxy()->var()); + EmitVariableLoad(expr->target()->AsVariableProxy()); PrepareForBailout(expr->target(), TOS_REG); break; case NAMED_PROPERTY: @@ -3839,7 +3843,7 @@ void FullCodeGenerator::VisitCountOperation(CountOperation* expr) { if (assign_type == VARIABLE) { ASSERT(expr->expression()->AsVariableProxy()->var() != NULL); AccumulatorValueContext context(this); - EmitVariableLoad(expr->expression()->AsVariableProxy()->var()); + EmitVariableLoad(expr->expression()->AsVariableProxy()); } else { // Reserve space for result of postfix operation. if (expr->is_postfix() && !context()->IsEffect()) { diff --git a/src/x64/full-codegen-x64.cc b/src/x64/full-codegen-x64.cc index 6629927..46f8c73 100644 --- a/src/x64/full-codegen-x64.cc +++ b/src/x64/full-codegen-x64.cc @@ -741,7 +741,7 @@ void FullCodeGenerator::EmitDeclaration(Variable* variable, // IDs for bailouts from optimized code. ASSERT(prop->obj()->AsVariableProxy() != NULL); { AccumulatorValueContext for_object(this); - EmitVariableLoad(prop->obj()->AsVariableProxy()->var()); + EmitVariableLoad(prop->obj()->AsVariableProxy()); } __ push(rax); VisitForAccumulatorValue(function); @@ -1071,7 +1071,7 @@ void FullCodeGenerator::EmitNewClosure(Handle info, void FullCodeGenerator::VisitVariableProxy(VariableProxy* expr) { Comment cmnt(masm_, "[ VariableProxy"); - EmitVariableLoad(expr->var()); + EmitVariableLoad(expr); } @@ -1222,7 +1222,11 @@ void FullCodeGenerator::EmitDynamicLoadFromSlotFastCase( } -void FullCodeGenerator::EmitVariableLoad(Variable* var) { +void FullCodeGenerator::EmitVariableLoad(VariableProxy* proxy) { + // Record position before possible IC call. + SetSourcePosition(proxy->position()); + Variable* var = proxy->var(); + // Three cases: non-this global variables, lookup slots, and all other // types of slots. Slot* slot = var->AsSlot(); @@ -1548,7 +1552,7 @@ void FullCodeGenerator::VisitAssignment(Assignment* expr) { { AccumulatorValueContext context(this); switch (assign_type) { case VARIABLE: - EmitVariableLoad(expr->target()->AsVariableProxy()->var()); + EmitVariableLoad(expr->target()->AsVariableProxy()); PrepareForBailout(expr->target(), TOS_REG); break; case NAMED_PROPERTY: @@ -3746,7 +3750,7 @@ void FullCodeGenerator::VisitCountOperation(CountOperation* expr) { if (assign_type == VARIABLE) { ASSERT(expr->expression()->AsVariableProxy()->var() != NULL); AccumulatorValueContext context(this); - EmitVariableLoad(expr->expression()->AsVariableProxy()->var()); + EmitVariableLoad(expr->expression()->AsVariableProxy()); } else { // Reserve space for result of postfix operation. if (expr->is_postfix() && !context()->IsEffect()) { diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc index 1531f90..8d8770f 100644 --- a/test/cctest/test-api.cc +++ b/test/cctest/test-api.cc @@ -12633,9 +12633,10 @@ v8::Handle AnalyzeStackInNativeCode(const v8::Arguments& args) { stackTrace->GetFrame(0)); checkStackFrame(origin, "foo", 6, 3, false, false, stackTrace->GetFrame(1)); - checkStackFrame(NULL, "", 1, 1, false, false, + // This is the source string inside the eval which has the call to foo. + checkStackFrame(NULL, "", 1, 5, false, false, stackTrace->GetFrame(2)); - // The last frame is an anonymous function that has the initial call. + // The last frame is an anonymous function which has the initial eval call. checkStackFrame(origin, "", 8, 7, false, false, stackTrace->GetFrame(3)); @@ -12654,9 +12655,10 @@ v8::Handle AnalyzeStackInNativeCode(const v8::Arguments& args) { bool is_eval = false; #endif // ENABLE_DEBUGGER_SUPPORT - checkStackFrame(NULL, "", 1, 1, is_eval, false, + // This is the source string inside the eval which has the call to baz. + checkStackFrame(NULL, "", 1, 5, is_eval, false, stackTrace->GetFrame(2)); - // The last frame is an anonymous function that has the initial call to foo. + // The last frame is an anonymous function which has the initial eval call. checkStackFrame(origin, "", 10, 1, false, false, stackTrace->GetFrame(3)); diff --git a/test/message/regress/regress-1527.js b/test/message/regress/regress-1527.js new file mode 100644 index 0000000..682e386 --- /dev/null +++ b/test/message/regress/regress-1527.js @@ -0,0 +1,33 @@ +// Copyright 2011 the V8 project authors. All rights reserved. +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following +// disclaimer in the documentation and/or other materials provided +// with the distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived +// from this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +var o1 = {foo: 'bar'}; +var o2 = { + 1: 'blah', + 2: o1.foo, + 3: foo +} diff --git a/test/message/regress/regress-1527.out b/test/message/regress/regress-1527.out new file mode 100644 index 0000000..dc17fb3 --- /dev/null +++ b/test/message/regress/regress-1527.out @@ -0,0 +1,32 @@ +# Copyright 2011 the V8 project authors. All rights reserved. +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are +# met: +# +# * Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# * Redistributions in binary form must reproduce the above +# copyright notice, this list of conditions and the following +# disclaimer in the documentation and/or other materials provided +# with the distribution. +# * Neither the name of Google Inc. nor the names of its +# contributors may be used to endorse or promote products derived +# from this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +*%(basename)s:32: ReferenceError: foo is not defined + 3: foo + ^ +ReferenceError: foo is not defined + at *%(basename)s:32:6 diff --git a/test/mjsunit/debug-backtrace.js b/test/mjsunit/debug-backtrace.js index d15b2d2..3647913 100644 --- a/test/mjsunit/debug-backtrace.js +++ b/test/mjsunit/debug-backtrace.js @@ -195,7 +195,7 @@ function listener(event, exec_state, event_data, data) { assertEquals("m", response.lookup(frame.func.ref).inferredName); assertFalse(frame.constructCall); assertEquals(35, frame.line); - assertEquals(2, frame.column); + assertEquals(6, frame.column); assertEquals(0, frame.arguments.length); json = '{"seq":0,"type":"request","command":"frame","arguments":{"number":3}}' @@ -269,4 +269,3 @@ g(); // Make sure that the debug event listener vas invoked. assertFalse(exception, "exception in listener"); assertTrue(listenerCalled); - -- 2.7.4