Skip to content
Permalink
Browse files

RegExp operations should not take fast patch if lastIndex is not nume…

…ric.

https://bugs.webkit.org/show_bug.cgi?id=191731
<rdar://problem/46017305>

Reviewed by Saam Barati.

JSTests:

* stress/regress-191731.js: Added.

Source/JavaScriptCore:

This is because if lastIndex is an object with a valueOf() method, it can execute
arbitrary code which may have side effects, and side effects are not permitted by
the RegExp fast paths.

* builtins/RegExpPrototype.js:
(globalPrivate.hasObservableSideEffectsForRegExpMatch):
(overriddenName.string_appeared_here.search):
(globalPrivate.hasObservableSideEffectsForRegExpSplit):
(intrinsic.RegExpTestIntrinsic.test):
* builtins/StringPrototype.js:
(globalPrivate.hasObservableSideEffectsForStringReplace):



git-svn-id: http://svn.webkit.org/repository/webkit/trunk@238267 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information...
mark.lam@apple.com
mark.lam@apple.com committed Nov 16, 2018
1 parent 3af5ce1 commit 7cf9d2911af9f255e0301ea16604c9fa4af340e2
@@ -1,3 +1,13 @@
2018-11-15 Mark Lam <mark.lam@apple.com>

RegExp operations should not take fast patch if lastIndex is not numeric.
https://bugs.webkit.org/show_bug.cgi?id=191731
<rdar://problem/46017305>

Reviewed by Saam Barati.

* stress/regress-191731.js: Added.

2018-11-13 Saam Barati <sbarati@apple.com> 2018-11-13 Saam Barati <sbarati@apple.com>


TypeProfileLog::processLogEntries should stash away any pending exceptions and re-apply them to the VM TypeProfileLog::processLogEntries should stash away any pending exceptions and re-apply them to the VM
@@ -0,0 +1,27 @@
function assertEq(actual, expected) {
if (actual != expected)
throw ("Expected: " + expected + ", actual: " + actual);
}

function foo(arr, regexp, str) {
regexp[Symbol.match](str);
arr[1] = 3.54484805889626e-310;
return arr[0];
}

let arr = [1.1, 2.2, 3.3];
let regexp = /a/y;

for (let i = 0; i < 10000; i++)
foo(arr, regexp, "abcd");

regexp.lastIndex = {
valueOf: () => {
arr[0] = arr;
return 0;
}
};
let result = foo(arr, regexp, "abcd");

assertEq(arr[1], "3.54484805889626e-310");
assertEq(result, ",3.54484805889626e-310,3.3");
@@ -1,3 +1,23 @@
2018-11-15 Mark Lam <mark.lam@apple.com>

RegExp operations should not take fast patch if lastIndex is not numeric.
https://bugs.webkit.org/show_bug.cgi?id=191731
<rdar://problem/46017305>

Reviewed by Saam Barati.

This is because if lastIndex is an object with a valueOf() method, it can execute
arbitrary code which may have side effects, and side effects are not permitted by
the RegExp fast paths.

* builtins/RegExpPrototype.js:
(globalPrivate.hasObservableSideEffectsForRegExpMatch):
(overriddenName.string_appeared_here.search):
(globalPrivate.hasObservableSideEffectsForRegExpSplit):
(intrinsic.RegExpTestIntrinsic.test):
* builtins/StringPrototype.js:
(globalPrivate.hasObservableSideEffectsForStringReplace):

2018-11-15 Keith Rollin <krollin@apple.com> 2018-11-15 Keith Rollin <krollin@apple.com>


