Skip to content

Commit

Permalink
[1.8>1.9] [MERGE #4626 @boingoing] OS#14115684: Cached scope is not i…
Browse files Browse the repository at this point in the history
…nvalidated when eval code leaks a function from the cached scope

Merge pull request #4626 from boingoing:InvalidateCachedScope

We detect property loads from an ActivationObject for which the property is a function stored in the cached scope. If we load one of those functions, we must mark the parent function as having a function escape and invalidate the cached scope. Right now we aren't doing this correctly due to a math error. We keep track of the indices of both the first and last functions in the activation object slots but the last slot index is always less than the first slot index. Because of this, when we load a property from the activation object it can never invalidate the cached scope even if it is an escaping function.

Fix seems to be to correct the math to compute the slot indices in `JavascriptOperators::OP_InitCachedScope`.

Fixes:
https://microsoft.visualstudio.com/web/wi.aspx?id=14115684
  • Loading branch information
boingoing committed Feb 2, 2018
2 parents 59f8530 + 4449e57 commit a5d8155
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 1 deletion.
2 changes: 1 addition & 1 deletion lib/Runtime/Language/JavascriptOperators.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5475,7 +5475,7 @@ namespace Js

if (firstFuncSlot != Constants::NoProperty)
{
if (firstVarSlot == Constants::NoProperty)
if (firstVarSlot == Constants::NoProperty || firstVarSlot < firstFuncSlot)
{
lastFuncSlot = propIds->count - 1;
}
Expand Down
27 changes: 27 additions & 0 deletions test/Bugs/bug_OS14326981.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
//-------------------------------------------------------------------------------------------------------
// Copyright (C) Microsoft. All rights reserved.
// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
//-------------------------------------------------------------------------------------------------------

WScript.LoadScriptFile("..\\UnitTestFramework\\UnitTestFramework.js");

var tests = [
{
name: "Verify cached scope is invalidated",
body: function () {
function foo() {
var x = 100;
//create a stack allocated func
function bar() {
return x;
}
eval("count = bar;");
}
var count = {};
foo();
assert.areEqual(100, count(), "Function leaked from cached scope should cause cached scope to be invalidated");
}
},
];

testRunner.runTests(tests, { verbose: WScript.Arguments[0] != "summary" });
7 changes: 7 additions & 0 deletions test/Bugs/rlexe.xml
Original file line number Diff line number Diff line change
Expand Up @@ -433,4 +433,11 @@
<files>bug15490382.js</files>
</default>
</test>
<test>
<default>
<files>bug_OS14326981.js</files>
<tags>BugFix</tags>
<compile-flags>-args summary -endargs</compile-flags>
</default>
</test>
</regress-exe>

0 comments on commit a5d8155

Please sign in to comment.