From 5bbb1d7bd64d2b63307a40d75e0d4a6ffb593f50 Mon Sep 17 00:00:00 2001 From: "lrn@chromium.org" Date: Tue, 8 Dec 2009 09:43:51 +0000 Subject: [PATCH] Fix for issue 545: don't reuse this VariableProxy. Review URL: http://codereview.chromium.org/464069 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@3432 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/compiler.cc | 2 ++ src/scopes.cc | 3 +-- src/scopes.h | 11 ++++++--- test/mjsunit/regress/regress-545.js | 47 +++++++++++++++++++++++++++++++++++++ 4 files changed, 58 insertions(+), 5 deletions(-) create mode 100644 test/mjsunit/regress/regress-545.js diff --git a/src/compiler.cc b/src/compiler.cc index 48da63d..03fc2d6 100644 --- a/src/compiler.cc +++ b/src/compiler.cc @@ -56,6 +56,8 @@ class CodeGenSelector: public AstVisitor { private: // Visit an expression in a given expression context. void ProcessExpression(Expression* expr, Expression::Context context) { + ASSERT(expr->context() == Expression::kUninitialized || + expr->context() == context); Expression::Context saved = context_; context_ = context; Visit(expr); diff --git a/src/scopes.cc b/src/scopes.cc index 7da06cd..a47d373 100644 --- a/src/scopes.cc +++ b/src/scopes.cc @@ -189,8 +189,7 @@ void Scope::Initialize(bool inside_with) { variables_.Declare(this, Factory::this_symbol(), Variable::VAR, false, Variable::THIS); var->rewrite_ = new Slot(var, Slot::PARAMETER, -1); - receiver_ = new VariableProxy(Factory::this_symbol(), true, false); - receiver_->BindTo(var); + receiver_ = var; if (is_function_scope()) { // Declare 'arguments' variable which exists in all functions. diff --git a/src/scopes.h b/src/scopes.h index fc627df..9b506d9 100644 --- a/src/scopes.h +++ b/src/scopes.h @@ -206,8 +206,13 @@ class Scope: public ZoneObject { // --------------------------------------------------------------------------- // Accessors. - // The variable corresponding to the (function) receiver. - VariableProxy* receiver() const { return receiver_; } + // A new variable proxy corresponding to the (function) receiver. + VariableProxy* receiver() const { + VariableProxy* proxy = + new VariableProxy(Factory::this_symbol(), true, false); + proxy->BindTo(receiver_); + return proxy; + } // The variable holding the function literal for named function // literals, or NULL. @@ -314,7 +319,7 @@ class Scope: public ZoneObject { // Declarations. ZoneList decls_; // Convenience variable. - VariableProxy* receiver_; + Variable* receiver_; // Function variable, if any; function scopes only. Variable* function_; // Convenience variable; function scopes only. diff --git a/test/mjsunit/regress/regress-545.js b/test/mjsunit/regress/regress-545.js new file mode 100644 index 0000000..36cde6d --- /dev/null +++ b/test/mjsunit/regress/regress-545.js @@ -0,0 +1,47 @@ +// Copyright 2009 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. + +// See: http://code.google.com/p/v8/issues/detail?id=545 +// and: http://code.google.com/p/chromium/issues/detail?id=28353 + +// The "this" variable proxy was reused. If context annotations differ between +// uses, this can cause a use in a value context to assume a test context. Since +// it has no true/false labels set, it causes a null-pointer dereference and +// segmentation fault. + +// Code should not crash: + +// Original bug report by Robert Swiecki (wrapped to not throw): +try { + new IsPrimitive(load())?this.join():String(' ').charCodeAt((!this>Math)); +} catch (e) {} + +// Shorter examples: + +this + !this; + +this + (this ? 1 : 2); -- 2.7.4