WebKit Bugzilla
Attachment 359701 Details for
Bug 193652
: DoesGC rule is wrong for nodes with BigIntUse
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch
bug-193652-20190121165833.patch (text/plain), 4.84 KB, created by
Caio Lima
on 2019-01-21 11:58:35 PST
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
Caio Lima
Created:
2019-01-21 11:58:35 PST
Size:
4.84 KB
patch
obsolete
>Subversion Revision: 240235 >diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog >index dbc7d46ad189e0e9407da5935177173aa925c0b8..152381058b8ba0b27c4de4d8242fa724d2bc356d 100644 >--- a/Source/JavaScriptCore/ChangeLog >+++ b/Source/JavaScriptCore/ChangeLog >@@ -1,3 +1,20 @@ >+2019-01-21 Caio Lima <ticaiolima@gmail.com> >+ >+ DoesGC rule is wrong for nodes with BigIntUse >+ https://bugs.webkit.org/show_bug.cgi?id=193652 >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ Former rule was that ValueOp does not GC. However this is wrong, since >+ these operations can trigger GC and mess up memory management. In the end, this >+ will generate wrong code because we will have wrong GC epoch value during >+ Store Barrier Insertion phase. >+ We changed this to consider BigIntUse for such nodes and properly return true when >+ they are BigIntUse. >+ >+ * dfg/DFGDoesGC.cpp: >+ (JSC::DFG::doesGC): >+ > 2019-01-20 Saam Barati <sbarati@apple.com> > > DFG: When inlining DataView set* intrinsics we need to set undefined as our result >diff --git a/Source/JavaScriptCore/dfg/DFGDoesGC.cpp b/Source/JavaScriptCore/dfg/DFGDoesGC.cpp >index 4f79af6c4cc5c1d9a9581e712441ad703994cb2d..548f759c01ceafbb6a3add628816ec391e13ed58 100644 >--- a/Source/JavaScriptCore/dfg/DFGDoesGC.cpp >+++ b/Source/JavaScriptCore/dfg/DFGDoesGC.cpp >@@ -97,14 +97,7 @@ bool doesGC(Graph& graph, Node* node) > case ArithTrunc: > case ArithFRound: > case ArithUnary: >- case ValueBitAnd: >- case ValueBitOr: >- case ValueBitXor: >- case ValueAdd: >- case ValueSub: >- case ValueMul: > case ValueNegate: >- case ValueDiv: > case TryGetById: > case GetById: > case GetByIdFlush: >@@ -385,6 +378,15 @@ bool doesGC(Graph& graph, Node* node) > case MapSet: > return true; > >+ case ValueBitAnd: >+ case ValueBitOr: >+ case ValueBitXor: >+ case ValueAdd: >+ case ValueSub: >+ case ValueMul: >+ case ValueDiv: >+ return node->binaryUseKind() == BigIntUse; >+ > case GetIndexedPropertyStorage: > if (node->arrayMode().type() == Array::String) > return true; >diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog >index 9dcfb8a3ba6b7f49a5e06a59083524d42e253358..0ea964fe843c6edbb4154e8eb7eb8350eb055bb3 100644 >--- a/JSTests/ChangeLog >+++ b/JSTests/ChangeLog >@@ -1,3 +1,20 @@ >+2019-01-21 Caio Lima <ticaiolima@gmail.com> >+ >+ DoesGC rule is wrong for nodes with BigIntUse >+ https://bugs.webkit.org/show_bug.cgi?id=193652 >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ * stress/big-int-value-op-update-gc-rules.js: Added. >+ (assert): >+ (doesGCAdd): >+ (doesGCSub): >+ (doesGCDiv): >+ (doesGCMul): >+ (doesGCBitAnd): >+ (doesGCBitOr): >+ (doesGCBitXor): >+ > 2019-01-20 Saam Barati <sbarati@apple.com> > > DFG: When inlining DataView set* intrinsics we need to set undefined as our result >diff --git a/JSTests/stress/big-int-value-op-update-gc-rules.js b/JSTests/stress/big-int-value-op-update-gc-rules.js >new file mode 100644 >index 0000000000000000000000000000000000000000..56e526f7e1103eafa01e82ce9e724d23cf142e77 >--- /dev/null >+++ b/JSTests/stress/big-int-value-op-update-gc-rules.js >@@ -0,0 +1,105 @@ >+//@ runBigIntEnabled >+ >+function assert(a, e) { >+ if (a !== e) >+ throw new Error("Expected: " + e + " but got: " + a); >+} >+ >+function doesGCAdd(a) { >+ let o = {}; >+ let c = a + 1n; >+ o.b = c; >+ >+ return o; >+} >+noInline(doesGCAdd); >+ >+for (var i = 0; i < 10000; i++) { >+ let o = doesGCAdd(3n); >+ assert(o.b, 4n); >+} >+ >+function doesGCSub(a) { >+ let o = {}; >+ let c = a - 1n; >+ o.b = c; >+ >+ return o; >+} >+noInline(doesGCSub); >+ >+for (var i = 0; i < 10000; i++) { >+ let o = doesGCSub(3n); >+ assert(o.b, 2n); >+} >+ >+function doesGCDiv(a) { >+ let o = {}; >+ let c = a / 2n; >+ o.b = c; >+ >+ return o; >+} >+noInline(doesGCDiv); >+ >+for (var i = 0; i < 10000; i++) { >+ let o = doesGCDiv(4n); >+ assert(o.b, 2n); >+} >+ >+function doesGCMul(a) { >+ let o = {}; >+ let c = a * 2n; >+ o.b = c; >+ >+ return o; >+} >+noInline(doesGCMul); >+ >+for (var i = 0; i < 10000; i++) { >+ let o = doesGCMul(4n); >+ assert(o.b, 8n); >+} >+ >+function doesGCBitAnd(a) { >+ let o = {}; >+ let c = a & 0b11n; >+ o.b = c; >+ >+ return o; >+} >+noInline(doesGCBitAnd); >+ >+for (var i = 0; i < 10000; i++) { >+ let o = doesGCBitAnd(0b1010n); >+ assert(o.b, 0b10n); >+} >+ >+function doesGCBitOr(a) { >+ let o = {}; >+ let c = a | 0b11n; >+ o.b = c; >+ >+ return o; >+} >+noInline(doesGCBitOr); >+ >+for (var i = 0; i < 10000; i++) { >+ let o = doesGCBitOr(0b10n); >+ assert(o.b, 0b11n); >+} >+ >+function doesGCBitXor(a) { >+ let o = {}; >+ let c = a ^ 0b11n; >+ o.b = c; >+ >+ return o; >+} >+noInline(doesGCBitXor); >+ >+for (var i = 0; i < 10000; i++) { >+ let o = doesGCBitXor(0b10n); >+ assert(o.b, 0b1n); >+} >+
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 193652
:
359701
|
359704
|
359705