Skip to content
Permalink
Browse files

[MERGE #6158 @atulkatti] MSFT:19767482 Add a NULL check for the sourc…

…e opnd while lowering CloneStr to avoid deferencing nullptr when we bailout from Simple JIT to Full JIT for loop bodies involving CloneStr optimization.

Merge pull request #6158 from atulkatti:Bug19767482.CloneStrNullCheck.2
  • Loading branch information...
atulkatti committed Jun 12, 2019
2 parents 8ca6279 + 618abe2 commit e3b2498f5a69dbfdc5aa3d81078c91aad6e17345
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 e3b2498

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