Skip to content

Commit 618abe2

Browse files
atulkattiatulkatti
committed
MSFT:19767482 Add a NULL check for the source opnd while lowering CloneStr to avoid deferencing nullptr when we bailout from Simple JIT to Full JIT for loop bodies involving CloneStr optimization.
1 parent 8ca6279 commit 618abe2

File tree

5 files changed

+153
-3
lines changed

5 files changed

+153
-3
lines changed

‎lib/Backend/Lower.cpp

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -594,7 +594,7 @@ Lowerer::LowerRange(IR::Instr *instrStart, IR::Instr *instrEnd, bool defaultDoFa
594594

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

2613226132
IR::RegOpnd *
26133-
Lowerer::GenerateGetImmutableOrScriptUnreferencedString(IR::RegOpnd * strOpnd, IR::Instr * insertBeforeInstr, IR::JnHelperMethod helperMethod, bool reloadDst)
26133+
Lowerer::GenerateGetImmutableOrScriptUnreferencedString(IR::RegOpnd * strOpnd, IR::Instr * insertBeforeInstr, IR::JnHelperMethod helperMethod, bool loweringCloneStr, bool reloadDst)
2613426134
{
2613526135
if (strOpnd->m_sym->m_isStrConst)
2613626136
{
@@ -26146,6 +26146,16 @@ Lowerer::GenerateGetImmutableOrScriptUnreferencedString(IR::RegOpnd * strOpnd, I
2614626146
{
2614726147
this->m_lowererMD.GenerateObjectTest(strOpnd, insertBeforeInstr, doneLabel);
2614826148
}
26149+
26150+
if (loweringCloneStr && func->IsLoopBody())
26151+
{
26152+
// 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.
26153+
// 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
26154+
// instructions defining/using strOpnd due to StSlot instructions added at the end of jitted loop body. As a result, when we bailout (BailOnSimpleJitToFullJitLoopBody)
26155+
// strOpnd could have a NULL value causing CloneStr to dereference a nullptr.
26156+
this->InsertCompareBranch(strOpnd, IR::AddrOpnd::New(nullptr, IR::AddrOpndKindDynamicMisc, this->m_func), Js::OpCode::BrEq_A, false /*isUnsigned*/, doneLabel, insertBeforeInstr);
26157+
}
26158+
2614926159
// CMP [strOpnd], Js::CompoundString::`vtable'
2615026160
// JEQ $helper
2615126161
InsertCompareBranch(

‎lib/Backend/Lower.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -667,7 +667,7 @@ class Lowerer
667667

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

670-
IR::RegOpnd * GenerateGetImmutableOrScriptUnreferencedString(IR::RegOpnd * strOpnd, IR::Instr * insertBeforeInstr, IR::JnHelperMethod helperMethod, bool reloadDst = true);
670+
IR::RegOpnd * GenerateGetImmutableOrScriptUnreferencedString(IR::RegOpnd * strOpnd, IR::Instr * insertBeforeInstr, IR::JnHelperMethod helperMethod, bool loweringCloneStr = false, bool reloadDst = true);
671671
void LowerNewConcatStrMulti(IR::Instr * instr);
672672
void LowerNewConcatStrMultiBE(IR::Instr * instr);
673673
void LowerSetConcatStrMultiItem(IR::Instr * instr);

‎test/Bugs/Bug19767482.js

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
//Configuration: Configs\full_with_strings.xml
2+
//Testcase Number: 4018
3+
//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
4+
//Baseline Switches: -nonative -werexceptionsupport -PrintSystemException -ArrayMutationTestSeed:2004204769
5+
//Arch: x86
6+
//Flavor: debug
7+
//BuildName: chakrafull.full
8+
//BuildRun: 180827_0314
9+
//BuildId:
10+
//BuildHash:
11+
//BinaryPath: \\chakrafs\fs\Builds\ChakraFull\unreleased\rs5\1808\00025.55398_180827.0925_sethb_4fd79162a0d5cbf937f5f47fa77d55a211ea9c2b\bin\x86_debug
12+
//MachineName: master-vm
13+
//Reduced Switches: -printsystemexception -maxinterpretcount:1 -maxsimplejitruncount:2 -werexceptionsupport -bgjit- -loopinterpretcount:1 -bvt -oopjit-
14+
//noRepro switches0: -printsystemexception -maxinterpretcount:1 -maxsimplejitruncount:2 -werexceptionsupport -bgjit- -loopinterpretcount:1 -bvt -oopjit- -off:DynamicProfile
15+
//noRepro switches1: -printsystemexception -maxinterpretcount:1 -maxsimplejitruncount:2 -werexceptionsupport -bgjit- -loopinterpretcount:1 -bvt -oopjit- -off:InterpreterAutoProfile
16+
//noRepro switches2: -printsystemexception -maxinterpretcount:1 -maxsimplejitruncount:2 -werexceptionsupport -bgjit- -loopinterpretcount:1 -bvt -oopjit- -off:InterpreterProfile
17+
//noRepro switches3: -printsystemexception -maxinterpretcount:1 -maxsimplejitruncount:2 -werexceptionsupport -bgjit- -loopinterpretcount:1 -bvt -oopjit- -off:JITLoopBody
18+
//noRepro switches4: -printsystemexception -maxinterpretcount:1 -maxsimplejitruncount:2 -werexceptionsupport -bgjit- -loopinterpretcount:1 -bvt -oopjit- -off:SimpleJit
19+
function test0() {
20+
var __loopvar0 = -4;
21+
while (true) {
22+
__loopvar0 += 2;
23+
if (__loopvar0 > 3) {
24+
break;
25+
}
26+
27+
if ('caller') {
28+
var strvar9 = '';
29+
} else {
30+
var strvar9 = '';
31+
do {
32+
(strvar9 + protoObj0)();
33+
} while ('');
34+
35+
return this;
36+
}
37+
}
38+
}
39+
40+
test0();
41+
test0();
42+
WScript.Echo("Pass");
43+
44+
// === Output ===
45+
// 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
46+
// exitcode: 0xC0000005
47+
// stdout:
48+
//
49+
// stderr:
50+
// FATAL ERROR: jshost.exe failed due to exception code c0000005
51+
//
52+
//
53+
//

‎test/Bugs/Bug19948792.js

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
//Configuration: Configs\lessmath.xml
2+
//Testcase Number: 4592
3+
//Bailout Testing: ON
4+
//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
5+
//Baseline Switches: -nonative -werexceptionsupport -PrintSystemException -ArrayMutationTestSeed:14309800
6+
//Arch: x86
7+
//Flavor: test
8+
//BuildName: chakrafull.full
9+
//BuildRun: 01231_0400
10+
//BuildId:
11+
//BuildHash:
12+
//BinaryPath: C:\Builds\ChakraFull_x86_test
13+
//MachineName: master-vm
14+
//Reduced Switches: -printsystemexception -maxinterpretcount:1 -maxsimplejitruncount:2 -werexceptionsupport -bgjit- -loopinterpretcount:1 -oopjit- -bvt -off:simplejit
15+
//noRepro switches0: -printsystemexception -maxinterpretcount:1 -maxsimplejitruncount:2 -werexceptionsupport -bgjit- -loopinterpretcount:1 -oopjit- -bvt -off:simplejit -off:DynamicProfile
16+
//noRepro switches1: -printsystemexception -maxinterpretcount:1 -maxsimplejitruncount:2 -werexceptionsupport -bgjit- -loopinterpretcount:1 -oopjit- -bvt -off:simplejit -off:FastPath
17+
//noRepro switches2: -printsystemexception -maxinterpretcount:1 -maxsimplejitruncount:2 -werexceptionsupport -bgjit- -loopinterpretcount:1 -oopjit- -bvt -off:simplejit -off:InterpreterProfile
18+
//noRepro switches3: -printsystemexception -maxinterpretcount:1 -maxsimplejitruncount:2 -werexceptionsupport -bgjit- -loopinterpretcount:1 -oopjit- -bvt -off:simplejit -off:JITLoopBody
19+
//noRepro switches4: -printsystemexception -maxinterpretcount:1 -maxsimplejitruncount:2 -werexceptionsupport -bgjit- -loopinterpretcount:1 -oopjit- -bvt -off:simplejit -off:ObjTypeSpec
20+
function test0() {
21+
var obj0 = {};
22+
var litObj0 = {};
23+
var func0 = function () {
24+
};
25+
obj0.method0 = func0;
26+
var __loopvar0 = 8;
27+
for (var __loopSecondaryVar0_0 = 8;;) {
28+
if (__loopvar0 <= 6) {
29+
break;
30+
}
31+
__loopvar0 = 3;
32+
function func5() {
33+
}
34+
function func6() {
35+
({
36+
v5: function () {
37+
func5();
38+
}
39+
});
40+
}
41+
var __loopvar1 = 3;
42+
while (this) {
43+
__loopvar1++;
44+
if (__loopvar1 >= 8) {
45+
break;
46+
}
47+
litObj0.prop1 = obj0.method0();
48+
continue;
49+
if (true) {
50+
func6;
51+
} else {
52+
(function () {
53+
func6();
54+
}());
55+
for (var _strvar0 of ary) {
56+
}
57+
}
58+
}
59+
}
60+
}
61+
62+
test0();
63+
test0();
64+
WScript.Echo("Pass");
65+
66+
// === Output ===
67+
// 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
68+
// exitcode: 0xC0000005
69+
// stdout:
70+
//
71+
// stderr:
72+
// FATAL ERROR: jshost.exe failed due to exception code c0000005
73+
//
74+
//
75+
//

‎test/Bugs/rlexe.xml

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -588,4 +588,16 @@
588588
<compile-flags>-force:deferparse</compile-flags>
589589
</default>
590590
</test>
591+
<test>
592+
<default>
593+
<files>Bug19767482.js</files>
594+
<compile-flags>-maxinterpretcount:1 -bgjit- -oopjit- -loopinterpretcount:1 -maxsimplejitruncount:2</compile-flags>
595+
</default>
596+
</test>
597+
<test>
598+
<default>
599+
<files>Bug19948792.js</files>
600+
<compile-flags>-maxinterpretcount:1 -bgjit- -oopjit- -loopinterpretcount:1 -maxsimplejitruncount:2</compile-flags>
601+
</default>
602+
</test>
591603
</regress-exe>

0 commit comments

Comments
 (0)