Bug 1919901 - wasm: Correct clamping and signedeness for js-string:substring. r=bvisness

Differential Revision: https://phabricator.services.mozilla.com/D225292

js/src/jit-test/tests/wasm/builtin-modules/js-string/basic.js

@@ -279,11 +279,13 @@ let polyFillImports = {
     startIndex >>>= 0;
     endIndex >>>= 0;
     throwIfNotString(string);
-    if (startIndex > string.length,
-        endIndex > string.length,
+    if (startIndex > string.length ||
         endIndex < startIndex) {
       return "";
     }
+    if (endIndex > string.length) {
+      endIndex = string.length;
+    }
     return string.substring(startIndex, endIndex);
   },
   equals: (stringA, stringB) => {
@@ -406,7 +408,10 @@ for (let a of testStrings) {
   );
 
   for (let i = 0; i < length; i++) {
-    for (let j = 0; j < length; j++) {
+    // The end parameter is interpreted as unsigned and is always clamped to
+    // the string length. This means that -1, and string.length + 1 are valid
+    // end indices.
+    for (let j = -1; j <= length + 1; j++) {
       assertSameBehavior(
         builtinExports['substring'],
         polyfillExports['substring'],

js/src/wasm/WasmInstance.cpp

@@ -2175,7 +2175,7 @@ void* Instance::stringConcat(Instance* instance, void* firstStringArg,
 }
 
 void* Instance::stringSubstring(Instance* instance, void* stringArg,
-                                int32_t startIndex, int32_t endIndex) {
+                                uint32_t startIndex, uint32_t endIndex) {
   MOZ_ASSERT(SASigStringSubstring.failureMode == FailureMode::FailOnNullPtr);
   JSContext* cx = instance->cx();
 
@@ -2185,13 +2185,17 @@ void* Instance::stringSubstring(Instance* instance, void* stringArg,
     return nullptr;
   }
 
-  RootedString string(cx, stringRef.toJSString());
   static_assert(JS::MaxStringLength <= INT32_MAX);
-  if ((uint32_t)startIndex > string->length() ||
-      (uint32_t)endIndex > string->length() || startIndex > endIndex) {
+  RootedString string(cx, stringRef.toJSString());
+  uint32_t stringLength = string->length();
+  if (startIndex > stringLength || startIndex > endIndex) {
     return AnyRef::fromJSString(cx->names().empty_).forCompiledCode();
   }
 
+  if (endIndex > stringLength) {
+    endIndex = stringLength;
+  }
+
   JSString* result =
       SubstringKernel(cx, string, startIndex, endIndex - startIndex);
   if (!result) {

js/src/wasm/WasmInstance.h

@@ -630,7 +630,7 @@ class alignas(16) Instance {
   static void* stringConcat(Instance* instance, void* firstStringArg,
                             void* secondStringArg);
   static void* stringSubstring(Instance* instance, void* stringArg,
-                               int32_t startIndex, int32_t endIndex);
+                               uint32_t startIndex, uint32_t endIndex);
   static int32_t stringEquals(Instance* instance, void* firstStringArg,
                               void* secondStringArg);
   static int32_t stringCompare(Instance* instance, void* firstStringArg,

testing/web-platform/meta/wasm/jsapi/js-string/basic.tentative.any.js.ini

@@ -1,16 +1,3 @@
 prefs: [javascript.options.wasm_js_string_builtins:true]
 [basic.tentative.any.shadowrealm.html]
   expected: ERROR
-
-[basic.tentative.any.html]
-  [basic]
-    expected: FAIL
-
-
-[basic.tentative.any.worker.html]
-  [basic]
-    expected: FAIL
-
-[basic.tentative.any.js]
-  [Untitled]
-    expected: FAIL