RESOLVED FIXED 151839
[ES6] Add support for Symbol.hasInstance
https://bugs.webkit.org/show_bug.cgi?id=151839
Summary [ES6] Add support for Symbol.hasInstance
Keith Miller
Reported 2015-12-03 17:36:26 PST
[ES6] Add support for Symbol.hasInstance
Attachments
Patch (88.89 KB, patch)
2015-12-03 18:57 PST, Keith Miller
no flags
Benchmark results (63.03 KB, text/plain)
2015-12-03 19:12 PST, Keith Miller
no flags
Patch (116.42 KB, patch)
2015-12-07 15:55 PST, Keith Miller
no flags
Patch (111.83 KB, patch)
2015-12-07 16:26 PST, Keith Miller
no flags
Patch (115.31 KB, patch)
2015-12-07 16:48 PST, Keith Miller
no flags
Patch (109.99 KB, patch)
2015-12-08 12:32 PST, Keith Miller
no flags
Patch (110.04 KB, patch)
2015-12-09 17:33 PST, Keith Miller
saam: review+
Benchmark results (62.75 KB, text/plain)
2015-12-09 19:30 PST, Keith Miller
no flags
Patch (115.75 KB, patch)
2015-12-09 20:21 PST, Keith Miller
no flags
Patch (115.25 KB, patch)
2015-12-10 17:20 PST, Keith Miller
no flags
Patch (120.33 KB, patch)
2015-12-17 13:26 PST, Keith Miller
saam: review+
Keith Miller
Comment 1 2015-12-03 18:57:51 PST
Keith Miller
Comment 2 2015-12-03 19:12:21 PST
Created attachment 266588 [details] Benchmark results 64-bit benchmark results.
Keith Miller
Comment 3 2015-12-07 15:55:15 PST
Keith Miller
Comment 4 2015-12-07 16:26:27 PST
WebKit Commit Bot
Comment 5 2015-12-07 16:27:40 PST
Attachment 266830 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/runtime/JSObject.h:488: The parameter name "value" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 64 files If any of these errors are false positives, please file a bug against check-webkit-style.
Keith Miller
Comment 6 2015-12-07 16:48:28 PST
WebKit Commit Bot
Comment 7 2015-12-07 16:49:39 PST
Attachment 266833 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/runtime/JSObject.h:488: The parameter name "value" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 64 files If any of these errors are false positives, please file a bug against check-webkit-style.
Keith Miller
Comment 8 2015-12-08 10:53:37 PST
Comment on attachment 266833 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=266833&action=review > Source/JavaScriptCore/ChangeLog:8 > + [WIP] Still need to do 32-bit, add tests, and cleanup but should be roughly done. Need to delete this.
Saam Barati
Comment 9 2015-12-08 11:01:23 PST
Comment on attachment 266833 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=266833&action=review > Source/JavaScriptCore/ChangeLog:17 > + needs non-default behavior for resolving the expression. i.e. The constructor has a Symbol.hasInstance that differs from the one on From reading this sentence I would think that jsTrue=>non-default behavior and that jsFalse=>default behavior but it's the opposite. > Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:2191 > + instructions().append(hasInstanceSymbol->index()); Nit: I think the name hasInstanceSymbol is a bit misleading here (and in other places) because it's the result of doing a GetById with the hasInstanceSymbol and not the symbol itself. Maybe something like "symbolHasInstanceResult" or "hasInstanceSymbolResult" or just "hasInstanceFunction"? > Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h:603 > + RegisterID* emitIsFunction(RegisterID* dst, RegisterID* src); This isn't called > Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp:4222 > + if (!hasInstanceSymbolNode->isCellConstant()) { > + > + notDefault = m_jit.branchPtr(MacroAssembler::NotEqual, hasInstanceSymbol.gpr(), TrustedImmPtr(defaultHasInstanceFunction)); > + } Style: This can be an "if" with no curly braces. > Source/JavaScriptCore/jit/JSInterfaceJIT.h:165 > + ALWAYS_INLINE JSInterfaceJIT::JumpList JSInterfaceJIT::emitJumpIfNull(JSValueRegs reg, GPRReg scratch1, GPRReg scratch2) { This really jumps if it's undefined or null, right? If so, we should name it emitJumpIfNullOrUndefined > Source/JavaScriptCore/llint/LLIntData.cpp:158 > + ASSERT(!(reinterpret_cast<ptrdiff_t>((reinterpret_cast<WriteBarrier<JSCell>*>(0x4000)->slot())) - 0x4000)); Why is this here?
Keith Miller
Comment 10 2015-12-08 11:16:45 PST
Comment on attachment 266833 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=266833&action=review >> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:2191 >> + instructions().append(hasInstanceSymbol->index()); > > Nit: I think the name hasInstanceSymbol is a bit misleading here (and in other places) because it's the result of doing a GetById with the hasInstanceSymbol and not the symbol itself. > Maybe something like "symbolHasInstanceResult" or "hasInstanceSymbolResult" or just "hasInstanceFunction"? How do you feel about hasInstanceValue (since it may or may not be a function)? >> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h:603 >> + RegisterID* emitIsFunction(RegisterID* dst, RegisterID* src); > > This isn't called Deleted. >> Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp:4222 >> + } > > Style: This can be an "if" with no curly braces. Fixed. >> Source/JavaScriptCore/llint/LLIntData.cpp:158 >> + ASSERT(!(reinterpret_cast<ptrdiff_t>((reinterpret_cast<WriteBarrier<JSCell>*>(0x4000)->slot())) - 0x4000)); > > Why is this here? This verifies that the offset of the jsvalue in a WriteBarrier is at the zero offset.
Keith Miller
Comment 11 2015-12-08 11:23:46 PST
> >> + ASSERT(!(reinterpret_cast<ptrdiff_t>((reinterpret_cast<WriteBarrier<JSCell>*>(0x4000)->slot())) - 0x4000)); > > > > Why is this here? > > This verifies that the offset of the jsvalue in a WriteBarrier is at the > zero offset. I should mention that we use this fact in the LLInt as part of the ptr test on Function.prototype[Symbol.hasInstance].
Geoffrey Garen
Comment 12 2015-12-08 11:26:05 PST
Need updated es6.yaml results. Please rename "check has instance" to "overrides has instance".
Keith Miller
Comment 13 2015-12-08 12:32:25 PST
Keith Miller
Comment 14 2015-12-09 17:33:14 PST
WebKit Commit Bot
Comment 15 2015-12-09 17:34:57 PST
Attachment 267058 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/runtime/JSObject.h:488: The parameter name "value" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 62 files If any of these errors are false positives, please file a bug against check-webkit-style.
Saam Barati
Comment 16 2015-12-09 18:56:35 PST
Comment on attachment 267058 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=267058&action=review r=me with comments > Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:2980 > + // Note that we intentionally clobber the hasInstanceValue register here on x86 because we > + // are short on registers. This is valid because we know InstanceOfCustom is the last use > + // of the GetById which produces it. This seems a bit sketchy to me and also probably unnecessary. We probably don't care much about X86. Also, if you're worried about the extra register here you could call an operation that returns a value that fits in a single register on 32-bit platforms. > Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:2997 > + MacroAssembler::Jump slowCase = m_jit.jump(); I don't think you need a variable for this (or above). > Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp:5780 > + // Unlike, the DFG we don't worry about cleaning this code up for the case where we have proven the hasInstanceValue is a constant as LLVM should fix it for us. Style: "Unlike, the DFG" => "Unlike in the DFG," > Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp:5785 > + m_out.branch(m_out.notEqual(hasInstance, m_out.constIntPtr(defaultHasInstanceFunction)), unsure(continuation), unsure(defaultHasInstance)); I think you should have unlikely(continuation) and likely(defaultHasInstance) > Source/JavaScriptCore/jit/JITOpcodes.cpp:854 > + int proto = currentInstruction[3].u.operand; style: You call this "proto" here and "constructor" below. I say just go w/ constructor. > Source/JavaScriptCore/runtime/JSTypeInfo.h:41 > +static const unsigned OverridesHasInstanceFlag = 1 << 2; // FixMe: This is only trivially used by the runtime and should be removed: https://bugs.webkit.org/show_bug.cgi?id=152005 style: FixMe => FIXME > LayoutTests/js/Object-getOwnPropertyNames-expected.txt:61 > PASS getSortedOwnPropertyNames(Error.prototype) is ['constructor', 'message', 'name', 'toString'] What happened to all the other tests?
Saam Barati
Comment 17 2015-12-09 18:57:19 PST
Also, have you run perf tests? Are they neutral? Can you post them?
Keith Miller
Comment 18 2015-12-09 19:28:06 PST
(In reply to comment #17) > Also, have you run perf tests? > Are they neutral? Can you post them? They are attached above and are neutral (The !s are on tests that don't use instanceof).
Keith Miller
Comment 19 2015-12-09 19:30:55 PST
Created attachment 267064 [details] Benchmark results More recent benchmark results.
Keith Miller
Comment 20 2015-12-09 19:55:38 PST
Comment on attachment 267058 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=267058&action=review >> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:2980 >> + // of the GetById which produces it. > > This seems a bit sketchy to me and also probably unnecessary. > We probably don't care much about X86. > Also, if you're worried about the extra register here you could call an operation that returns a value that fits in a single register on 32-bit platforms. I suppose I could add a function that returns an int32_t or something that contains the boolean result of the slow path but I'm not sure that feels any less sketchy. >> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:2997 >> + MacroAssembler::Jump slowCase = m_jit.jump(); > > I don't think you need a variable for this (or above). While you are correct in this case, I dealt with bugs related assigning a gpr after a jump due to calls occurring in the wrong order. I'd feel a bit safer when the order is extra explicit in this context. >> Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp:5780 >> + // Unlike, the DFG we don't worry about cleaning this code up for the case where we have proven the hasInstanceValue is a constant as LLVM should fix it for us. > > Style: "Unlike, the DFG" => "Unlike in the DFG," Fixed. >> Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp:5785 >> + m_out.branch(m_out.notEqual(hasInstance, m_out.constIntPtr(defaultHasInstanceFunction)), unsure(continuation), unsure(defaultHasInstance)); > > I think you should have unlikely(continuation) and likely(defaultHasInstance) I had that at one point but this node is almost never generated unless JS code uses a custom hasInstance in some form. So it makes sense to not punish users when they have a custom hasInstance. >> Source/JavaScriptCore/jit/JITOpcodes.cpp:854 >> + int proto = currentInstruction[3].u.operand; > > style: You call this "proto" here and "constructor" below. I say just go w/ constructor. This is just the diff being laid out poorly... All of this code is just deleted emitSlow_op_check_has_instance being mixed with existing emitSlow_op_instanceof that I did not change... "proto" is actually the prototype of the constructor not the constructor itself. >> Source/JavaScriptCore/runtime/JSTypeInfo.h:41 >> +static const unsigned OverridesHasInstanceFlag = 1 << 2; // FixMe: This is only trivially used by the runtime and should be removed: https://bugs.webkit.org/show_bug.cgi?id=152005 > > style: FixMe => FIXME Fixed. >> LayoutTests/js/Object-getOwnPropertyNames-expected.txt:61 >> PASS getSortedOwnPropertyNames(Error.prototype) is ['constructor', 'message', 'name', 'toString'] > > What happened to all the other tests? Ughh... I forgot to add them... I'll re-upload another version with them again.
Keith Miller
Comment 21 2015-12-09 20:21:35 PST
Saam Barati
Comment 22 2015-12-09 21:15:57 PST
Comment on attachment 267058 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=267058&action=review >>> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:2980 >>> + // of the GetById which produces it. >> >> This seems a bit sketchy to me and also probably unnecessary. >> We probably don't care much about X86. >> Also, if you're worried about the extra register here you could call an operation that returns a value that fits in a single register on 32-bit platforms. > > I suppose I could add a function that returns an int32_t or something that contains the boolean result of the slow path but I'm not sure that feels any less sketchy. Maybe just have one path for everything then? Up to you. >>> Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp:5785 >>> + m_out.branch(m_out.notEqual(hasInstance, m_out.constIntPtr(defaultHasInstanceFunction)), unsure(continuation), unsure(defaultHasInstance)); >> >> I think you should have unlikely(continuation) and likely(defaultHasInstance) > > I had that at one point but this node is almost never generated unless JS code uses a custom hasInstance in some form. So it makes sense to not punish users when they have a custom hasInstance. makes sense >>> Source/JavaScriptCore/jit/JITOpcodes.cpp:854 >>> + int proto = currentInstruction[3].u.operand; >> >> style: You call this "proto" here and "constructor" below. I say just go w/ constructor. > > This is just the diff being laid out poorly... All of this code is just deleted emitSlow_op_check_has_instance being mixed with existing emitSlow_op_instanceof that I did not change... "proto" is actually the prototype of the constructor not the constructor itself. oops I missed that. ok
WebKit Commit Bot
Comment 23 2015-12-10 00:32:13 PST
Attachment 267069 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/runtime/JSObject.h:488: The parameter name "value" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 67 files If any of these errors are false positives, please file a bug against check-webkit-style.
Keith Miller
Comment 24 2015-12-10 17:20:19 PST
WebKit Commit Bot
Comment 25 2015-12-10 17:21:45 PST
Attachment 267142 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/runtime/JSObject.h:488: The parameter name "value" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 67 files If any of these errors are false positives, please file a bug against check-webkit-style.
Keith Miller
Comment 26 2015-12-10 17:24:26 PST
After some investigation, I think you are right that we should use a non-encodedjsvalue to hold the result of the custom slow path. It looks like we use int32_t for that so I changed JIT_OPERATION to return an int32_t. If you want to give it another quick look over, I would appreciate it.
Saam Barati
Comment 27 2015-12-11 12:50:05 PST
Comment on attachment 267142 [details] Patch LGTM
WebKit Commit Bot
Comment 28 2015-12-11 13:44:08 PST
Comment on attachment 267142 [details] Patch Clearing flags on attachment: 267142 Committed r193974: <http://trac.webkit.org/changeset/193974>
WebKit Commit Bot
Comment 29 2015-12-11 13:44:13 PST
All reviewed patches have been landed. Closing bug.
Csaba Osztrogonác
Comment 30 2015-12-12 02:37:50 PST
Csaba Osztrogonác
Comment 31 2015-12-14 02:25:30 PST
(In reply to comment #30) > (In reply to comment #28) > > Comment on attachment 267142 [details] > > Patch > > > > Clearing flags on attachment: 267142 > > > > Committed r193974: <http://trac.webkit.org/changeset/193974> > > It made many JSC tests crash on Linux bots: > - GTK X86_64 - > https://build.webkit.org/builders/GTK%20Linux%2064- > bit%20Release%20%28Tests%29/builds/12637 > - GTK X86 - > https://build.webkit.org/builders/GTK%20Linux%2032-bit%20Release/builds/57694 > - EFL X86_64 - > https://build.webkit.org/builders/EFL%20Linux%2064-bit%20Release%20WK2/ > builds/25878 - EFL Linux AArch64: https://build.webkit.org/builders/EFL%20Linux%20AArch64%20Release/builds/4700 - GTK ARMv7(?): https://build.webkit.org/builders/GTK%20Linux%20ARM%20Release/builds/9652 (EFL ARMv7 bots work fine.)
Csaba Osztrogonác
Comment 32 2015-12-14 02:29:42 PST
I filed a new bug report to fix this regression: bug152245
Carlos Alberto Lopez Perez
Comment 33 2015-12-14 03:05:15 PST
The debug bots are giving this assertion since r193974: ASSERTION FAILED: i < m_numBits ../../Source/WTF/wtf/FastBitVector.h(151) : void WTF::FastBitVector::set(size_t) WTFCrashWithSecurityImplication(...) Example: [...] stress/instanceof-not-cell.js.no-llint: ASSERTION FAILED: i < m_numBits stress/instanceof-not-cell.js.no-llint: ../../Source/WTF/wtf/FastBitVector.h(151) : void WTF::FastBitVector::set(size_t) stress/instanceof-not-cell.js.no-llint: 1 0x7f58e30974ae /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(WTFCrashWithSecurityImplication+0x1e) [0x7f58e30974ae] stress/instanceof-not-cell.js.no-llint: 2 0x7f58e2719b66 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(_ZN3WTF13FastBitVector3setEm+0x42) [0x7f58e2719b66] stress/instanceof-not-cell.js.no-llint: 3 0x7f58e2716964 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(+0x10e7964) [0x7f58e2716964] stress/instanceof-not-cell.js.no-llint: 4 0x7f58e2717a96 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(+0x10e8a96) [0x7f58e2717a96] stress/instanceof-not-cell.js.no-llint: 5 0x7f58e2718543 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(+0x10e9543) [0x7f58e2718543] stress/instanceof-not-cell.js.no-llint: 6 0x7f58e2717af5 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(+0x10e8af5) [0x7f58e2717af5] stress/instanceof-not-cell.js.no-llint: 7 0x7f58e27169dc /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(+0x10e79dc) [0x7f58e27169dc] stress/instanceof-not-cell.js.no-llint: 8 0x7f58e2716ae6 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(+0x10e7ae6) [0x7f58e2716ae6] stress/instanceof-not-cell.js.no-llint: 9 0x7f58e2716bad /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(+0x10e7bad) [0x7f58e2716bad] stress/instanceof-not-cell.js.no-llint: 10 0x7f58e2716de5 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(_ZN3JSC24BytecodeLivenessAnalysis19runLivenessFixpointEv+0x231) [0x7f58e2716de5] stress/instanceof-not-cell.js.no-llint: 11 0x7f58e27179b9 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(_ZN3JSC24BytecodeLivenessAnalysis7computeEv+0x6b) [0x7f58e27179b9] stress/instanceof-not-cell.js.no-llint: 12 0x7f58e271665a /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(_ZN3JSC24BytecodeLivenessAnalysisC1EPNS_9CodeBlockE+0x68) [0x7f58e271665a] stress/instanceof-not-cell.js.no-llint: 13 0x7f58e2951e6a /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(_ZSt11make_uniqueIN3JSC24BytecodeLivenessAnalysisEIPNS0_9CodeBlockEEENSt10_Unique_ifIT_E14_Single_objectEDpOT0_+0x3a) [0x7f58e2951e6a] stress/instanceof-not-cell.js.no-llint: 14 0x7f58e295024d /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(_ZN3JSC9CodeBlock16livenessAnalysisEv+0x91) [0x7f58e295024d] stress/instanceof-not-cell.js.no-llint: 15 0x7f58e294d53d /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(_ZN3JSC3DFG5Graph11livenessForEPNS_9CodeBlockE+0xd9) [0x7f58e294d53d] stress/instanceof-not-cell.js.no-llint: 16 0x7f58e294d643 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(_ZN3JSC3DFG5Graph11livenessForEPNS_15InlineCallFrameE+0x35) [0x7f58e294d643] stress/instanceof-not-cell.js.no-llint: 17 0x7f58e294d9a1 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(_ZN3JSC3DFG5Graph16isLiveInBytecodeENS_15VirtualRegisterENS_10CodeOriginE+0x179) [0x7f58e294d9a1] stress/instanceof-not-cell.js.no-llint: 18 0x7f58e2a2af02 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(+0x13fbf02) [0x7f58e2a2af02] stress/instanceof-not-cell.js.no-llint: 19 0x7f58e2a2acf6 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(+0x13fbcf6) [0x7f58e2a2acf6] stress/instanceof-not-cell.js.no-llint: 20 0x7f58e2a2a8be /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(+0x13fb8be) [0x7f58e2a2a8be] stress/instanceof-not-cell.js.no-llint: 21 0x7f58e2a2b753 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(+0x13fc753) [0x7f58e2a2b753] stress/instanceof-not-cell.js.no-llint: 22 0x7f58e2a2b173 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(+0x13fc173) [0x7f58e2a2b173] stress/instanceof-not-cell.js.no-llint: 23 0x7f58e2a2ad60 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(_ZN3JSC3DFG23performPhantomInsertionERNS0_5GraphE+0x2b) [0x7f58e2a2ad60] stress/instanceof-not-cell.js.no-llint: 24 0x7f58e2a2f940 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(_ZN3JSC3DFG4Plan19compileInThreadImplERNS0_14LongLivedStateE+0x5c6) [0x7f58e2a2f940] stress/instanceof-not-cell.js.no-llint: 25 0x7f58e2a2efdf /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(_ZN3JSC3DFG4Plan15compileInThreadERNS0_14LongLivedStateEPNS0_10ThreadDataE+0x165) [0x7f58e2a2efdf] stress/instanceof-not-cell.js.no-llint: 26 0x7f58e2b1a9af /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(_ZN3JSC3DFG8Worklist9runThreadEPNS0_10ThreadDataE+0x2d7) [0x7f58e2b1a9af] stress/instanceof-not-cell.js.no-llint: 27 0x7f58e2b1ad5c /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(_ZN3JSC3DFG8Worklist14threadFunctionEPv+0x2a) [0x7f58e2b1ad5c] stress/instanceof-not-cell.js.no-llint: 28 0x7f58e30b2598 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(+0x1a83598) [0x7f58e30b2598] stress/instanceof-not-cell.js.no-llint: 29 0x7f58e30b2748 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(+0x1a83748) [0x7f58e30b2748] stress/instanceof-not-cell.js.no-llint: 30 0x7f58e2b3d046 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(_ZNKSt8functionIFvvEEclEv+0x32) [0x7f58e2b3d046] stress/instanceof-not-cell.js.no-llint: 31 0x7f58e30b247a /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(+0x1a8347a) [0x7f58e30b247a] stress/instanceof-not-cell.js.no-llint: Segmentation fault stress/instanceof-not-cell.js.no-llint: ERROR: Unexpected exit code: 139 [...] More at: https://build.webkit.org/builders/GTK%20Linux%2064-bit%20Debug%20%28Tests%29/builds/6385/steps/jscore-test/logs/stdio
Chris Dumez
Comment 34 2015-12-14 09:40:52 PST
Reopening as it was rolled out in <http://trac.webkit.org/changeset/194036> for causing crashes.
Keith Miller
Comment 35 2015-12-17 13:26:01 PST
WebKit Commit Bot
Comment 36 2015-12-17 13:27:38 PST
Attachment 267579 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/runtime/JSObject.h:490: The parameter name "value" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 68 files If any of these errors are false positives, please file a bug against check-webkit-style.
Saam Barati
Comment 37 2015-12-17 13:47:24 PST
Comment on attachment 267579 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=267579&action=review r=me again > Source/JavaScriptCore/bytecode/BytecodeUseDef.h:76 > + ASSERT(opcodeLengths[opcodeID] >= 1); All these asserts should be > and not >=.
Keith Miller
Comment 38 2015-12-17 13:50:28 PST
Comment on attachment 267579 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=267579&action=review >> Source/JavaScriptCore/bytecode/BytecodeUseDef.h:76 >> + ASSERT(opcodeLengths[opcodeID] >= 1); > > All these asserts should be > and not >=. Fixed.
Keith Miller
Comment 39 2015-12-17 16:38:02 PST
Joseph Pecoraro
Comment 40 2016-06-06 20:53:33 PDT
*** Bug 151120 has been marked as a duplicate of this bug. ***
Note You need to log in before you can comment on or make changes to this bug.