Fixed bug in stack overflow check code for IA-32 targets where a

non-tagged value in register eax was pushed to the stack.

Fixed potential quadratic behavior when converting strings to numbers.

Fixed bug where the return value from Object::SetProperty could end up
being the property holder instead of the written value.

Improved debugger support by allowing nested break points and by
dealing with stack-overflows when compiling functions before setting
break points in them.


git-svn-id: http://v8.googlecode.com/svn/trunk@4 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

ChangeLog

@@ -1,4 +1,20 @@
+2008-07-09: Version 0.1.1 (126448)
+
+        Fixed bug in stack overflow check code for IA-32 targets where a
+        non-tagged value in register eax was pushed to the stack.
+
+        Fixed potential quadratic behavior when converting strings to
+        numbers.
+
+        Fixed bug where the return value from Object::SetProperty could
+        end up being the property holder instead of the written value.
+
+        Improved debugger support by allowing nested break points and by
+        dealing with stack-overflows when compiling functions before
+        setting break points in them.
+
+
 2008-07-03: Version 0.1.0 (125876)
 
-    Initial export.
+        Initial export.
 

src/api.cc

@@ -588,9 +588,11 @@ void FunctionTemplate::Inherit(v8::Handle<FunctionTemplate> value) {
 
 void FunctionTemplate::SetInternalFieldCount(int value) {
   if (IsDeadCheck("v8::FunctionTemplate::SetInternalFieldCount()")) return;
-  ApiCheck(i::Smi::IsValid(value),
-           "v8::FunctionTemplate::SetInternalFieldCount()",
-           "Invalid internal field count");
+  if (!ApiCheck(i::Smi::IsValid(value),
+                "v8::FunctionTemplate::SetInternalFieldCount()",
+                "Invalid internal field count")) {
+    return;
+  }
   Utils::OpenHandle(this)->set_internal_field_count(i::Smi::FromInt(value));
 }
 
@@ -2042,6 +2044,11 @@ int v8::Object::InternalFieldCount() {
 Local<Value> v8::Object::GetInternal(int index) {
   if (IsDeadCheck("v8::Object::GetInternal()")) return Local<Value>();
   i::Handle<i::JSObject> obj = Utils::OpenHandle(this);
+  if (!ApiCheck(index < obj->GetInternalFieldCount(),
+                "v8::Object::GetInternal()",
+                "Reading internal field out of bounds")) {
+    return Local<Value>();
+  }
   i::Handle<i::Object> value(obj->GetInternalField(index));
   return Utils::ToLocal(value);
 }
@@ -2050,6 +2057,11 @@ Local<Value> v8::Object::GetInternal(int index) {
 void v8::Object::SetInternal(int index, v8::Handle<Value> value) {
   if (IsDeadCheck("v8::Object::SetInternal()")) return;
   i::Handle<i::JSObject> obj = Utils::OpenHandle(this);
+  if (!ApiCheck(index < obj->GetInternalFieldCount(),
+                "v8::Object::SetInternal()",
+                "Writing internal field out of bounds")) {
+    return;
+  }
   i::Handle<i::Object> val = Utils::OpenHandle(*value);
   obj->SetInternalField(index, *val);
 }

src/debug-delay.js

@@ -687,9 +687,8 @@ ExecutionState.prototype.prepareStep = function(opt_action, opt_count) {
   return %PrepareStep(this.break_id, action, count);
 }
 
-ExecutionState.prototype.evaluateGlobal = function(source) {
-  var result = %DebugEvaluateGlobal(this.break_id, source);
-  return result;
+ExecutionState.prototype.evaluateGlobal = function(source, disable_break) {
+  return %DebugEvaluateGlobal(this.break_id, source, Boolean(disable_break));
 };
 
 ExecutionState.prototype.GetFrameCount = function() {
@@ -1760,6 +1759,15 @@ DebugCommandProcessor.prototype.evaluateRequest_ = function(request, response) {
   var expression = request.arguments.expression;
   var frame = request.arguments.frame;
   var global = request.arguments.global;
+  var disable_break = request.arguments.disable_break;
+
+  // The expression argument could be an integer so we convert it to a
+  // string.
+  try {
+    expression = String(expression);
+  } catch(e) {
+    return response.failed('Failed to convert expression argument to string');
+  }
 
   // Check for legal arguments.
   if (!IS_UNDEFINED(frame) && global) {
@@ -1769,10 +1777,16 @@ DebugCommandProcessor.prototype.evaluateRequest_ = function(request, response) {
   // Global evaluate.
   if (global) {
     // Evaluate in the global context.
-    response.body = MakeMirror(this.exec_state_.evaluateGlobal(expression));
+    response.body = MakeMirror(
+        this.exec_state_.evaluateGlobal(expression), Boolean(disable_break));
     return;
   }
 
+  // Default value for disable_break is true.
+  if (IS_UNDEFINED(disable_break)) {
+    disable_break = true;
+  }
+
   // Check whether a frame was specified.
   if (!IS_UNDEFINED(frame)) {
     var frame_number = %ToNumber(frame);
@@ -1780,11 +1794,13 @@ DebugCommandProcessor.prototype.evaluateRequest_ = function(request, response) {
       return response.failed('Invalid frame "' + frame + '"');
     }
     // Evaluate in the specified frame.
-    response.body = this.exec_state_.GetFrame(frame_number).evaluate(expression);
+    response.body = this.exec_state_.GetFrame(frame_number).evaluate(
+        expression, Boolean(disable_break));
     return;
   } else {
     // Evaluate in the selected frame.
-    response.body = this.exec_state_.GetFrame().evaluate(expression);
+    response.body = this.exec_state_.GetFrame().evaluate(
+        expression, Boolean(disable_break));
     return;
   }
 };

src/debug.cc

@@ -473,6 +473,9 @@ int Debug::ArchiveSpacePerThread() {
 }
 
 
+// Default break enabled.
+bool Debug::disable_break_ = false;
+
 // Default call debugger on uncaught exception.
 bool Debug::break_on_exception_ = false;
 bool Debug::break_on_uncaught_exception_ = true;
@@ -640,17 +643,19 @@ Object* Debug::Break(Arguments args) {
   HandleScope scope;
   ASSERT(args.length() == 1);
 
-  if (!Load()) {
-    return Heap::undefined_value();
+  // Get the top-most JavaScript frame.
+  JavaScriptFrameIterator it;
+  JavaScriptFrame* frame = it.frame();
+
+  // Just continue if breaks are disabled or debugger cannot be loaded.
+  if (disable_break() || !Load()) {
+    SetAfterBreakTarget(frame);
+    return args[0];
   }
 
   SaveBreakFrame save;
   EnterDebuggerContext enter;
 
-  // Get the top-most JavaScript frame.
-  JavaScriptFrameIterator it;
-  JavaScriptFrame* frame = it.frame();
-
   // Deactivate interrupt during breakpoint processing.
   StackGuard::DisableInterrupts();
 
@@ -710,8 +715,7 @@ Object* Debug::Break(Arguments args) {
   // Install jump to the call address which was overwritten.
   SetAfterBreakTarget(frame);
 
-  // Return whatever - return value is ignored.
-  return Heap::undefined_value();
+  return args[0];
 }
 
 
@@ -788,17 +792,10 @@ bool Debug::HasDebugInfo(Handle<SharedFunctionInfo> shared) {
 }
 
 
-// Return the debug info for this function. If the function currently has no
-// debug info it will be created. The reason for having this function is that
-// the debug info member is of type Object and not DebugInfo, as it can contain
-// undefined to indicate that currently no debug info exists for the function.
+// Return the debug info for this function. EnsureDebugInfo must be called
+// prior to ensure the debug info has been generated for shared.
 Handle<DebugInfo> Debug::GetDebugInfo(Handle<SharedFunctionInfo> shared) {
-  // If the debug info does not exist create it.
-  if (!HasDebugInfo(shared)) {
-    AddDebugInfo(shared);
-  }
-
-  // Return the debug info.
+  ASSERT(HasDebugInfo(shared));
   return Handle<DebugInfo>(DebugInfo::cast(shared->debug_info()));
 }
 
@@ -806,17 +803,12 @@ Handle<DebugInfo> Debug::GetDebugInfo(Handle<SharedFunctionInfo> shared) {
 void Debug::SetBreakPoint(Handle<SharedFunctionInfo> shared,
                           int source_position,
                           Handle<Object> break_point_object) {
-  // Make sure the function is compiled before accessing code object.
-  EnsureCompiled(shared);
-
-  // Get the debug info (create it if it does not exist).
-  Handle<DebugInfo> debug_info;
-  if (shared->debug_info()->IsUndefined()) {
-    debug_info = AddDebugInfo(shared);
-  } else {
-    debug_info = Handle<DebugInfo>(DebugInfo::cast(shared->debug_info()));
+  if (!EnsureDebugInfo(shared)) {
+    // Return if retrieving debug info failed.
+    return;
   }
 
+  Handle<DebugInfo> debug_info = GetDebugInfo(shared);
   // Source positions starts with zero.
   ASSERT(source_position >= 0);
 
@@ -864,14 +856,14 @@ void Debug::ClearBreakPoint(Handle<Object> break_point_object) {
 
 
 void Debug::FloodWithOneShot(Handle<SharedFunctionInfo> shared) {
-  // Make sure the function is compiled before accessing code object.
-  EnsureCompiled(shared);
-
-  // Get the debug info.
-  Handle<DebugInfo> debug_info = GetDebugInfo(shared);
+  // Make sure the function has setup the debug info.
+  if (!EnsureDebugInfo(shared)) {
+    // Return if we failed to retrieve the debug info.
+    return;
+  }
 
   // Flood the function with break points.
-  BreakLocationIterator it(debug_info, ALL_BREAK_LOCATIONS);
+  BreakLocationIterator it(GetDebugInfo(shared), ALL_BREAK_LOCATIONS);
   while (!it.Done()) {
     it.SetOneShot();
     it.Next();
@@ -940,6 +932,10 @@ void Debug::PrepareStep(StepAction step_action, int step_count) {
   // Get the debug info (create it if it does not exist).
   Handle<SharedFunctionInfo> shared =
       Handle<SharedFunctionInfo>(JSFunction::cast(frame->function())->shared());
+  if (!EnsureDebugInfo(shared)) {
+    // Return if ensuring debug info failed.
+    return;
+  }
   Handle<DebugInfo> debug_info = GetDebugInfo(shared);
 
   // Find the break location where execution has stopped.
@@ -1164,23 +1160,19 @@ void Debug::ClearStepNext() {
 }
 
 
-void Debug::EnsureCompiled(Handle<SharedFunctionInfo> shared) {
-  if (!shared->is_compiled()) {
-    // TODO(1240742): We need to handle stack-overflow exceptions
-    // here. It might make sense to add a boolean return value to
-    // EnsureCompiled to indicate whether or not the compilation
-    // succeeded.
-    CompileLazyShared(shared, KEEP_EXCEPTION);
-  }
-  ASSERT(shared->is_compiled());
+bool Debug::EnsureCompiled(Handle<SharedFunctionInfo> shared) {
+  if (shared->is_compiled()) return true;
+  return CompileLazyShared(shared, CLEAR_EXCEPTION);
 }
 
 
-Handle<DebugInfo> Debug::AddDebugInfo(Handle<SharedFunctionInfo> shared) {
-  ASSERT(!HasDebugInfo(shared));
+// Ensures the debug information is present for shared.
+bool Debug::EnsureDebugInfo(Handle<SharedFunctionInfo> shared) {
+  // Return if we already have the debug info for shared.
+  if (HasDebugInfo(shared)) return true;
 
-  // Make sure that the function is compiled.
-  EnsureCompiled(shared);
+  // Ensure shared in compiled. Return false if this failed.
+  if (!EnsureCompiled(shared)) return false;
 
   // Create the debug info object.
   Handle<DebugInfo> debug_info =
@@ -1210,7 +1202,7 @@ Handle<DebugInfo> Debug::AddDebugInfo(Handle<SharedFunctionInfo> shared) {
   // Now there is at least one break point.
   has_break_points_ = true;
 
-  return debug_info;
+  return true;
 }
 
 
@@ -1248,6 +1240,10 @@ void Debug::SetAfterBreakTarget(JavaScriptFrame* frame) {
   // Get the executing function in which the debug break occurred.
   Handle<SharedFunctionInfo> shared =
       Handle<SharedFunctionInfo>(JSFunction::cast(frame->function())->shared());
+  if (!EnsureDebugInfo(shared)) {
+    // Return if we failed to retrieve the debug info.
+    return;
+  }
   Handle<DebugInfo> debug_info = GetDebugInfo(shared);
   Handle<Code> code(debug_info->code());
   Handle<Code> original_code(debug_info->original_code());

src/debug.h

@@ -175,6 +175,10 @@ class Debug {
                                JavaScriptFrame* frame);
   static Handle<DebugInfo> GetDebugInfo(Handle<SharedFunctionInfo> shared);
   static bool HasDebugInfo(Handle<SharedFunctionInfo> shared);
+
+  // Returns whether the operation succedded.
+  static bool EnsureDebugInfo(Handle<SharedFunctionInfo> shared);
+
   static bool IsDebugBreak(Address addr);
 
   // Check whether a code stub with the specified major key is a possible break
@@ -202,6 +206,12 @@ class Debug {
   static Address step_in_fp() { return thread_local_.step_into_fp_; }
   static Address* step_in_fp_addr() { return &thread_local_.step_into_fp_; }
 
+  // Getter and setter for the disable break state.
+  static bool disable_break() { return disable_break_; }
+  static void set_disable_break(bool disable_break) {
+    disable_break_ = disable_break;
+  }
+
   // Getters for the current exception break state.
   static bool break_on_exception() { return break_on_exception_; }
   static bool break_on_uncaught_exception() {
@@ -255,8 +265,8 @@ class Debug {
   static void ActivateStepIn(StackFrame* frame);
   static void ClearStepIn();
   static void ClearStepNext();
-  static void EnsureCompiled(Handle<SharedFunctionInfo> shared);
-  static Handle<DebugInfo> AddDebugInfo(Handle<SharedFunctionInfo> shared);
+  // Returns whether the compile succedded.
+  static bool EnsureCompiled(Handle<SharedFunctionInfo> shared);
   static void RemoveDebugInfo(Handle<DebugInfo> debug_info);
   static void SetAfterBreakTarget(JavaScriptFrame* frame);
   static Handle<Object> CheckBreakPoints(Handle<Object> break_point);
@@ -270,6 +280,7 @@ class Debug {
   static bool has_break_points_;
   static DebugInfoListNode* debug_info_list_;
 
+  static bool disable_break_;
   static bool break_on_exception_;
   static bool break_on_uncaught_exception_;
 
@@ -484,6 +495,26 @@ class EnterDebuggerContext BASE_EMBEDDED {
 };
 
 
+// Stack allocated class for disabling break.
+class DisableBreak BASE_EMBEDDED {
+ public:
+  // Enter the debugger by storing the previous top context and setting the
+  // current top context to the debugger context.
+  explicit DisableBreak(bool disable_break)  {
+    prev_disable_break_ = Debug::disable_break();
+    Debug::set_disable_break(disable_break);
+  }
+  ~DisableBreak() {
+    Debug::set_disable_break(prev_disable_break_);
+  }
+
+ private:
+  // The previous state of the disable break used to restore the value when this
+  // object is destructed.
+  bool prev_disable_break_;
+};
+
+
 // Debug_Address encapsulates the Address pointers used in generating debug
 // code.
 class Debug_Address {

src/mirror-delay.js

@@ -1548,8 +1548,9 @@ FrameMirror.prototype.sourceLineText = function() {
 };
 
 
-FrameMirror.prototype.evaluate = function(source) {
-  var result = %DebugEvaluate(this.break_id_, this.details_.frameId(), source);
+FrameMirror.prototype.evaluate = function(source, disable_break) {
+  var result = %DebugEvaluate(this.break_id_, this.details_.frameId(),
+                              source, Boolean(disable_break));
   return MakeMirror(result);
 };
 

src/objects.cc

@@ -1486,7 +1486,7 @@ Object* JSObject::SetProperty(LookupResult* result,
           return AddFastProperty(name, value, attributes);
         }
       case CONSTANT_FUNCTION:
-        if (value == result->GetConstantFunction()) return this;
+        if (value == result->GetConstantFunction()) return value;
         // Only replace the function if necessary.
         return ReplaceConstantFunctionProperty(name, value);
       case CALLBACKS:

src/runtime.cc

@@ -61,6 +61,13 @@ namespace v8 { namespace internal {
   RUNTIME_ASSERT(args[index]->Is##Type());                           \
   Handle<Type> name = args.at<Type>(index);
 
+// Cast the given object to a boolean and store it in a variable with
+// the given name.  If the object is not a boolean call IllegalOperation
+// and return.
+#define CONVERT_BOOLEAN_CHECKED(name, obj)                            \
+  RUNTIME_ASSERT(obj->IsBoolean());                                   \
+  bool name = (obj)->IsTrue();
+
 // Cast the given object to a double and store it in a variable with
 // the given name.  If the object is not a number (as opposed to
 // the number not-a-number) call IllegalOperation and return.
@@ -956,8 +963,9 @@ static Object* Runtime_StringLastIndexOf(Arguments args) {
   uint32_t pattern_length = pat->length();
   uint32_t sub_length = sub->length();
 
-  if (start_index + pattern_length > sub_length)
+  if (start_index + pattern_length > sub_length) {
     start_index = sub_length - pattern_length;
+  }
 
   for (int i = start_index; i >= 0; i--) {
     bool found = true;
@@ -1553,6 +1561,7 @@ static Object* Runtime_StringToNumber(Arguments args) {
   NoHandleAllocation ha;
   ASSERT(args.length() == 1);
   CONVERT_CHECKED(String, subject, args[0]);
+  subject->TryFlatten();
   return Heap::NumberFromDouble(StringToDouble(subject, ALLOW_HEX));
 }
 
@@ -2979,6 +2988,11 @@ static Object* RuntimePreempt(Arguments args) {
 
 
 static Object* Runtime_DebugBreak(Arguments args) {
+  // Just continue if breaks are disabled.
+  if (Debug::disable_break()) {
+    return args[0];
+  }
+
   // Don't break in system functions. If the current function is either in the
   // builtins object of some context or is in the debug context just return with
   // the debug break stack guard active.
@@ -4356,11 +4370,15 @@ static Object* Runtime_DebugEvaluate(Arguments args) {
 
   // Check the execution state and decode arguments frame and source to be
   // evaluated.
-  ASSERT(args.length() == 3);
+  ASSERT(args.length() == 4);
   Object* check_result = Runtime_CheckExecutionState(args);
   if (check_result->IsFailure()) return check_result;
   CONVERT_CHECKED(Smi, wrapped_id, args[1]);
   CONVERT_ARG_CHECKED(String, source, 2);
+  CONVERT_BOOLEAN_CHECKED(disable_break, args[3]);
+
+  // Handle the processing of break.
+  DisableBreak disable_break_save(disable_break);
 
   // Get the frame where the debugging is performed.
   StackFrame::Id id = UnwrapFrameId(wrapped_id);
@@ -4489,10 +4507,14 @@ static Object* Runtime_DebugEvaluateGlobal(Arguments args) {
 
   // Check the execution state and decode arguments frame and source to be
   // evaluated.
-  ASSERT(args.length() == 2);
+  ASSERT(args.length() == 3);
   Object* check_result = Runtime_CheckExecutionState(args);
   if (check_result->IsFailure()) return check_result;
   CONVERT_ARG_CHECKED(String, source, 1);
+  CONVERT_BOOLEAN_CHECKED(disable_break, args[2]);
+
+  // Handle the processing of break.
+  DisableBreak disable_break_save(disable_break);
 
   // Enter the top context from before the debugger was invoked.
   SaveContext save;

src/runtime.h

@@ -233,8 +233,8 @@ namespace v8 { namespace internal {
   F(ChangeBreakOnException, 2) \
   F(PrepareStep, 3) \
   F(ClearStepping, 1) \
-  F(DebugEvaluate, 3) \
-  F(DebugEvaluateGlobal, 2) \
+  F(DebugEvaluate, 4) \
+  F(DebugEvaluateGlobal, 3) \
   F(DebugGetLoadedScripts, 1) \
   F(DebugReferencedBy, 3) \
   F(DebugConstructedBy, 2) \