Skip to content
Permalink
Browse files

MSFT:19767482 Add a NULL check for the source opnd while lowering Clo…

…neStr to avoid deferencing nullptr when we bailout from Simple JIT to Full JIT for loop bodies involving CloneStr optimization.
  • Loading branch information...
atulkatti committed Jun 10, 2019
1 parent 8ca6279 commit 618abe2bee25559494d48ae1f82836161b2ecd2b
Showing with 153 additions and 3 deletions.
  1. +12 −2 lib/Backend/Lower.cpp
  2. +1 −1 lib/Backend/Lower.h
  3. +53 −0 test/Bugs/Bug19767482.js
  4. +75 −0 test/Bugs/Bug19948792.js
  5. +12 −0 test/Bugs/rlexe.xml
@@ -594,7 +594,7 @@ Lowerer::LowerRange(IR::Instr *instrStart, IR::Instr *instrEnd, bool defaultDoFa

case Js::OpCode::CloneStr:
{
GenerateGetImmutableOrScriptUnreferencedString(instr->GetSrc1()->AsRegOpnd(), instr, IR::HelperOp_CompoundStringCloneForAppending, false);
GenerateGetImmutableOrScriptUnreferencedString(instr->GetSrc1()->AsRegOpnd(), instr, IR::HelperOp_CompoundStringCloneForAppending, true /*loweringCloneStr*/, false /*reloadDst*/);
instr->Remove();
break;
}
@@ -26130,7 +26130,7 @@ Lowerer::LowerSetConcatStrMultiItem(IR::Instr * instr)
}

