Skip to content

Commit

Permalink
[regexp] Avoid side effects between map load and fast path check
Browse files Browse the repository at this point in the history
Loading the map, performing a side-effect, and then using the stored
pointer for the fast-path check is another antipattern that can lead to
unintended shapes on the fast path.

Backmerge of commit db61537.

BUG=chromium:709029
TBR=yangguo@chromium.org
NOPRESUBMIT=true
NOTRY=true

Review-Url: https://codereview.chromium.org/2818683005
Cr-Commit-Position: refs/branch-heads/5.8@{crosswalk-project#67}
Cr-Branched-From: eda659c-refs/heads/5.8.283@{crosswalk-project#1}
Cr-Branched-From: 4310cd0-refs/heads/master@{#43429}
  • Loading branch information
schuay authored and Commit bot committed Apr 17, 2017
1 parent 39cf343 commit 5c39a61
Showing 1 changed file with 27 additions and 24 deletions.
51 changes: 27 additions & 24 deletions src/builtins/builtins-regexp.cc
Original file line number Diff line number Diff line change
Expand Up @@ -514,16 +514,16 @@ TF_BUILTIN(RegExpPrototypeExec, RegExpBuiltinsAssembler) {
Node* const context = Parameter(4);

// Ensure {maybe_receiver} is a JSRegExp.
Node* const regexp_map = ThrowIfNotInstanceType(
context, maybe_receiver, JS_REGEXP_TYPE, "RegExp.prototype.exec");
ThrowIfNotInstanceType(context, maybe_receiver, JS_REGEXP_TYPE,
"RegExp.prototype.exec");
Node* const receiver = maybe_receiver;

// Convert {maybe_string} to a String.
Node* const string = ToString(context, maybe_string);

Label if_isfastpath(this), if_isslowpath(this);
Branch(IsInitialRegExpMap(context, receiver, regexp_map), &if_isfastpath,
&if_isslowpath);
Branch(IsInitialRegExpMap(context, receiver, LoadMap(receiver)),
&if_isfastpath, &if_isslowpath);

Bind(&if_isfastpath);
{
Expand Down Expand Up @@ -1315,16 +1315,17 @@ TF_BUILTIN(RegExpPrototypeTest, RegExpBuiltinsAssembler) {
Node* const context = Parameter(4);

// Ensure {maybe_receiver} is a JSReceiver.
Node* const map = ThrowIfNotJSReceiver(
context, maybe_receiver, MessageTemplate::kIncompatibleMethodReceiver,
"RegExp.prototype.test");
ThrowIfNotJSReceiver(context, maybe_receiver,
MessageTemplate::kIncompatibleMethodReceiver,
"RegExp.prototype.test");
Node* const receiver = maybe_receiver;

// Convert {maybe_string} to a String.
Node* const string = ToString(context, maybe_string);

Label fast_path(this), slow_path(this);
BranchIfFastRegExp(context, receiver, map, &fast_path, &slow_path);
BranchIfFastRegExp(context, receiver, LoadMap(receiver), &fast_path,
&slow_path);

Bind(&fast_path);
{
Expand Down Expand Up @@ -1724,16 +1725,17 @@ TF_BUILTIN(RegExpPrototypeMatch, RegExpBuiltinsAssembler) {
Node* const context = Parameter(4);

// Ensure {maybe_receiver} is a JSReceiver.
Node* const map = ThrowIfNotJSReceiver(
context, maybe_receiver, MessageTemplate::kIncompatibleMethodReceiver,
"RegExp.prototype.@@match");
ThrowIfNotJSReceiver(context, maybe_receiver,
MessageTemplate::kIncompatibleMethodReceiver,
"RegExp.prototype.@@match");
Node* const receiver = maybe_receiver;

// Convert {maybe_string} to a String.
Node* const string = ToString(context, maybe_string);

Label fast_path(this), slow_path(this);
BranchIfFastRegExp(context, receiver, map, &fast_path, &slow_path);
BranchIfFastRegExp(context, receiver, LoadMap(receiver), &fast_path,
&slow_path);

Bind(&fast_path);
RegExpPrototypeMatchBody(context, receiver, string, true);
Expand Down Expand Up @@ -1849,16 +1851,17 @@ TF_BUILTIN(RegExpPrototypeSearch, RegExpBuiltinsAssembler) {
Node* const context = Parameter(4);

// Ensure {maybe_receiver} is a JSReceiver.
Node* const map = ThrowIfNotJSReceiver(
context, maybe_receiver, MessageTemplate::kIncompatibleMethodReceiver,
"RegExp.prototype.@@search");
ThrowIfNotJSReceiver(context, maybe_receiver,
MessageTemplate::kIncompatibleMethodReceiver,
"RegExp.prototype.@@search");
Node* const receiver = maybe_receiver;

// Convert {maybe_string} to a String.
Node* const string = ToString(context, maybe_string);

Label fast_path(this), slow_path(this);
BranchIfFastRegExp(context, receiver, map, &fast_path, &slow_path);
BranchIfFastRegExp(context, receiver, LoadMap(receiver), &fast_path,
&slow_path);

Bind(&fast_path);
RegExpPrototypeSearchBodyFast(context, receiver, string);
Expand Down Expand Up @@ -2185,16 +2188,16 @@ TF_BUILTIN(RegExpPrototypeSplit, RegExpBuiltinsAssembler) {
Node* const context = Parameter(5);

// Ensure {maybe_receiver} is a JSReceiver.
Node* const map = ThrowIfNotJSReceiver(
context, maybe_receiver, MessageTemplate::kIncompatibleMethodReceiver,
"RegExp.prototype.@@split");
ThrowIfNotJSReceiver(context, maybe_receiver,
MessageTemplate::kIncompatibleMethodReceiver,
"RegExp.prototype.@@split");
Node* const receiver = maybe_receiver;

// Convert {maybe_string} to a String.
Node* const string = ToString(context, maybe_string);

Label stub(this), runtime(this, Label::kDeferred);
BranchIfFastRegExp(context, receiver, map, &stub, &runtime);
BranchIfFastRegExp(context, receiver, LoadMap(receiver), &stub, &runtime);

Bind(&stub);
Callable split_callable = CodeFactory::RegExpSplit(isolate());
Expand Down Expand Up @@ -2609,9 +2612,9 @@ TF_BUILTIN(RegExpPrototypeReplace, RegExpBuiltinsAssembler) {
// }

// Ensure {maybe_receiver} is a JSReceiver.
Node* const map = ThrowIfNotJSReceiver(
context, maybe_receiver, MessageTemplate::kIncompatibleMethodReceiver,
"RegExp.prototype.@@replace");
ThrowIfNotJSReceiver(context, maybe_receiver,
MessageTemplate::kIncompatibleMethodReceiver,
"RegExp.prototype.@@replace");
Node* const receiver = maybe_receiver;

// Convert {maybe_string} to a String.
Expand All @@ -2620,7 +2623,7 @@ TF_BUILTIN(RegExpPrototypeReplace, RegExpBuiltinsAssembler) {

// Fast-path checks: 1. Is the {receiver} an unmodified JSRegExp instance?
Label stub(this), runtime(this, Label::kDeferred);
BranchIfFastRegExp(context, receiver, map, &stub, &runtime);
BranchIfFastRegExp(context, receiver, LoadMap(receiver), &stub, &runtime);

Bind(&stub);
Callable replace_callable = CodeFactory::RegExpReplace(isolate());
Expand Down

0 comments on commit 5c39a61

Please sign in to comment.