Bug 151839

Created attachment 266587 [details] Patch

Created attachment 266588 [details] Benchmark results 64-bit benchmark results.

Created attachment 266823 [details] Patch

Created attachment 266830 [details] Patch

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.

Created attachment 266833 [details] Patch

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.

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.

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?

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.

> >> + 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].

Need updated es6.yaml results. Please rename "check has instance" to "overrides has instance".

Created attachment 266927 [details] Patch

Created attachment 267058 [details] Patch

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.

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?

Also, have you run perf tests? Are they neutral? Can you post them?

(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).

Created attachment 267064 [details] Benchmark results More recent benchmark results.

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.

Created attachment 267069 [details] Patch

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

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.

Created attachment 267142 [details] Patch

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.

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.

Comment on attachment 267142 [details] Patch LGTM

Comment on attachment 267142 [details] Patch Clearing flags on attachment: 267142 Committed r193974: <http://trac.webkit.org/changeset/193974>

All reviewed patches have been landed. Closing bug.

(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

(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.)

I filed a new bug report to fix this regression: bug152245

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

Reopening as it was rolled out in <http://trac.webkit.org/changeset/194036> for causing crashes.

Created attachment 267579 [details] Patch

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.

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 >=.

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.

Committed r194248: <http://trac.webkit.org/changeset/194248>

*** Bug 151120 has been marked as a duplicate of this bug. ***