Skip to content

Commit e3b2498

Browse files
atulkattiatulkatti
committed
[MERGE #6158 @atulkatti] 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.
Merge pull request #6158 from atulkatti:Bug19767482.CloneStrNullCheck.2
2 parents 8ca6279 + 618abe2 commit e3b2498

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)