Bug 171786

Summary: REGRESSION(r206870): ChakraCore.yaml/ChakraCore/test/Strings/HTMLHelpers.js.default intermittently fails
Product: WebKit Reporter: Mark Lam <mark.lam>
Component: JavaScriptCoreAssignee: Mark Lam <mark.lam>
Status: RESOLVED FIXED    
Severity: Normal CC: fpizlo, jfbastien, keith_miller, msaboff, oliver, saam, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 171737    
Attachments:
Description Flags
reduced repro test case. Run with JSC_useConcurrentJIT=0.
none
proposed patch. saam: review+

Mark Lam
Reported 2017-05-06 21:33:19 PDT
After r216301, ChakraCore.yaml/ChakraCore/test/Strings/HTMLHelpers.js.default intermittently fails. Here's how I reproduce the issue: 1. cd JSTests/ChakraCore/test/Strings 2. lldb jsc -- --useFunctionDotArguments\=true --maxPerThreadStackUsage\=1572864 --thresholdForJITAfterWarmUp\=10 --thresholdForJITSoon\=10 --thresholdForOptimizeAfterWarmUp\=20 --thresholdForOptimizeAfterLongWarmUp\=20 --thresholdForOptimizeSoon\=20 --thresholdForFTLOptimizeAfterWarmUp\=20 --thresholdForFTLOptimizeSoon\=20 --maximumEvalCacheableSourceLength\=150000 --useEagerCodeBlockJettisonTiming\=true ../jsc-lib.js HTMLHelpers.js The stack trace: (lldb) bt 15 * thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0xbbadbeef) frame #0: 0x0000000101499cb4 JavaScriptCore`::WTFCrash() at Assertions.cpp:292 * frame #1: 0x00000001005dce07 JavaScriptCore`::slow_path_throw_static_error(exec=0x00007fff5fbfb620, pc=0x00000001093f0780) at CommonSlowPaths.cpp:993 frame #2: 0x00003047ef803323 frame #3: 0x0000000100ff9e71 JavaScriptCore`llint_entry at LowLevelInterpreter.asm:795 frame #4: 0x0000000100ff272e JavaScriptCore`vmEntryToJavaScript at LowLevelInterpreter64.asm:254 frame #5: 0x0000000100dc5d0e JavaScriptCore`JSC::JITCode::execute(this=0x00000001044ec898, vm=0x0000000109000000, protoCallFrame=0x00007fff5fbfb990) at JITCode.cpp:81 frame #6: 0x0000000100d71c41 JavaScriptCore`JSC::Interpreter::execute(this=0x00000001045f4040, eval=0x000000010977f8a0, callFrame=0x00007fff5fbfd150, thisValue=JSValue @ 0x00007fff5fbfbd40, scope=0x00000001097a4de0) at Interpreter.cpp:1223 frame #7: 0x0000000100d70594 JavaScriptCore`JSC::eval(callFrame=0x00007fff5fbfd150) at Interpreter.cpp:170 frame #8: 0x0000000100ddadc1 JavaScriptCore`::operationCallEval(exec=0x00007fff5fbfd1e0, execCallee=0x00007fff5fbfd150) at JITOperations.cpp:845 frame #9: 0x00003047ef821627 frame #10: 0x00003047ef8272dd frame #11: 0x0000000100ff9df7 JavaScriptCore`llint_entry at LowLevelInterpreter.asm:795 frame #12: 0x0000000100ff9e71 JavaScriptCore`llint_entry at LowLevelInterpreter.asm:795 frame #13: 0x0000000100ff9e71 JavaScriptCore`llint_entry at LowLevelInterpreter.asm:795 frame #14: 0x0000000100ff9e71 JavaScriptCore`llint_entry at LowLevelInterpreter.asm:795 (lldb) up frame #1: 0x00000001005dce07 JavaScriptCore`::slow_path_throw_static_error(exec=0x00007fff5fbfb620, pc=0x00000001093f0780) at CommonSlowPaths.cpp:993 990 { 991 BEGIN(); 992 JSValue errorMessageValue = OP_C(1).jsValue(); -> 993 RELEASE_ASSERT(errorMessageValue.isString()); 994 String errorMessage = asString(errorMessageValue)->value(exec); 995 ErrorType errorType = static_cast<ErrorType>(pc[2].u.unsignedValue); 996 THROW(createError(exec, errorType, errorMessage)); (lldb) p errorMessageValue (JSC::JSValue) $0 = { u = { asInt64 = 10 ptr = 0x000000000000000a asBits = (payload = 10, tag = 0) } } i.e. errorMessageValue is undefined. AFAICT, this issue only manifests if the DFG is enabled (FTL not required). Needless to say, it also manifests on a release build.
Attachments
reduced repro test case. Run with JSC_useConcurrentJIT=0. (350 bytes, application/x-javascript)
2017-05-08 12:56 PDT, Mark Lam 		no flags
Details
proposed patch. (4.92 KB, patch)
2017-05-08 15:01 PDT, Mark Lam 		saam: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2017-05-08 09:21:58 PDT
<rdar://problem/32051023>
Mark Lam
Comment 2 2017-05-08 10:45:02 PDT
I've created a reduced test case. Still investigating the root cause.
Oliver Hunt
Comment 3 2017-05-08 12:12:25 PDT
fails during these calls: assert.throws(function () { eval("String.prototype." + wrappers[i] + ".call(null);") }, TypeError); assert.throws(function () { eval("String.prototype." + wrappers[i] + ".call(undefined);") }, TypeError); Specifically at: frame #6: 0x00000001002f4e3e JavaScriptCore`::slow_path_throw_static_error(exec=0x00007fff5fbfd920, pc=0x0000000103ff0780) at CommonSlowPaths.cpp:992 [opt] 989 SLOW_PATH_DECL(slow_path_throw_static_error) 990 { 991 BEGIN(); -> 992 JSValue errorMessageValue = OP_C(1).jsValue(); OP_C(1) goes to: frame #5: 0x00000001002f4e3e JavaScriptCore`::slow_path_throw_static_error(JSC::ExecState *, JSC::Instruction *) [inlined] JSC::ExecState::r(this=0x00007fff5fbfd920) at CodeBlock.h:1044 [opt] 1041 { 1042 CodeBlock* codeBlock = this->codeBlock(); 1043 if (codeBlock->isConstantRegisterIndex(index)) -> 1044 return *reinterpret_cast<Register*>(&codeBlock->constantRegister(index)); (where lldb won't show me the value index) I think something is going wrong with: @throwTypeError(`${func} requires that |this| not be null or undefined`); I can fix this by switching to func + " ... "
Oliver Hunt
Comment 4 2017-05-08 12:12:35 PDT
(In reply to Mark Lam from comment #2) > I've created a reduced test case. Still investigating the root cause. Can you attach it?
Mark Lam
Comment 5 2017-05-08 12:56:53 PDT
Created attachment 309397 [details] reduced repro test case. Run with JSC_useConcurrentJIT=0.
Mark Lam
Comment 6 2017-05-08 14:49:00 PDT
This is actually a regression due to r206870. r206870 changed op_throw_static_error to use a variable, but did not reflect this in DFGBytecodeUseDef. The fix is also to reflect the change in DFGBytecodeUseDef. There's also a parallel bug in BytecodeDumper where the dumper for op_throw_static_error was expecting op1 to be a constant. But r206870 changed op1 to be a variable. I'll also apply the fix for the dumper.
Mark Lam
Comment 7 2017-05-08 15:01:45 PDT
Created attachment 309418 [details] proposed patch.
Mark Lam
Comment 8 2017-05-08 15:25:27 PDT
Thanks for the review. Landed in r216459: <http://trac.webkit.org/r216459>.
Yusuke Suzuki
Comment 9 2017-05-08 16:05:34 PDT
Oops, right. It's my fault. Thanks for fixing this :)
Note You need to log in before you can comment on or make changes to this bug.