Fix a bug in scope analysis.

When recompiling code (e.g., when optimizing) we could incorrectly hoist
some function expressions.  This leads to incorrect results or a crash.  The
root cause was that functions were not correctly categorized as expression
or declaration at parse time.

This requires some extra hoops to prevent the print name "anonymous" for
functions created by 'new Function' from establishing a binding.

R=vegorov@chromium.org,kasperl@chromium.org
BUG=1583
TEST=regress-1583

Review URL: http://codereview.chromium.org/7572019

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@8838 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/objects-inl.h

@@ -3534,35 +3534,13 @@ void SharedFunctionInfo::set_optimization_disabled(bool disable) {
 }
 
 
-BOOL_ACCESSORS(SharedFunctionInfo,
-               compiler_hints,
-               strict_mode,
+BOOL_ACCESSORS(SharedFunctionInfo, compiler_hints, strict_mode,
                kStrictModeFunction)
-
-
-bool SharedFunctionInfo::native() {
-  return BooleanBit::get(compiler_hints(), kNative);
-}
-
-
-void SharedFunctionInfo::set_native(bool value) {
-  set_compiler_hints(BooleanBit::set(compiler_hints(),
-                                     kNative,
-                                     value));
-}
-
-
-bool SharedFunctionInfo::bound() {
-  return BooleanBit::get(compiler_hints(), kBoundFunction);
-}
-
-
-void SharedFunctionInfo::set_bound(bool value) {
-  set_compiler_hints(BooleanBit::set(compiler_hints(),
-                                     kBoundFunction,
-                                     value));
-}
-
+BOOL_ACCESSORS(SharedFunctionInfo, compiler_hints, native, kNative)
+BOOL_ACCESSORS(SharedFunctionInfo, compiler_hints,
+               name_should_print_as_anonymous,
+               kNameShouldPrintAsAnonymous)
+BOOL_ACCESSORS(SharedFunctionInfo, compiler_hints, bound, kBoundFunction)
 
 ACCESSORS(CodeCache, default_cache, FixedArray, kDefaultCacheOffset)
 ACCESSORS(CodeCache, normal_type_cache, Object, kNormalTypeCacheOffset)

src/objects.h

@@ -4730,6 +4730,13 @@ class SharedFunctionInfo: public HeapObject {
   inline bool native();
   inline void set_native(bool value);
 
+  // Indicates that the function was created by the Function function.
+  // Though it's anonymous, toString should treat it as if it had the name
+  // "anonymous".  We don't set the name itself so that the system does not
+  // see a binding for it.
+  inline bool name_should_print_as_anonymous();
+  inline void set_name_should_print_as_anonymous(bool flag);
+
   // Indicates whether the function is a bound function created using
   // the bind function.
   inline bool bound();
@@ -4915,7 +4922,6 @@ class SharedFunctionInfo: public HeapObject {
   // Bit positions in compiler_hints.
   static const int kCodeAgeSize = 3;
   static const int kCodeAgeMask = (1 << kCodeAgeSize) - 1;
-  static const int kBoundFunction = 9;
 
   enum CompilerHints {
     kHasOnlySimpleThisPropertyAssignments,
@@ -4926,7 +4932,10 @@ class SharedFunctionInfo: public HeapObject {
     kStrictModeFunction,
     kUsesArguments,
     kHasDuplicateParameters,
-    kNative
+    kNative,
+    kBoundFunction,
+    kNameShouldPrintAsAnonymous,
+    kCompilerHintsCount  // Pseudo entry
   };
 
  private:
@@ -4940,6 +4949,9 @@ class SharedFunctionInfo: public HeapObject {
   static const int kCompilerHintsSize = kIntSize;
 #endif
 
+  STATIC_ASSERT(SharedFunctionInfo::kCompilerHintsCount <=
+                SharedFunctionInfo::kCompilerHintsSize * kBitsPerByte);
+
  public:
   // Constants for optimizing codegen for strict mode function and
   // native tests.

src/parser.cc

@@ -2842,8 +2842,11 @@ Expression* Parser::ParseMemberWithNewPrefixesExpression(PositionStack* stack,
       name = ParseIdentifierOrStrictReservedWord(&is_strict_reserved_name,
                                                  CHECK_OK);
     }
-    result = ParseFunctionLiteral(name, is_strict_reserved_name,
-                                  function_token_position, NESTED, CHECK_OK);
+    result = ParseFunctionLiteral(name,
+                                  is_strict_reserved_name,
+                                  function_token_position,
+                                  EXPRESSION,
+                                  CHECK_OK);
   } else {
     result = ParsePrimaryExpression(CHECK_OK);
   }
@@ -3412,7 +3415,7 @@ ObjectLiteral::Property* Parser::ParseObjectLiteralGetSet(bool is_getter,
         ParseFunctionLiteral(name,
                              false,   // reserved words are allowed here
                              RelocInfo::kNoPosition,
-                             DECLARATION,
+                             EXPRESSION,
                              CHECK_OK);
     // Allow any number of parameters for compatiabilty with JSC.
     // Specification only allows zero parameters for get and one for set.
@@ -3619,25 +3622,23 @@ ZoneList<Expression*>* Parser::ParseArguments(bool* ok) {
 }
 
 
-FunctionLiteral* Parser::ParseFunctionLiteral(Handle<String> var_name,
+FunctionLiteral* Parser::ParseFunctionLiteral(Handle<String> function_name,
                                               bool name_is_strict_reserved,
                                               int function_token_position,
                                               FunctionLiteralType type,
                                               bool* ok) {
   // Function ::
   //   '(' FormalParameterList? ')' '{' FunctionBody '}'
-  bool is_named = !var_name.is_null();
+  bool has_name = !function_name.is_null();
+  // The function's name, empty if the function is anonymous.
+  if (!has_name) {
+    function_name = isolate()->factory()->empty_symbol();
+  }
 
-  // The name associated with this function. If it's a function expression,
-  // this is the actual function name, otherwise this is the name of the
-  // variable declared and initialized with the function (expression). In
-  // that case, we don't have a function name (it's empty).
-  Handle<String> name =
-      is_named ? var_name : isolate()->factory()->empty_symbol();
-  // The function name, if any.
-  Handle<String> function_name = isolate()->factory()->empty_symbol();
-  if (is_named && (type == EXPRESSION || type == NESTED)) {
-    function_name = name;
+  // The name of a named function expression, otherwise an empty handle.
+  Handle<String> self_name;
+  if (type == EXPRESSION && function_name->length() > 0) {
+    self_name = function_name;
   }
 
   int num_parameters = 0;
@@ -3655,7 +3656,7 @@ FunctionLiteral* Parser::ParseFunctionLiteral(Handle<String> var_name,
   bool has_duplicate_parameters = false;
   // Parse function body.
   { LexicalScope lexical_scope(this, scope, isolate());
-    top_scope_->SetScopeName(name);
+    top_scope_->SetScopeName(function_name);
 
     //  FormalParameterList ::
     //    '(' (Identifier)*[','] ')'
@@ -3705,10 +3706,10 @@ FunctionLiteral* Parser::ParseFunctionLiteral(Handle<String> var_name,
     // NOTE: We create a proxy and resolve it here so that in the
     // future we can change the AST to only refer to VariableProxies
     // instead of Variables and Proxis as is the case now.
-    if (!function_name.is_null() && function_name->length() > 0) {
-      Variable* fvar = top_scope_->DeclareFunctionVar(function_name);
+    if (!self_name.is_null()) {
+      Variable* fvar = top_scope_->DeclareFunctionVar(self_name);
       VariableProxy* fproxy =
-          top_scope_->NewUnresolved(function_name, inside_with());
+          top_scope_->NewUnresolved(self_name, inside_with());
       fproxy->BindTo(fvar);
       body->Add(new(zone()) ExpressionStatement(
           new(zone()) Assignment(isolate(),
@@ -3739,7 +3740,7 @@ FunctionLiteral* Parser::ParseFunctionLiteral(Handle<String> var_name,
         end_pos = entry.end_pos();
         if (end_pos <= function_block_pos) {
           // End position greater than end of stream is safe, and hard to check.
-          ReportInvalidPreparseData(name, CHECK_OK);
+          ReportInvalidPreparseData(function_name, CHECK_OK);
         }
         isolate()->counters()->total_preparse_skipped()->Increment(
             end_pos - function_block_pos);
@@ -3769,7 +3770,7 @@ FunctionLiteral* Parser::ParseFunctionLiteral(Handle<String> var_name,
 
     // Validate strict mode.
     if (top_scope_->is_strict_mode()) {
-      if (IsEvalOrArguments(name)) {
+      if (IsEvalOrArguments(function_name)) {
         int position = function_token_position != RelocInfo::kNoPosition
             ? function_token_position
             : (start_pos > 0 ? start_pos - 1 : start_pos);
@@ -3813,7 +3814,7 @@ FunctionLiteral* Parser::ParseFunctionLiteral(Handle<String> var_name,
 
   FunctionLiteral* function_literal =
       new(zone()) FunctionLiteral(isolate(),
-                                  name,
+                                  function_name,
                                   scope,
                                   body,
                                   materialized_literal_count,
@@ -3823,11 +3824,11 @@ FunctionLiteral* Parser::ParseFunctionLiteral(Handle<String> var_name,
                                   num_parameters,
                                   start_pos,
                                   end_pos,
-                                  (function_name->length() > 0),
+                                  type == EXPRESSION,
                                   has_duplicate_parameters);
   function_literal->set_function_token_position(function_token_position);
 
-  if (fni_ != NULL && !is_named) fni_->AddFunction(function_literal);
+  if (fni_ != NULL && !has_name) fni_->AddFunction(function_literal);
   return function_literal;
 }
 

src/parser.h

@@ -554,8 +554,7 @@ class Parser {
 
   enum FunctionLiteralType {
     EXPRESSION,
-    DECLARATION,
-    NESTED
+    DECLARATION
   };
 
   ZoneList<Expression*>* ParseArguments(bool* ok);

src/runtime.cc

@@ -615,8 +615,7 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_CreateJSProxy) {
 RUNTIME_FUNCTION(MaybeObject*, Runtime_IsJSProxy) {
   ASSERT(args.length() == 1);
   Object* obj = args[0];
-  return obj->IsJSProxy()
-      ? isolate->heap()->true_value() : isolate->heap()->false_value();
+  return isolate->heap()->ToBoolean(obj->IsJSProxy());
 }
 
 
@@ -1038,8 +1037,7 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_IsExtensible) {
     ASSERT(proto->IsJSGlobalObject());
     obj = JSObject::cast(proto);
   }
-  return obj->map()->is_extensible() ? isolate->heap()->true_value()
-                                     : isolate->heap()->false_value();
+  return isolate->heap()->ToBoolean(obj->map()->is_extensible());
 }
 
 
@@ -1105,8 +1103,7 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_DisableAccessChecks) {
     Map::cast(new_map)->set_is_access_check_needed(false);
     object->set_map(Map::cast(new_map));
   }
-  return needs_access_checks ? isolate->heap()->true_value()
-                             : isolate->heap()->false_value();
+  return isolate->heap()->ToBoolean(needs_access_checks);
 }
 
 
@@ -1917,6 +1914,24 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_FunctionSetName) {
 }
 
 
+RUNTIME_FUNCTION(MaybeObject*, Runtime_FunctionNameShouldPrintAsAnonymous) {
+  NoHandleAllocation ha;
+  ASSERT(args.length() == 1);
+  CONVERT_CHECKED(JSFunction, f, args[0]);
+  return isolate->heap()->ToBoolean(
+      f->shared()->name_should_print_as_anonymous());
+}
+
+
+RUNTIME_FUNCTION(MaybeObject*, Runtime_FunctionMarkNameShouldPrintAsAnonymous) {
+  NoHandleAllocation ha;
+  ASSERT(args.length() == 1);
+  CONVERT_CHECKED(JSFunction, f, args[0]);
+  f->shared()->set_name_should_print_as_anonymous(true);
+  return isolate->heap()->undefined_value();
+}
+
+
 RUNTIME_FUNCTION(MaybeObject*, Runtime_FunctionSetBound) {
   HandleScope scope(isolate);
   ASSERT(args.length() == 1);
@@ -2097,8 +2112,7 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_FunctionIsAPIFunction) {
   ASSERT(args.length() == 1);
 
   CONVERT_CHECKED(JSFunction, f, args[0]);
-  return f->shared()->IsApiFunction() ? isolate->heap()->true_value()
-                                      : isolate->heap()->false_value();
+  return isolate->heap()->ToBoolean(f->shared()->IsApiFunction());
 }
 
 
@@ -2107,8 +2121,7 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_FunctionIsBuiltin) {
   ASSERT(args.length() == 1);
 
   CONVERT_CHECKED(JSFunction, f, args[0]);
-  return f->IsBuiltin() ? isolate->heap()->true_value() :
-                          isolate->heap()->false_value();
+  return isolate->heap()->ToBoolean(f->IsBuiltin());
 }
 
 
@@ -9980,9 +9993,7 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_DebugGetPropertyDetails) {
       details->set(0, *value);
       details->set(1, property_details);
       if (hasJavaScriptAccessors) {
-        details->set(2,
-                     caught_exception ? isolate->heap()->true_value()
-                                      : isolate->heap()->false_value());
+        details->set(2, isolate->heap()->ToBoolean(caught_exception));
         details->set(3, FixedArray::cast(*result_callback_obj)->get(0));
         details->set(4, FixedArray::cast(*result_callback_obj)->get(1));
       }
@@ -12165,8 +12176,7 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_DeleteLOL) {
 #ifdef LIVE_OBJECT_LIST
   CONVERT_SMI_ARG_CHECKED(id, 0);
   bool success = LiveObjectList::Delete(id);
-  return success ? isolate->heap()->true_value() :
-                   isolate->heap()->false_value();
+  return isolate->heap()->ToBoolean(success);
 #else
   return isolate->heap()->undefined_value();
 #endif

src/runtime.h

@@ -214,6 +214,8 @@ namespace internal {
   F(FunctionSetReadOnlyPrototype, 1, 1) \
   F(FunctionGetName, 1, 1) \
   F(FunctionSetName, 2, 1) \
+  F(FunctionNameShouldPrintAsAnonymous, 1, 1) \
+  F(FunctionMarkNameShouldPrintAsAnonymous, 1, 1) \
   F(FunctionSetBound, 1, 1) \
   F(FunctionRemovePrototype, 1, 1) \
   F(FunctionGetSourceCode, 1, 1) \

src/v8natives.js

-// Copyright 2006-2008 the V8 project authors. All rights reserved.
+// 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:
@@ -1428,7 +1428,9 @@ function FunctionSourceString(func) {
     }
   }
 
-  var name = %FunctionGetName(func);
+  var name = %FunctionNameShouldPrintAsAnonymous(func)
+      ? 'anonymous'
+      : %FunctionGetName(func);
   return 'function ' + name + source;
 }
 
@@ -1523,7 +1525,7 @@ function NewFunction(arg1) {  // length == 1
   // The call to SetNewFunctionAttributes will ensure the prototype
   // property of the resulting function is enumerable (ECMA262, 15.3.5.2).
   var f = %CompileString(source)();
-  %FunctionSetName(f, "anonymous");
+  %FunctionMarkNameShouldPrintAsAnonymous(f);
   return %SetNewFunctionAttributes(f);
 }
 

test/mjsunit/regress/regress-1583.js

+// 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.
+
+// Flags: --allow-natives-syntax
+
+// Regression test for a bug in recompilation of anonymous functions inside
+// catch.  We would incorrectly hoist them outside the catch in some cases.
+function f() {
+  try {
+    throw 0;
+  } catch (e) {
+    try {
+      var x = { a: 'hest' };
+      x.m = function (e) { return x.a; };
+    } catch (e) {
+    }
+  }
+  return x;
+}
+
+var o = f();
+assertEquals('hest', o.m());
+assertEquals('hest', o.m());
+assertEquals('hest', o.m());
+%OptimizeFunctionOnNextCall(o.m);
+assertEquals('hest', o.m());