IR::RegOpnd *
Lowerer::GenerateGetImmutableOrScriptUnreferencedString(IR::RegOpnd * strOpnd, IR::Instr * insertBeforeInstr, IR::JnHelperMethod helperMethod, bool reloadDst)
Lowerer::GenerateGetImmutableOrScriptUnreferencedString(IR::RegOpnd * strOpnd, IR::Instr * insertBeforeInstr, IR::JnHelperMethod helperMethod, bool loweringCloneStr, bool reloadDst)
{
if (strOpnd->m_sym->m_isStrConst)
{
@@ -26146,6 +26146,16 @@ Lowerer::GenerateGetImmutableOrScriptUnreferencedString(IR::RegOpnd * strOpnd, I
{
this->m_lowererMD.GenerateObjectTest(strOpnd, insertBeforeInstr, doneLabel);
}

if (loweringCloneStr && func->IsLoopBody())
{
// Check if strOpnd is NULL before the CloneStr. There could be cases where SimpleJit might have dead stored instructions corresponding to the definition/use of strOpnd.
// As a result during a bailout when we restore values from interpreter stack frame we may end up having strOpnd as nullptr. During FullJit we may not dead store the
// instructions defining/using strOpnd due to StSlot instructions added at the end of jitted loop body. As a result, when we bailout (BailOnSimpleJitToFullJitLoopBody)
// strOpnd could have a NULL value causing CloneStr to dereference a nullptr.
this->InsertCompareBranch(strOpnd, IR::AddrOpnd::New(nullptr, IR::AddrOpndKindDynamicMisc, this->m_func), Js::OpCode::BrEq_A, false /*isUnsigned*/, doneLabel, insertBeforeInstr);
}

// CMP [strOpnd], Js::CompoundString::`vtable'
// JEQ $helper
InsertCompareBranch(
@@ -667,7 +667,7 @@ class Lowerer

IR::Instr * LowerInitClass(IR::Instr * instr);

IR::RegOpnd * GenerateGetImmutableOrScriptUnreferencedString(IR::RegOpnd * strOpnd, IR::Instr * insertBeforeInstr, IR::JnHelperMethod helperMethod, bool reloadDst = true);
IR::RegOpnd * GenerateGetImmutableOrScriptUnreferencedString(IR::RegOpnd * strOpnd, IR::Instr * insertBeforeInstr, IR::JnHelperMethod helperMethod, bool loweringCloneStr = false, bool reloadDst = true);
void LowerNewConcatStrMulti(IR::Instr * instr);
void LowerNewConcatStrMultiBE(IR::Instr * instr);
void LowerSetConcatStrMultiItem(IR::Instr * instr);
@@ -0,0 +1,53 @@
//Configuration: Configs\full_with_strings.xml
//Testcase Number: 4018
//Switches: -PrintSystemException -ArrayMutationTestSeed:2004204769 -maxinterpretcount:1 -maxsimplejitruncount:2 -MinMemOpCount:0 -werexceptionsupport -MinSwitchJumpTableSize:2 -bgjit- -loopinterpretcount:1 -MaxLinearStringCaseCount:2 -MaxLinearIntCaseCount:2 -on:CaptureByteCodeRegUse -force:rejit -PoisonIntArrayLoad- -force:ScriptFunctionWithInlineCache -force:atom -force:fixdataprops
//Baseline Switches: -nonative -werexceptionsupport -PrintSystemException -ArrayMutationTestSeed:2004204769
//Arch: x86
//Flavor: debug
//BuildName: chakrafull.full
//BuildRun: 180827_0314
//BuildId:
//BuildHash:
//BinaryPath: \\chakrafs\fs\Builds\ChakraFull\unreleased\rs5\1808\00025.55398_180827.0925_sethb_4fd79162a0d5cbf937f5f47fa77d55a211ea9c2b\bin\x86_debug
//MachineName: master-vm
//Reduced Switches: -printsystemexception -maxinterpretcount:1 -maxsimplejitruncount:2 -werexceptionsupport -bgjit- -loopinterpretcount:1 -bvt -oopjit-
//noRepro switches0: -printsystemexception -maxinterpretcount:1 -maxsimplejitruncount:2 -werexceptionsupport -bgjit- -loopinterpretcount:1 -bvt -oopjit- -off:DynamicProfile
//noRepro switches1: -printsystemexception -maxinterpretcount:1 -maxsimplejitruncount:2 -werexceptionsupport -bgjit- -loopinterpretcount:1 -bvt -oopjit- -off:InterpreterAutoProfile
//noRepro switches2: -printsystemexception -maxinterpretcount:1 -maxsimplejitruncount:2 -werexceptionsupport -bgjit- -loopinterpretcount:1 -bvt -oopjit- -off:InterpreterProfile
//noRepro switches3: -printsystemexception -maxinterpretcount:1 -maxsimplejitruncount:2 -werexceptionsupport -bgjit- -loopinterpretcount:1 -bvt -oopjit- -off:JITLoopBody
//noRepro switches4: -printsystemexception -maxinterpretcount:1 -maxsimplejitruncount:2 -werexceptionsupport -bgjit- -loopinterpretcount:1 -bvt -oopjit- -off:SimpleJit
function test0() {
var __loopvar0 = -4;
while (true) {
__loopvar0 += 2;
if (__loopvar0 > 3) {
break;
}

if ('caller') {
var strvar9 = '';
} else {
var strvar9 = '';
do {
(strvar9 + protoObj0)();
} while ('');

return this;
}
}
}

test0();
test0();
WScript.Echo("Pass");

// === Output ===
// command: D:\\BinariesCache\\1808\00025.55398_180827.0925_sethb_4fd79162a0d5cbf937f5f47fa77d55a211ea9c2b\bin\x86_debug\jshost.exe -printsystemexception -maxinterpretcount:1 -maxsimplejitruncount:2 -werexceptionsupport -bgjit- -loopinterpretcount:1 -bvt -oopjit- step.js
// exitcode: 0xC0000005
// stdout:
//
// stderr:
// FATAL ERROR: jshost.exe failed due to exception code c0000005
//
//
//
@@ -0,0 +1,75 @@
//Configuration: Configs\lessmath.xml
//Testcase Number: 4592
//Bailout Testing: ON
//Switches: -PrintSystemException -ArrayMutationTestSeed:14309800 -maxinterpretcount:1 -maxsimplejitruncount:2 -MinMemOpCount:0 -werexceptionsupport -MinSwitchJumpTableSize:2 -force:fieldcopyprop -bgjit- -loopinterpretcount:1 -MaxLinearStringCaseCount:2 -MaxLinearIntCaseCount:2 -forcedeferparse -force:fieldPRE -off:polymorphicinlinecache -off:simplejit -force:inline -sse:3 -force:rejit -ForceStaticInterpreterThunk -ForceJITCFGCheck -UseJITTrampoline- -force:atom -force:fixdataprops -PoisonStringLoad- -force:ScriptFunctionWithInlineCache -off:cachedScope -off:fefixedmethods -off:stackargopt -PoisonVarArrayLoad- -ForceArrayBTree -off:checkthis -on:CaptureByteCodeRegUse -PoisonTypedArrayLoad- -oopjit- -off:ArrayLengthHoist -PoisonIntArrayLoad- -off:DelayCapture -PoisonFloatArrayLoad- -off:ArrayCheckHoist -off:aggressiveinttypespec -off:lossyinttypespec -off:ParallelParse -off:trackintusage -off:floattypespec
//Baseline Switches: -nonative -werexceptionsupport -PrintSystemException -ArrayMutationTestSeed:14309800
//Arch: x86
//Flavor: test
//BuildName: chakrafull.full
//BuildRun: 01231_0400
//BuildId:
//BuildHash:
//BinaryPath: C:\Builds\ChakraFull_x86_test
//MachineName: master-vm
//Reduced Switches: -printsystemexception -maxinterpretcount:1 -maxsimplejitruncount:2 -werexceptionsupport -bgjit- -loopinterpretcount:1 -oopjit- -bvt -off:simplejit
//noRepro switches0: -printsystemexception -maxinterpretcount:1 -maxsimplejitruncount:2 -werexceptionsupport -bgjit- -loopinterpretcount:1 -oopjit- -bvt -off:simplejit -off:DynamicProfile
//noRepro switches1: -printsystemexception -maxinterpretcount:1 -maxsimplejitruncount:2 -werexceptionsupport -bgjit- -loopinterpretcount:1 -oopjit- -bvt -off:simplejit -off:FastPath
//noRepro switches2: -printsystemexception -maxinterpretcount:1 -maxsimplejitruncount:2 -werexceptionsupport -bgjit- -loopinterpretcount:1 -oopjit- -bvt -off:simplejit -off:InterpreterProfile
//noRepro switches3: -printsystemexception -maxinterpretcount:1 -maxsimplejitruncount:2 -werexceptionsupport -bgjit- -loopinterpretcount:1 -oopjit- -bvt -off:simplejit -off:JITLoopBody
//noRepro switches4: -printsystemexception -maxinterpretcount:1 -maxsimplejitruncount:2 -werexceptionsupport -bgjit- -loopinterpretcount:1 -oopjit- -bvt -off:simplejit -off:ObjTypeSpec
function test0() {
var obj0 = {};
var litObj0 = {};
var func0 = function () {
};
obj0.method0 = func0;
var __loopvar0 = 8;
for (var __loopSecondaryVar0_0 = 8;;) {
if (__loopvar0 <= 6) {
break;
}
__loopvar0 = 3;
function func5() {
}
function func6() {
({
v5: function () {
func5();
}
});
}
var __loopvar1 = 3;
while (this) {
__loopvar1++;
if (__loopvar1 >= 8) {
break;
}
litObj0.prop1 = obj0.method0();
continue;
if (true) {
func6;
} else {
(function () {
func6();
}());
for (var _strvar0 of ary) {
}
}
}
}
}

test0();
test0();
WScript.Echo("Pass");

// === Output ===
// command: E:\\BinariesCache\\1812\00029.27247_181214.1751_t-nhng_1b40a2baa24e6b9b20178f54a406dc7faa5949bd\bin\x86_test\jshost.exe -printsystemexception -maxinterpretcount:1 -maxsimplejitruncount:2 -werexceptionsupport -bgjit- -loopinterpretcount:1 -oopjit- -bvt -off:simplejit step.js
// exitcode: 0xC0000005
// stdout:
//
// stderr:
// FATAL ERROR: jshost.exe failed due to exception code c0000005
//
//
//
@@ -588,4 +588,16 @@
<compile-flags>-force:deferparse</compile-flags>
</default>
</test>
<test>
<default>
<files>Bug19767482.js</files>
<compile-flags>-maxinterpretcount:1 -bgjit- -oopjit- -loopinterpretcount:1 -maxsimplejitruncount:2</compile-flags>
</default>
</test>
<test>
<default>
<files>Bug19948792.js</files>
<compile-flags>-maxinterpretcount:1 -bgjit- -oopjit- -loopinterpretcount:1 -maxsimplejitruncount:2</compile-flags>
</default>
</test>
</regress-exe>

0 comments on commit 618abe2

Please sign in to comment.
You can’t perform that action at this time.