Allow implicit 'close your eyes' of native methods. This will hopefully fix issue 3466.

vm/code_generator_ia32.cc

 // Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #include "vm/globals.h"  // Needed here to get TARGET_ARCH_IA32.
 #if defined(TARGET_ARCH_IA32)
 
 #include "vm/code_generator.h"
 
 #include "lib/error.h"
@@ skipping 355 matching lines @@
 
 // Input parameters:
 //   ESP : points to return address.
 //   ESP + 4 : address of last argument (arg n-1).
 //   ESP + 4*n : address of first argument (arg 0).
 //   EDX : arguments descriptor array.
 void CodeGenerator::GenerateEntryCode() {
   const Immediate raw_null =
       Immediate(reinterpret_cast<intptr_t>(Object::null()));
   const Function& function = parsed_function_.function();
+  const bool is_implicit_native_closure =
+      function.is_native() && function.IsImplicitClosureFunction();

[regis, 2012/06/15 17:13:34]

You should be using IsImplicitInstanceClosureFunction().

[siva, 2012/06/16 00:25:43]

Nice catch. Thanks for finding this.

On 2012/06/15 17:13:34, regis wrote:

 
   // 1. Compute the frame size and enter the frame (reserving local space
   // for copied incoming and default arguments and stack-allocated local
   // variables).
   //
   // TODO(regis): We may give up reserving space on stack for args/locals
   // because pushes of initial values may be more effective than moves.
   LocalScope* scope = parsed_function_.node_sequence()->scope();
   const int num_fixed_params = function.num_fixed_parameters();
   const int num_opt_params = function.num_optional_parameters();
@@ skipping 48 matching lines @@
     __ movl(EBX, FieldAddress(EDX, Array::data_offset()));
     // Check that num_args <= num_params.
     Label wrong_num_arguments;
     __ cmpl(EBX, Immediate(Smi::RawValue(num_params)));
     __ j(GREATER, &wrong_num_arguments);
     // Number of positional args is the second Smi in descriptor array (EDX).
     __ movl(ECX, FieldAddress(EDX, Array::data_offset() + (1 * kWordSize)));
     // Check that num_pos_args >= num_fixed_params.
     __ cmpl(ECX, Immediate(Smi::RawValue(num_fixed_params)));
     __ j(LESS, &wrong_num_arguments);
+
     // Since EBX and ECX are Smi, use TIMES_2 instead of TIMES_4.
     // Let EBX point to the last passed positional argument, i.e. to
     // fp[1 + num_args - (num_pos_args - 1)].
     __ subl(EBX, ECX);
     __ leal(EBX, Address(EBP, EBX, TIMES_2, 2 * kWordSize));
     // Let EDI point to the last copied positional argument, i.e. to
     // fp[ParsedFunction::kFirstLocalSlotIndex - (num_pos_args - 1)].
-    const int index = ParsedFunction::kFirstLocalSlotIndex + 1;
+    int implicit_this_param_pos = is_implicit_native_closure ? -1 : 0;
+    const int index =
+        ParsedFunction::kFirstLocalSlotIndex + 1 + implicit_this_param_pos;
+    // First copy captured receiver if function is an implicit native closure.
+    if (is_implicit_native_closure) {
+      __ movl(EAX, FieldAddress(CTX, Context::variable_offset(0)));
+      __ movl(Address(EBP, (index * kWordSize)), EAX);
+    }
     __ leal(EDI, Address(EBP, (index * kWordSize)));
     __ subl(EDI, ECX);  // ECX is a Smi, subtract twice for TIMES_4 scaling.
     __ subl(EDI, ECX);
     __ SmiUntag(ECX);
     Label loop, loop_condition;
     __ jmp(&loop_condition, Assembler::kNearJump);
     // We do not use the final allocation index of the variable here, i.e.
     // scope->VariableAt(i)->index(), because captured variables still need
     // to be copied to the context that is not yet allocated.
     const Address argument_addr(EBX, ECX, TIMES_4, 0);
     const Address copy_addr(EDI, ECX, TIMES_4, 0);
     __ Bind(&loop);
     __ movl(EAX, argument_addr);
     __ movl(copy_addr, EAX);
     __ Bind(&loop_condition);
     __ decl(ECX);
     __ j(POSITIVE, &loop, Assembler::kNearJump);
 
     // Copy or initialize optional named arguments.
-    ASSERT(num_opt_params > 0);  // Or we would not have to copy arguments.
-    // Start by alphabetically sorting the names of the optional parameters.
-    LocalVariable** opt_param = new LocalVariable*[num_opt_params];
-    int* opt_param_position = new int[num_opt_params];
-    for (int pos = num_fixed_params; pos < num_params; pos++) {
-      LocalVariable* parameter = scope->VariableAt(pos);
-      const String& opt_param_name = parameter->name();
-      int i = pos - num_fixed_params;
-      while (--i >= 0) {
-        LocalVariable* param_i = opt_param[i];
-        const intptr_t result = opt_param_name.CompareTo(param_i->name());
-        ASSERT(result != 0);
-        if (result > 0) break;
-        opt_param[i + 1] = opt_param[i];
-        opt_param_position[i + 1] = opt_param_position[i];
+    Label all_arguments_processed;
+    if (num_opt_params > 0) {
+      // Start by alphabetically sorting the names of the optional parameters.
+      LocalVariable** opt_param = new LocalVariable*[num_opt_params];
+      int* opt_param_position = new int[num_opt_params];
+      for (int pos = num_fixed_params; pos < num_params; pos++) {
+        LocalVariable* parameter = scope->VariableAt(pos);
+        const String& opt_param_name = parameter->name();
+        int i = pos - num_fixed_params;
+        while (--i >= 0) {
+          LocalVariable* param_i = opt_param[i];
+          const intptr_t result = opt_param_name.CompareTo(param_i->name());
+          ASSERT(result != 0);
+          if (result > 0) break;
+          opt_param[i + 1] = opt_param[i];
+          opt_param_position[i + 1] = opt_param_position[i];
+        }
+        opt_param[i + 1] = parameter;
+        opt_param_position[i + 1] = pos;
       }
-      opt_param[i + 1] = parameter;
-      opt_param_position[i + 1] = pos;
+      // Generate code handling each optional parameter in alphabetical order.
+      // Total number of args is the first Smi in args descriptor array (EDX).
+      __ movl(EBX, FieldAddress(EDX, Array::data_offset()));
+      // Number of positional args is the second Smi in descriptor array (EDX).
+      __ movl(ECX, FieldAddress(EDX, Array::data_offset() + (1 * kWordSize)));
+      __ SmiUntag(ECX);
+      // Let EBX point to the first passed argument, i.e. to fp[1 + argc - 0].
+      __ leal(EBX, Address(EBP, EBX, TIMES_2, kWordSize));
+      // Let EDI point to the name/pos pair of the first named argument.
+      __ leal(EDI, FieldAddress(EDX, Array::data_offset() + (2 * kWordSize)));
+      for (int i = 0; i < num_opt_params; i++) {
+        // Handle this optional parameter only if k or fewer positional args
+        // have been passed, where k is the position of this optional parameter
+        // in the formal parameter list.
+        Label load_default_value, assign_optional_parameter, next_parameter;
+        const int param_pos = opt_param_position[i];
+        __ cmpl(ECX, Immediate(param_pos));
+        __ j(GREATER, &next_parameter, Assembler::kNearJump);
+        // Check if this named parameter was passed in.
+        __ movl(EAX, Address(EDI, 0));  // Load EAX with the name of the arg.
+        __ CompareObject(EAX, opt_param[i]->name());
+        __ j(NOT_EQUAL, &load_default_value, Assembler::kNearJump);
+        // Load EAX with passed-in argument at provided arg_pos, i.e. at
+        // fp[1 + argc - arg_pos].
+        __ movl(EAX, Address(EDI, kWordSize));  // EAX is arg_pos as Smi.
+        __ addl(EDI, Immediate(2 * kWordSize));  // Point to next name/pos pair.
+        __ negl(EAX);
+        Address argument_addr(EBX, EAX, TIMES_2, 0);  // EAX is a negative Smi.
+        __ movl(EAX, argument_addr);
+        __ jmp(&assign_optional_parameter, Assembler::kNearJump);
+        __ Bind(&load_default_value);
+        // Load EAX with default argument at pos.
+        const Object& value = Object::ZoneHandle(
+            parsed_function_.default_parameter_values().At(
+                param_pos - num_fixed_params));
+        __ LoadObject(EAX, value);
+        __ Bind(&assign_optional_parameter);
+        // Assign EAX to fp[ParsedFunction::kFirstLocalSlotIndex - param_pos].
+        // We do not use the final allocation index of the variable here, i.e.
+        // scope->VariableAt(i)->index(), because captured variables still need
+        // to be copied to the context that is not yet allocated.
+        intptr_t computed_param_pos = (ParsedFunction::kFirstLocalSlotIndex -
+                                       param_pos + implicit_this_param_pos);
+        const Address param_addr(EBP, (computed_param_pos * kWordSize));
+        __ movl(param_addr, EAX);
+        __ Bind(&next_parameter);
+      }
+      delete[] opt_param;
+      delete[] opt_param_position;
+      // Check that EDI now points to null terminator in the array descriptor.
+      __ cmpl(Address(EDI, 0), raw_null);
+      __ j(EQUAL, &all_arguments_processed, Assembler::kNearJump);
+    } else {
+      ASSERT(is_implicit_native_closure);
+      __ jmp(&all_arguments_processed, Assembler::kNearJump);
     }
-    // Generate code handling each optional parameter in alphabetical order.
-    // Total number of args is the first Smi in args descriptor array (EDX).
-    __ movl(EBX, FieldAddress(EDX, Array::data_offset()));
-    // Number of positional args is the second Smi in descriptor array (EDX).
-    __ movl(ECX, FieldAddress(EDX, Array::data_offset() + (1 * kWordSize)));
-    __ SmiUntag(ECX);
-    // Let EBX point to the first passed argument, i.e. to fp[1 + argc - 0].
-    __ leal(EBX, Address(EBP, EBX, TIMES_2, kWordSize));
-    // Let EDI point to the name/pos pair of the first named argument.
-    __ leal(EDI, FieldAddress(EDX, Array::data_offset() + (2 * kWordSize)));
-    for (int i = 0; i < num_opt_params; i++) {
-      // Handle this optional parameter only if k or fewer positional arguments
-      // have been passed, where k is the position of this optional parameter in
-      // the formal parameter list.
-      Label load_default_value, assign_optional_parameter, next_parameter;
-      const int param_pos = opt_param_position[i];
-      __ cmpl(ECX, Immediate(param_pos));
-      __ j(GREATER, &next_parameter, Assembler::kNearJump);
-      // Check if this named parameter was passed in.
-      __ movl(EAX, Address(EDI, 0));  // Load EAX with the name of the argument.
-      __ CompareObject(EAX, opt_param[i]->name());
-      __ j(NOT_EQUAL, &load_default_value, Assembler::kNearJump);
-      // Load EAX with passed-in argument at provided arg_pos, i.e. at
-      // fp[1 + argc - arg_pos].
-      __ movl(EAX, Address(EDI, kWordSize));  // EAX is arg_pos as Smi.
-      __ addl(EDI, Immediate(2 * kWordSize));  // Point to next name/pos pair.
-      __ negl(EAX);
-      Address argument_addr(EBX, EAX, TIMES_2, 0);  // EAX is a negative Smi.
-      __ movl(EAX, argument_addr);
-      __ jmp(&assign_optional_parameter, Assembler::kNearJump);
-      __ Bind(&load_default_value);
-      // Load EAX with default argument at pos.
-      const Object& value = Object::ZoneHandle(
-          parsed_function_.default_parameter_values().At(
-              param_pos - num_fixed_params));
-      __ LoadObject(EAX, value);
-      __ Bind(&assign_optional_parameter);
-      // Assign EAX to fp[ParsedFunction::kFirstLocalSlotIndex - param_pos].
-      // We do not use the final allocation index of the variable here, i.e.
-      // scope->VariableAt(i)->index(), because captured variables still need
-      // to be copied to the context that is not yet allocated.
-      const Address param_addr(
-          EBP, (ParsedFunction::kFirstLocalSlotIndex - param_pos) * kWordSize);
-      __ movl(param_addr, EAX);
-      __ Bind(&next_parameter);
-    }
-    delete[] opt_param;
-    delete[] opt_param_position;
-    // Check that EDI now points to the null terminator in the array descriptor.
-    Label all_arguments_processed;
-    __ cmpl(Address(EDI, 0), raw_null);
-    __ j(EQUAL, &all_arguments_processed, Assembler::kNearJump);
 
     __ Bind(&wrong_num_arguments);
     if (locals_space_size() != 0) {
       // We need to unwind the space we reserved for locals and copied
       // parameters. The NoSuchMethodFunction stub does not expect to
       // see that area on the stack.
       __ addl(ESP, Immediate(locals_space_size()));
     }
     if (function.IsClosureFunction()) {
       GenerateCallRuntime(AstNode::kNoId,
@@ skipping 2208 matching lines @@
   }
   // It is not necessary to generate a type test of the assigned value here,
   // because the setter will check the type of its incoming arguments.
   GenerateStaticSetterCall(node->token_index(),
                            node->cls(),
                            node->field_name());
 }
 
 
 void CodeGenerator::VisitNativeBodyNode(NativeBodyNode* node) {
+  intptr_t argument_count = node->argument_count();
+  if (node->is_implicit_native_closure()) {
+    argument_count += 1;
+  }
+
   // Push the result place holder initialized to NULL.
   __ PushObject(Object::ZoneHandle());
   // Pass a pointer to the first argument in EAX.
-  if (!node->has_optional_parameters()) {
-    __ leal(EAX, Address(EBP, (1 + node->argument_count()) * kWordSize));
+  if (!node->has_optional_parameters() && !node->is_implicit_native_closure()) {
+    __ leal(EAX, Address(EBP, (1 + argument_count) * kWordSize));
   } else {
     __ leal(EAX,
             Address(EBP, ParsedFunction::kFirstLocalSlotIndex * kWordSize));
   }
   __ movl(ECX, Immediate(reinterpret_cast<uword>(node->native_c_function())));
-  __ movl(EDX, Immediate(node->argument_count()));
+  __ movl(EDX, Immediate(argument_count));
   GenerateCall(node->token_index(),
                &StubCode::CallNativeCFunctionLabel(),
                PcDescriptors::kOther);
   // Result is on the stack.
   if (!IsResultNeeded(node)) {
     __ popl(EAX);
   }
 }
 
 
@@ skipping 152 matching lines @@
   const Error& error = Error::Handle(
       Parser::FormatError(script, token_index, "Error", format, args));
   va_end(args);
   Isolate::Current()->long_jump_base()->Jump(1, error);
   UNREACHABLE();
 }
 
 }  // namespace dart
 
 #endif  // defined TARGET_ARCH_IA32