Skip to content

Commit

Permalink
Revert of Revert of Debugger: deduplicate shared function info when s…
Browse files Browse the repository at this point in the history
…etting script break points. (patchset nwjs#1 id:1 of https://codereview.chromium.org/999273003/)

Reason for revert:
Reland since the failure has been fixed in https://codereview.chromium.org/1035523005/

Original issue's description:
> Revert of Debugger: deduplicate shared function info when setting script break points. (patchset nwjs#4 id:60001 of https://codereview.chromium.org/998253005/)
>
> Reason for revert:
> Code caching failures.
>
> Original issue's description:
> > Debugger: deduplicate shared function info when setting script break points.
> >
> > Also fix Debug.showBreakPoints for multiple break points at the same location.
> >
> > BUG=v8:3960
> > LOG=N
> >
> > Committed: https://crrev.com/73b17a71a22564c0b66d9aa7c00948c748f5b290
> > Cr-Commit-Position: refs/heads/master@{#27444}
>
> TBR=mstarzinger@chromium.org
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=v8:3960
>
> Committed: https://crrev.com/9b29d008dfcc00bf56be8040add1d2c5e404673b
> Cr-Commit-Position: refs/heads/master@{#27448}

TBR=mstarzinger@chromium.org
BUG=v8:3960
LOG=N

Review URL: https://codereview.chromium.org/1037013002

Cr-Commit-Position: refs/heads/master@{#27472}
  • Loading branch information
hashseed authored and Commit bot committed Mar 26, 2015
1 parent ed91912 commit 69383d6
Show file tree
Hide file tree
Showing 8 changed files with 77 additions and 18 deletions.
2 changes: 1 addition & 1 deletion src/compiler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1428,7 +1428,7 @@ Handle<SharedFunctionInfo> Compiler::BuildFunctionInfo(
result->set_is_toplevel(false);

RecordFunctionCompilation(Logger::FUNCTION_TAG, &info, result);
result->set_allows_lazy_compilation(allow_lazy);
result->set_allows_lazy_compilation(literal->AllowsLazyCompilation());
result->set_allows_lazy_compilation_without_context(allow_lazy_without_ctx);

// Set the expected number of properties for instances and return
Expand Down
41 changes: 27 additions & 14 deletions src/debug.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1506,23 +1506,22 @@ Handle<Object> Debug::GetSourceBreakLocations(
Handle<FixedArray> locations =
isolate->factory()->NewFixedArray(debug_info->GetBreakPointCount());
int count = 0;
for (int i = 0; i < debug_info->break_points()->length(); i++) {
for (int i = 0; i < debug_info->break_points()->length(); ++i) {
if (!debug_info->break_points()->get(i)->IsUndefined()) {
BreakPointInfo* break_point_info =
BreakPointInfo::cast(debug_info->break_points()->get(i));
if (break_point_info->GetBreakPointCount() > 0) {
Smi* position = NULL;
switch (position_alignment) {
case STATEMENT_ALIGNED:
position = break_point_info->statement_position();
break;
case BREAK_POSITION_ALIGNED:
position = break_point_info->source_position();
break;
}

locations->set(count++, position);
int break_points = break_point_info->GetBreakPointCount();
if (break_points == 0) continue;
Smi* position = NULL;
switch (position_alignment) {
case STATEMENT_ALIGNED:
position = break_point_info->statement_position();
break;
case BREAK_POSITION_ALIGNED:
position = break_point_info->source_position();
break;
}
for (int j = 0; j < break_points; ++j) locations->set(count++, position);
}
}
return locations;
Expand Down Expand Up @@ -1822,7 +1821,6 @@ static void RecompileAndRelocateSuspendedGenerators(
static bool SkipSharedFunctionInfo(SharedFunctionInfo* shared,
Object* active_code_marker) {
if (!shared->allows_lazy_compilation()) return true;
if (!shared->script()->IsScript()) return true;
Object* script = shared->script();
if (!script->IsScript()) return true;
if (Script::cast(script)->type()->value() == Script::TYPE_NATIVE) return true;
Expand Down Expand Up @@ -2103,6 +2101,21 @@ Handle<Object> Debug::FindSharedFunctionInfoInScript(Handle<Script> script,
}
} // End while loop.

// JSFunctions from the same literal may not have the same shared function
// info. Find those JSFunctions and deduplicate the shared function info.
HeapIterator iterator(heap, FLAG_lazy ? HeapIterator::kNoFiltering
: HeapIterator::kFilterUnreachable);
for (HeapObject* obj = iterator.next(); obj != NULL; obj = iterator.next()) {
if (!obj->IsJSFunction()) continue;
JSFunction* function = JSFunction::cast(obj);
SharedFunctionInfo* shared = function->shared();
if (shared != *target && shared->script() == target->script() &&
shared->start_position_and_type() ==
target->start_position_and_type()) {
function->set_shared(*target);
}
}

return target;
}

Expand Down
1 change: 1 addition & 0 deletions src/liveedit.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1222,6 +1222,7 @@ void LiveEdit::SetFunctionScript(Handle<JSValue> function_wrapper,
UnwrapSharedFunctionInfoFromJSValue(function_wrapper);
CHECK(script_handle->IsScript() || script_handle->IsUndefined());
shared_info->set_script(*script_handle);
shared_info->DisableOptimization(kLiveEdit);

function_wrapper->GetIsolate()->compilation_cache()->Remove(shared_info);
}
Expand Down
9 changes: 9 additions & 0 deletions src/runtime/runtime-debug.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2646,6 +2646,15 @@ RUNTIME_FUNCTION(Runtime_ExecuteInDebugContext) {
}


RUNTIME_FUNCTION(Runtime_GetDebugContext) {
HandleScope scope(isolate);
DCHECK(args.length() == 0);
Handle<Context> context = isolate->debug()->GetDebugContext();
context->set_security_token(isolate->native_context()->security_token());
return context->global_proxy();
}


// Performs a GC.
// Presently, it only does a full GC.
RUNTIME_FUNCTION(Runtime_CollectGarbage) {
Expand Down
2 changes: 1 addition & 1 deletion src/runtime/runtime.h
Original file line number Diff line number Diff line change
Expand Up @@ -577,7 +577,7 @@ namespace internal {
F(LiveEditRestartFrame, 2, 1) \
F(GetFunctionCodePositionFromSource, 2, 1) \
F(ExecuteInDebugContext, 1, 1) \
\
F(GetDebugContext, 0, 1) \
F(SetFlags, 1, 1) \
F(CollectGarbage, 1, 1) \
F(GetHeapUsage, 0, 1)
Expand Down
2 changes: 1 addition & 1 deletion test/mjsunit/debug-liveedit-2.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
// (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: --expose-debug-as debug
// Flags: --expose-debug-as debug --noalways-opt
// Get the Debug object exposed from the debug context global object.


Expand Down
2 changes: 1 addition & 1 deletion test/mjsunit/debug-liveedit-4.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
// (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: --expose-debug-as debug
// Flags: --expose-debug-as debug --noalways-opt
// Get the Debug object exposed from the debug context global object.

// In this test case we edit a script so that techincally function text
Expand Down
36 changes: 36 additions & 0 deletions test/mjsunit/regress/regress-3960.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// Copyright 2015 the V8 project authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

// Flags: --allow-natives-syntax

// Test that setting break point is works correctly when the debugger is
// activated late, which leads to duplicate shared function infos.

(function() {
var Debug = %GetDebugContext().Debug;

function listener(event, exec_state, event_data, data) {
if (event != Debug.DebugEvent.Break) return;
try {
assertTrue(/foo/.test(exec_state.frame(0).sourceLineText()));
break_count++;
} catch (e) {
exception = e;
}
}

for (var i = 0; i < 3; i++) {
var foo = function() { a = 1; }
var exception = null;
var break_count = 0;
Debug.setListener(listener);
if (i < 2) Debug.setBreakPoint(foo, 0, 0);
assertTrue(/\[B\d\]a = 1/.test(Debug.showBreakPoints(foo)));
foo();
assertEquals(1, break_count);
assertNull(exception);
}

Debug.setListener(null);
})();

0 comments on commit 69383d6

Please sign in to comment.