Skip to content

Commit

Permalink
Merge r236022 - Don't dump OSRAvailabilityData in Graph::dump because…
Browse files Browse the repository at this point in the history
… a stale Availability may point to a Node that is already freed

https://bugs.webkit.org/show_bug.cgi?id=189628
<rdar://problem/39481690>

Reviewed by Mark Lam.

JSTests:

* stress/verbose-failure-dont-graph-dump-availability-already-freed.js: Added.
(foo):

Source/JavaScriptCore:

An Availability may point to a Node. And that Node may be removed from
the graph, e.g, it's freed and its memory is no longer owned by Graph.
This patch makes it so we no longer dump this metadata by default. If
this metadata is interesting to you, you'll need to go in and change
Graph::dump to dump the needed metadata.

* dfg/DFGGraph.cpp:
(JSC::DFG::Graph::dump):
  • Loading branch information
Saam Barati authored and carlosgcampos committed Sep 19, 2018
1 parent f0bab6d commit c8f3f5a
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 2 deletions.
11 changes: 11 additions & 0 deletions JSTests/ChangeLog
@@ -1,3 +1,14 @@
2018-09-14 Saam barati <sbarati@apple.com>

Don't dump OSRAvailabilityData in Graph::dump because a stale Availability may point to a Node that is already freed
https://bugs.webkit.org/show_bug.cgi?id=189628
<rdar://problem/39481690>

Reviewed by Mark Lam.

* stress/verbose-failure-dont-graph-dump-availability-already-freed.js: Added.
(foo):

2018-09-07 Mark Lam <mark.lam@apple.com>

Ensure that handleIntrinsicCall() is only applied on op_call shaped instructions.
Expand Down
@@ -0,0 +1,9 @@
//@ runDefault("--verboseValidationFailure=true")

function foo() {
arguments.length;
}
let a = 0;
for (var i = 0; i < 1000000; i++) {
a += foo();
}
17 changes: 17 additions & 0 deletions Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,20 @@
2018-09-14 Saam barati <sbarati@apple.com>

Don't dump OSRAvailabilityData in Graph::dump because a stale Availability may point to a Node that is already freed
https://bugs.webkit.org/show_bug.cgi?id=189628
<rdar://problem/39481690>

Reviewed by Mark Lam.

An Availability may point to a Node. And that Node may be removed from
the graph, e.g, it's freed and its memory is no longer owned by Graph.
This patch makes it so we no longer dump this metadata by default. If
this metadata is interesting to you, you'll need to go in and change
Graph::dump to dump the needed metadata.

* dfg/DFGGraph.cpp:
(JSC::DFG::Graph::dump):

2018-09-07 Mark Lam <mark.lam@apple.com>

Ensure that handleIntrinsicCall() is only applied on op_call shaped instructions.
Expand Down
8 changes: 6 additions & 2 deletions Source/JavaScriptCore/dfg/DFGGraph.cpp
Expand Up @@ -60,6 +60,8 @@

namespace JSC { namespace DFG {

static constexpr bool dumpOSRAvailabilityData = false;

// Creates an array of stringized names.
static const char* dfgOpNames[] = {
#define STRINGIZE_DFG_OP_ENUM(opcode, flags) #opcode ,
Expand Down Expand Up @@ -569,7 +571,8 @@ void Graph::dump(PrintStream& out, DumpContext* context)

case SSA: {
RELEASE_ASSERT(block->ssa);
out.print(" Availability: ", block->ssa->availabilityAtHead, "\n");
if (dumpOSRAvailabilityData)
out.print(" Availability: ", block->ssa->availabilityAtHead, "\n");
out.print(" Live: ", nodeListDump(block->ssa->liveAtHead), "\n");
out.print(" Values: ", nodeValuePairListDump(block->ssa->valuesAtHead, context), "\n");
break;
Expand Down Expand Up @@ -597,7 +600,8 @@ void Graph::dump(PrintStream& out, DumpContext* context)

case SSA: {
RELEASE_ASSERT(block->ssa);
out.print(" Availability: ", block->ssa->availabilityAtTail, "\n");
if (dumpOSRAvailabilityData)
out.print(" Availability: ", block->ssa->availabilityAtTail, "\n");
out.print(" Live: ", nodeListDump(block->ssa->liveAtTail), "\n");
out.print(" Values: ", nodeValuePairListDump(block->ssa->valuesAtTail, context), "\n");
break;
Expand Down

0 comments on commit c8f3f5a

Please sign in to comment.