Skip to content
This repository has been archived by the owner on Oct 15, 2020. It is now read-only.

Commit

Permalink
deps: update ChakraCore to chakra-core/ChakraCore@0615a510f6
Browse files Browse the repository at this point in the history
[1.8>1.9] [MERGE #4607 @boingoing] OS#14763260: Correctly update RegExp.$1 after RegExp.prototype.test matches and used a cached value

Merge pull request #4607 from boingoing:FixRegExpTestCache

Regression from chakra-core/ChakraCore#3802 - if we hit the cache, we don't update the Regex constructor object with the last match and so retrieving properties about the match from the regex constructor object fails.

Fixed by adding an invalidation mechanism to the `JavascriptRegExpConstructor`. If we hit the cache, we'll mark the Regex constructor object last match properties as invalidated and we will then compute them on-demand the first time they're needed.

Fixes:
https://microsoft.visualstudio.com/web/wi.aspx?id=14763260

Reviewed-By: chakrabot <chakrabot@users.noreply.github.com>
  • Loading branch information
boingoing authored and kfarnung committed Feb 1, 2018
1 parent 340e097 commit 307513e
Show file tree
Hide file tree
Showing 6 changed files with 111 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ namespace Js
JavascriptRegExpConstructor::JavascriptRegExpConstructor(DynamicType * type) :
RuntimeFunction(type, &JavascriptRegExp::EntryInfo::NewInstance),
reset(false),
invalidatedLastMatch(false),
lastPattern(nullptr),
lastMatch() // undefined
{
Expand Down Expand Up @@ -53,17 +54,44 @@ namespace Js
this->lastInput = lastInput;
this->lastMatch = lastMatch;
this->reset = true;
this->invalidatedLastMatch = false;
}

void JavascriptRegExpConstructor::InvalidateLastMatch(UnifiedRegex::RegexPattern* lastPattern, JavascriptString* lastInput)
{
AssertMsg(lastPattern != nullptr, "lastPattern should not be null");
AssertMsg(lastInput != nullptr, "lastInput should not be null");
AssertMsg(JavascriptOperators::GetTypeId(lastInput) != TypeIds_Null, "lastInput should not be JavaScript null");

this->lastPattern = lastPattern;
this->lastInput = lastInput;
this->lastMatch.Reset();
this->reset = true;
this->invalidatedLastMatch = true;
}

void JavascriptRegExpConstructor::EnsureValues()
{
if (reset)
{
Assert(!lastMatch.IsUndefined());
ScriptContext* scriptContext = this->GetScriptContext();
const CharCount lastInputLen = lastInput->GetLength();
const char16* lastInputStr = lastInput->GetString();
UnifiedRegex::RegexPattern* pattern = lastPattern;

// When we perform a regex test operation it's possible the result of the operation will be loaded from a cache and the match will not be computed and updated in the ctor.
// In that case we invalidate the last match stored in the ctor and will need to compute it before it will be accessible via $1 etc.
// Since we only do this for the case of RegExp.prototype.test cache hit, we know several things:
// - The regex is not global or sticky
// - There was a match (test returned true)
if (invalidatedLastMatch)
{
this->lastMatch = RegexHelper::SimpleMatch(scriptContext, pattern, lastInputStr, lastInputLen, 0);
invalidatedLastMatch = false;
}

Assert(!lastMatch.IsUndefined());
JavascriptString* emptyString = scriptContext->GetLibrary()->GetEmptyString();
const CharCount lastInputLen = lastInput->GetLength();
// IE8 quirk: match of length 0 is regarded as length 1
CharCount lastIndexVal = lastMatch.EndOffset();
this->index = JavascriptNumber::ToVar(lastMatch.offset, scriptContext);
Expand All @@ -82,7 +110,7 @@ namespace Js
// every match is prohibitively slow. Instead, run the match again using the known last input string.
if (!pattern->WasLastMatchSuccessful())
{
RegexHelper::SimpleMatch(scriptContext, pattern, lastInput->GetString(), lastInputLen, lastMatch.offset);
RegexHelper::SimpleMatch(scriptContext, pattern, lastInputStr, lastInputLen, lastMatch.offset);
}
Assert(pattern->WasLastMatchSuccessful());
for (int groupId = 1; groupId < min(numGroups, NumCtorCaptures); groupId++)
Expand All @@ -100,6 +128,11 @@ namespace Js
captures[groupId] = emptyString;
reset = false;
}
else
{
// If we are not resetting the values, the last match cannot be invalidated.
Assert(!invalidatedLastMatch);
}
}

/*static*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,14 @@ namespace Js
bool GetPropertyBuiltIns(PropertyId propertyId, Var* value, BOOL* result);
bool SetPropertyBuiltIns(PropertyId propertyId, Var value, BOOL* result);
void SetLastMatch(UnifiedRegex::RegexPattern* lastPattern, JavascriptString* lastInput, UnifiedRegex::GroupInfo lastMatch);
void InvalidateLastMatch(UnifiedRegex::RegexPattern* lastPattern, JavascriptString* lastInput);

void EnsureValues();

Field(UnifiedRegex::RegexPattern*) lastPattern;
Field(JavascriptString*) lastInput;
Field(UnifiedRegex::GroupInfo) lastMatch;
Field(bool) invalidatedLastMatch; // true if last match must be recalculated before use
Field(bool) reset; // true if following fields must be recalculated from above before first use
Field(Var) lastParen;
Field(Var) lastIndex;
Expand Down
19 changes: 18 additions & 1 deletion deps/chakrashim/core/lib/Runtime/Library/RegexHelper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -691,13 +691,20 @@ namespace Js
{
Assert(useCache);
cachedResult = (cache->resultBV.Test(cacheIndex) != 0);

// If our cache says this test should produce a match (which we aren't going to compute),
// notify the Ctor to invalidate the last match so it must be recomputed before access.
if (cachedResult)
{
InvalidateLastMatchOnCtor(scriptContext, regularExpression, input);
}

// for debug builds, let's still do the real test so we can validate values in the cache
#if !DBG
return JavascriptBoolean::ToVar(cachedResult, scriptContext);
#endif
}


CharCount offset;
if (!GetInitialOffset(isGlobal, isSticky, regularExpression, inputLength, offset))
{
Expand Down Expand Up @@ -2087,6 +2094,16 @@ namespace Js
}
}

void RegexHelper::InvalidateLastMatchOnCtor(ScriptContext* scriptContext, JavascriptRegExp* regularExpression, JavascriptString* lastInput, bool useSplitPattern)
{
Assert(lastInput);

UnifiedRegex::RegexPattern* pattern = useSplitPattern
? regularExpression->GetSplitPattern()
: regularExpression->GetPattern();
scriptContext->GetLibrary()->GetRegExpConstructor()->InvalidateLastMatch(pattern, lastInput);
}

bool RegexHelper::GetInitialOffset(bool isGlobal, bool isSticky, JavascriptRegExp* regularExpression, CharCount inputLength, CharCount& offset)
{
if (isGlobal || isSticky)
Expand Down
2 changes: 2 additions & 0 deletions deps/chakrashim/core/lib/Runtime/Library/RegexHelper.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ namespace Js
, UnifiedRegex::GroupInfo lastSuccessfulMatch
, bool useSplitPattern );

static void InvalidateLastMatchOnCtor(ScriptContext* scriptContext, JavascriptRegExp* regularExpression, JavascriptString* lastInput, bool useSplitPattern = false);

static bool GetInitialOffset(bool isGlobal, bool isSticky, JavascriptRegExp* regularExpression, CharCount inputLength, CharCount& offset);
static JavascriptArray* CreateMatchResult(void *const stackAllocationPointer, ScriptContext* scriptContext, bool isGlobal, int numGroups, JavascriptString* input);
static void FinalizeMatchResult(ScriptContext* scriptContext, bool isGlobal, JavascriptArray* arr, UnifiedRegex::GroupInfo match);
Expand Down
47 changes: 47 additions & 0 deletions deps/chakrashim/core/test/Regex/bug_OS14763260.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
//-------------------------------------------------------------------------------------------------------
// 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 last match invalidated as expected",
body: function () {
const r1 = /(abc)/;
const r2 = /(def)/;
const s1 = "abc";
const s2 = " def";

r1.test(s1);

assert.areEqual("abc", RegExp.input, "RegExp.input property calculated correctly");
assert.areEqual("abc", RegExp['$_'], "RegExp.$_ property calculated correctly");
assert.areEqual("abc", RegExp.lastMatch, "RegExp.lastMatch property calculated correctly");
assert.areEqual("abc", RegExp['$&'], "RegExp.$& property calculated correctly");
assert.areEqual("abc", RegExp.$1, "RegExp.$1 property calculated correctly");
assert.areEqual(0, RegExp.index, "RegExp.index property calculated correctly");

r2.test(s2);

assert.areEqual(" def", RegExp.input, "RegExp.input property calculated correctly");
assert.areEqual(" def", RegExp['$_'], "RegExp.$_ property calculated correctly");
assert.areEqual("def", RegExp.lastMatch, "RegExp.lastMatch property calculated correctly");
assert.areEqual("def", RegExp['$&'], "RegExp.$& property calculated correctly");
assert.areEqual("def", RegExp.$1, "RegExp.$1 property calculated correctly");
assert.areEqual(1, RegExp.index, "RegExp.index property calculated correctly");

r1.test(s1);

assert.areEqual("abc", RegExp.input, "Stale RegExp.input property should be invalidated by second r1.test(s1)");
assert.areEqual("abc", RegExp['$_'], "Stale RegExp.$_ property should be invalidated by second r1.test(s1)");
assert.areEqual("abc", RegExp.lastMatch, "Stale RegExp.lastMatch should be invalidated by second r1.test(s1)");
assert.areEqual("abc", RegExp['$&'], "Stale RegExp.$& property should be invalidated by second r1.test(s1)");
assert.areEqual("abc", RegExp.$1, "Stale RegExp.$1 should be invalidated by second r1.test(s1)");
assert.areEqual(0, RegExp.index, "Stale RegExp.index property should be invalidated by second r1.test(s1)");
}
},
];

testRunner.runTests(tests, { verbose: WScript.Arguments[0] != "summary" });
6 changes: 6 additions & 0 deletions deps/chakrashim/core/test/Regex/rlexe.xml
Original file line number Diff line number Diff line change
Expand Up @@ -206,4 +206,10 @@
<compile-flags>-args summary -endargs</compile-flags>
</default>
</test>
<test>
<default>
<files>bug_OS14763260.js</files>
<compile-flags>-args summary -endargs</compile-flags>
</default>
</test>
</regress-exe>

0 comments on commit 307513e

Please sign in to comment.