Delete old .xcfilelist files Delete old .xcfilelist files
@@ -1,5 +1,5 @@
/* /*
* Copyright (C) 2016 Apple Inc. All rights reserved. * Copyright (C) 2016-2018 Apple Inc. All rights reserved.
* *
* Redistribution and use in source and binary forms, with or without * Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions * modification, are permitted provided that the following conditions
@@ -67,6 +67,9 @@ function hasObservableSideEffectsForRegExpMatch(regexp)
{ {
"use strict"; "use strict";


if (!@isRegExpObject(regexp))
return true;

// This is accessed by the RegExpExec internal function. // This is accessed by the RegExpExec internal function.
let regexpExec = @tryGetById(regexp, "exec"); let regexpExec = @tryGetById(regexp, "exec");
if (regexpExec !== @regExpBuiltinExec) if (regexpExec !== @regExpBuiltinExec)
@@ -79,7 +82,7 @@ function hasObservableSideEffectsForRegExpMatch(regexp)
if (regexpUnicode !== @regExpProtoUnicodeGetter) if (regexpUnicode !== @regExpProtoUnicodeGetter)
return true; return true;


return !@isRegExpObject(regexp); return typeof regexp.lastIndex !== "number";
} }


@globalPrivate @globalPrivate
@@ -315,7 +318,9 @@ function search(strArg)
let regexp = this; let regexp = this;


// Check for observable side effects and call the fast path if there aren't any. // Check for observable side effects and call the fast path if there aren't any.
if (@isRegExpObject(regexp) && @tryGetById(regexp, "exec") === @regExpBuiltinExec) if (@isRegExpObject(regexp)
&& @tryGetById(regexp, "exec") === @regExpBuiltinExec
&& typeof regexp.lastIndex === "number")
return @regExpSearchFast.@call(regexp, strArg); return @regExpSearchFast.@call(regexp, strArg);


// 1. Let rx be the this value. // 1. Let rx be the this value.
@@ -358,6 +363,9 @@ function hasObservableSideEffectsForRegExpSplit(regexp)
{ {
"use strict"; "use strict";


if (!@isRegExpObject(regexp))
return true;

// This is accessed by the RegExpExec internal function. // This is accessed by the RegExpExec internal function.
let regexpExec = @tryGetById(regexp, "exec"); let regexpExec = @tryGetById(regexp, "exec");
if (regexpExec !== @regExpBuiltinExec) if (regexpExec !== @regExpBuiltinExec)
@@ -389,8 +397,8 @@ function hasObservableSideEffectsForRegExpSplit(regexp)
let regexpSource = @tryGetById(regexp, "source"); let regexpSource = @tryGetById(regexp, "source");
if (regexpSource !== @regExpProtoSourceGetter) if (regexpSource !== @regExpProtoSourceGetter)
return true; return true;

return !@isRegExpObject(regexp); return typeof regexp.lastIndex !== "number";
} }


// ES 21.2.5.11 RegExp.prototype[@@split](string, limit) // ES 21.2.5.11 RegExp.prototype[@@split](string, limit)
@@ -536,7 +544,9 @@ function test(strArg)
let regexp = this; let regexp = this;


// Check for observable side effects and call the fast path if there aren't any. // Check for observable side effects and call the fast path if there aren't any.
if (@isRegExpObject(regexp) && @tryGetById(regexp, "exec") === @regExpBuiltinExec) if (@isRegExpObject(regexp)
&& @tryGetById(regexp, "exec") === @regExpBuiltinExec
&& typeof regexp.lastIndex === "number")
return @regExpTestFast.@call(regexp, strArg); return @regExpTestFast.@call(regexp, strArg);


// 1. Let R be the this value. // 1. Let R be the this value.
@@ -1,7 +1,7 @@
/* /*
* Copyright (C) 2015 Andy VanWagoner <andy@vanwagoner.family>. * Copyright (C) 2015 Andy VanWagoner <andy@vanwagoner.family>.
* Copyright (C) 2016 Yusuke Suzuki <utatane.tea@gmail.com> * Copyright (C) 2016 Yusuke Suzuki <utatane.tea@gmail.com>
* Copyright (C) 2016 Apple Inc. All rights reserved. * Copyright (C) 2016-2018 Apple Inc. All rights reserved.
* *
* Redistribution and use in source and binary forms, with or without * Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions * modification, are permitted provided that the following conditions
@@ -195,6 +195,9 @@ function hasObservableSideEffectsForStringReplace(regexp, replacer)
{ {
"use strict"; "use strict";


if (!@isRegExpObject(regexp))
return true;

if (replacer !== @regExpPrototypeSymbolReplace) if (replacer !== @regExpPrototypeSymbolReplace)
return true; return true;


@@ -210,7 +213,7 @@ function hasObservableSideEffectsForStringReplace(regexp, replacer)
if (regexpUnicode !== @regExpProtoUnicodeGetter) if (regexpUnicode !== @regExpProtoUnicodeGetter)
return true; return true;


return !@isRegExpObject(regexp); return typeof regexp.lastIndex !== "number";
} }


@intrinsic=StringPrototypeReplaceIntrinsic @intrinsic=StringPrototypeReplaceIntrinsic

0 comments on commit 7cf9d29

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