Bug 244196 - String.prototype.includes incorrectly returns false when string is empty and position is past end of string
Summary: String.prototype.includes incorrectly returns false when string is empty and ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Local Build
Hardware: All All
: P2 Normal
Assignee: Darin Adler
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-08-22 04:27 PDT by EntryHi
Modified: 2022-09-09 12:47 PDT (History)
5 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description EntryHi 2022-08-22 04:27:04 PDT
function foo(n) {
    return ("aaa").includes("", n)
}
noInline(foo);
for (var m = 0; m < 200; ++m) {
  print(foo(m));
}

With the above script as input to JSC, run JSC with the following parameters:
./jsc test.js --useConcurrentJIT=0 --jitPolicyScale=1

According to ECMAScript Language Specification, when the first parameter of String.prototype.includes is empty, the result should be true regardless of whether the second parameter is greater than length. However, JSC prints false when n exceeds length.

The problem is in StringPrototype.cpp stringIncluesImpl. When positionArg is Int32, it should be clamped. Otherwise, the function findCommon will return notFound and the final result will be false.
Comment 1 Alexey Proskuryakov 2022-08-22 08:48:07 PDT
On macOS, this prints 4x "true" and 196x "false".
Comment 2 Darin Adler 2022-08-22 09:03:59 PDT
The suggested change sounds OK. There are other ways to fix it as well. Makes sense that JavaScript calls for this behavior, since it's consistent with indexOf.

I am surprised this is not covered by Test262. Maybe this function is too new?

It looks like the startsWith and endsWith implementations may have a similar bug.
Comment 3 Radar WebKit Bug Importer 2022-08-29 04:28:16 PDT
<rdar://problem/99271518>
Comment 4 Darin Adler 2022-09-09 11:40:11 PDT
Pull request: https://github.com/WebKit/WebKit/pull/4180
Comment 5 EWS 2022-09-09 12:46:59 PDT
Committed 254319@main (ebf196ed5724): <https://commits.webkit.org/254319@main>

Reviewed commits have been landed. Closing PR #4180 and removing active labels.