[wasm] Introduce new heap type 'imported string'

Liedtke · V8 LUCI CQ · commit 841c7c1117f8 · 2024-02-15T14:43:41.000Z
This heap type does not exist in the wasm spec. It is used as an internal type to mark externref values in our optimizations for which we know that it is a (potentially nullable) JS string. Previously the wasm string type from the stringref proposal was used for that. This can cause all kinds of issues as ref.string is a subtype of any, not extern. (It also uses a different null representation.) Fixed: chromium:324747822 Change-Id: I9cc2975b8612510de18e02d3b66ed54489d9cc22 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5293801 Reviewed-by: Jakob Kummerow <jkummerow@chromium.org> Commit-Queue: Matthias Liedtke <mliedtke@chromium.org> Cr-Commit-Position: refs/heads/main@{#92348}
diff --git a/src/compiler/turboshaft/wasm-gc-type-reducer.h b/src/compiler/turboshaft/wasm-gc-type-reducer.h
@@ -137,6 +137,8 @@ class WasmGCTypeReducer : public Next {
 
     wasm::ValueType type = analyzer_.GetInputType(op_idx);
     if (type != wasm::ValueType() && !type.is_uninhabited()) {
+      DCHECK(wasm::IsSameTypeHierarchy(type.heap_type(),
+                                       cast_op.config.to.heap_type(), module_));
       bool to_nullable = cast_op.config.to.is_nullable();
       if (wasm::IsHeapSubtypeOf(type.heap_type(), cast_op.config.to.heap_type(),
                                 module_, module_)) {
@@ -154,8 +156,7 @@ class WasmGCTypeReducer : public Next {
       }
       if (wasm::HeapTypesUnrelated(type.heap_type(),
                                    cast_op.config.to.heap_type(), module_,
-                                   module_) &&
-          !wasm::IsImplicitInternalization(type, cast_op.config.to, module_)) {
+                                   module_)) {
         // A cast between unrelated types can only succeed if the argument is
         // null. Otherwise, it always fails.
         V<Word32> non_trapping_condition =
@@ -191,6 +192,8 @@ class WasmGCTypeReducer : public Next {
 
     wasm::ValueType type = analyzer_.GetInputType(op_idx);
     if (type != wasm::ValueType() && !type.is_uninhabited()) {
+      DCHECK(wasm::IsSameTypeHierarchy(
+          type.heap_type(), type_check.config.to.heap_type(), module_));
       bool to_nullable = type_check.config.to.is_nullable();
       if (wasm::IsHeapSubtypeOf(type.heap_type(),
                                 type_check.config.to.heap_type(), module_,
@@ -208,9 +211,7 @@ class WasmGCTypeReducer : public Next {
       }
       if (wasm::HeapTypesUnrelated(type.heap_type(),
                                    type_check.config.to.heap_type(), module_,
-                                   module_) &&
-          !wasm::IsImplicitInternalization(type, type_check.config.to,
-                                           module_)) {
+                                   module_)) {
         if (to_nullable && type.is_nullable()) {
           return __ IsNull(__ MapToNewGraph(type_check.object()), type);
         } else {
diff --git a/src/compiler/turboshaft/wasm-lowering-reducer.h b/src/compiler/turboshaft/wasm-lowering-reducer.h
@@ -595,7 +595,8 @@ class WasmLoweringReducer : public Next {
         result = __ HasInstanceType(object, WASM_STRUCT_TYPE);
         break;
       }
-      if (to_rep == wasm::HeapType::kString) {
+      if (to_rep == wasm::HeapType::kString ||
+          to_rep == wasm::HeapType::kExternString) {
         V<Word32> instance_type =
             __ LoadInstanceTypeField(__ LoadMapField(object));
         result = __ Uint32LessThan(instance_type,
@@ -671,7 +672,8 @@ class WasmLoweringReducer : public Next {
                      OpIndex::Invalid(), TrapId::kTrapIllegalCast);
         break;
       }
-      if (to_rep == wasm::HeapType::kString) {
+      if (to_rep == wasm::HeapType::kString ||
+          to_rep == wasm::HeapType::kExternString) {
         V<Word32> instance_type =
             __ LoadInstanceTypeField(__ LoadMapField(object));
         __ TrapIfNot(__ Uint32LessThan(instance_type,
diff --git a/src/wasm/turboshaft-graph-interface.cc b/src/wasm/turboshaft-graph-interface.cc
@@ -1067,14 +1067,14 @@ class TurboshaftGraphBuildingInterface {
   }
 
   V<Word32> IsExternRefString(const Value value) {
-    compiler::WasmTypeCheckConfig config{value.type, kWasmRefString};
+    compiler::WasmTypeCheckConfig config{value.type, kWasmRefExternString};
     V<Map> rtt = OpIndex::Invalid();
     return __ WasmTypeCheck(value.op, rtt, config);
   }
 
   V<String> ExternRefToString(const Value value, bool null_succeeds = false) {
     wasm::ValueType target_type =
-        null_succeeds ? kWasmStringRef : kWasmRefString;
+        null_succeeds ? kWasmRefNullExternString : kWasmRefExternString;
     compiler::WasmTypeCheckConfig config{value.type, target_type};
     V<Map> rtt = OpIndex::Invalid();
     return V<String>::Cast(__ WasmTypeCast(value.op, rtt, config));
@@ -1084,7 +1084,7 @@ class TurboshaftGraphBuildingInterface {
     if (__ generating_unreachable_operations()) return false;
     const WasmTypeCastOp* cast =
         __ output_graph().Get(value.op).TryCast<WasmTypeCastOp>();
-    return cast && cast->config.to == kWasmRefString;
+    return cast && cast->config.to == kWasmRefExternString;
   }
 
   V<Word32> GetStringIndexOf(FullDecoder* decoder, V<String> string,
@@ -1155,7 +1155,7 @@ class TurboshaftGraphBuildingInterface {
         BuiltinCallDescriptor::StringToLowerCaseIntl>(
         decoder, __ NoContextConstant(), {string});
     BuildModifyThreadInWasmFlag(decoder, true);
-    return __ AnnotateWasmType(result, kWasmRefString);
+    return result;
   }
 #endif
 
@@ -1403,6 +1403,19 @@ class TurboshaftGraphBuildingInterface {
                             GetExternalArrayType(op_type));
   }
 
+  // Adds a wasm type annotation to the graph and replaces any extern type with
+  // the extern string type.
+  V<String> AnnotateAsString(OpIndex value, wasm::ValueType type) {
+    DCHECK(type.is_reference_to(HeapType::kString) ||
+           type.is_reference_to(HeapType::kExternString) ||
+           type.is_reference_to(HeapType::kExtern));
+    if (type.is_reference_to(HeapType::kExtern)) {
+      type =
+          ValueType::RefMaybeNull(HeapType::kExternString, type.nullability());
+    }
+    return __ AnnotateWasmType(value, type);
+  }
+
   bool HandleWellKnownImport(FullDecoder* decoder, uint32_t index,
                              const Value args[], Value returns[]) {
     if (!decoder->module_) return false;  // Only needed for tests.
@@ -1461,7 +1474,7 @@ class TurboshaftGraphBuildingInterface {
             BuiltinCallDescriptor::StringAdd_CheckNone>(
             decoder, V<Context>::Cast(native_context),
             {head_string, tail_string});
-        result = __ AnnotateWasmType(result, kWasmRefString);
+        result = __ AnnotateWasmType(result, kWasmRefExternString);
         decoder->detected_->Add(kFeature_imported_strings);
         break;
       }
@@ -1480,7 +1493,7 @@ class TurboshaftGraphBuildingInterface {
         V<Word32> capped = __ Word32BitwiseAnd(args[0].op, 0xFFFF);
         result = CallBuiltinThroughJumptable<
             BuiltinCallDescriptor::WasmStringFromCodePoint>(decoder, {capped});
-        result = __ AnnotateWasmType(result, kWasmRefString);
+        result = __ AnnotateWasmType(result, kWasmRefExternString);
         decoder->detected_->Add(kFeature_imported_strings);
         break;
       }
@@ -1489,21 +1502,21 @@ class TurboshaftGraphBuildingInterface {
         result = CallBuiltinThroughJumptable<
             BuiltinCallDescriptor::WasmStringFromCodePoint>(decoder,
                                                             {args[0].op});
-        result = __ AnnotateWasmType(result, kWasmRefString);
+        result = __ AnnotateWasmType(result, kWasmRefExternString);
         decoder->detected_->Add(kFeature_imported_strings);
         break;
       case WKI::kStringFromWtf16Array:
         result = CallBuiltinThroughJumptable<
             BuiltinCallDescriptor::WasmStringNewWtf16Array>(
             decoder,
             {V<WasmArray>::Cast(NullCheck(args[0])), args[1].op, args[2].op});
-        result = __ AnnotateWasmType(result, kWasmRefString);
+        result = __ AnnotateWasmType(result, kWasmRefExternString);
         decoder->detected_->Add(kFeature_imported_strings);
         break;
       case WKI::kStringFromUtf8Array:
-        result =
-            StringNewWtf8ArrayImpl(decoder, unibrow::Utf8Variant::kLossyUtf8,
-                                   args[0], args[1], args[2]);
+        result = StringNewWtf8ArrayImpl(
+            decoder, unibrow::Utf8Variant::kLossyUtf8, args[0], args[1],
+            args[2], kWasmRefExternString);
         decoder->detected_->Add(kFeature_imported_strings);
         break;
       case WKI::kStringIntoUtf8Array: {
@@ -1544,7 +1557,7 @@ class TurboshaftGraphBuildingInterface {
         result = CallBuiltinThroughJumptable<
             BuiltinCallDescriptor::WasmStringViewWtf16Slice>(
             decoder, {V<String>::Cast(view), args[1].op, args[2].op});
-        result = __ AnnotateWasmType(result, kWasmRefString);
+        result = __ AnnotateWasmType(result, kWasmRefExternString);
         decoder->detected_->Add(kFeature_imported_strings);
         break;
       }
@@ -1563,7 +1576,7 @@ class TurboshaftGraphBuildingInterface {
         BuildModifyThreadInWasmFlag(decoder, false);
         result = CallBuiltinThroughJumptable<
             BuiltinCallDescriptor::WasmFloat64ToString>(decoder, {args[0].op});
-        result = __ AnnotateWasmType(result, kWasmRefString);
+        result = AnnotateAsString(result, returns[0].type);
         BuildModifyThreadInWasmFlag(decoder, true);
         decoder->detected_->Add(
             returns[0].type.is_reference_to(wasm::HeapType::kString)
@@ -1575,7 +1588,7 @@ class TurboshaftGraphBuildingInterface {
         result =
             CallBuiltinThroughJumptable<BuiltinCallDescriptor::WasmIntToString>(
                 decoder, {args[0].op, args[1].op});
-        result = __ AnnotateWasmType(result, kWasmRefString);
+        result = AnnotateAsString(result, returns[0].type);
         BuildModifyThreadInWasmFlag(decoder, true);
         decoder->detected_->Add(
             returns[0].type.is_reference_to(wasm::HeapType::kString)
@@ -1585,7 +1598,7 @@ class TurboshaftGraphBuildingInterface {
       case WKI::kParseFloat: {
         if (args[0].type.is_nullable()) {
           Label<Float64> done(&asm_);
-          GOTO_IF(__ IsNull(args[0].op, wasm::kWasmStringRef), done,
+          GOTO_IF(__ IsNull(args[0].op, args[0].type), done,
                   __ Float64Constant(std::numeric_limits<double>::quiet_NaN()));
 
           BuildModifyThreadInWasmFlag(decoder, false);
@@ -1612,7 +1625,7 @@ class TurboshaftGraphBuildingInterface {
 
         // If string is null, throw.
         if (args[0].type.is_nullable()) {
-          IF (__ IsNull(string, wasm::kWasmStringRef)) {
+          IF (__ IsNull(string, args[0].type)) {
             CallBuiltinThroughJumptable<
                 BuiltinCallDescriptor::ThrowIndexOfCalledOnNull>(decoder, {});
             __ Unreachable();
@@ -1623,8 +1636,8 @@ class TurboshaftGraphBuildingInterface {
         // If search is null, replace it with "null".
         if (args[1].type.is_nullable()) {
           Label<String> search_done_label(&asm_);
-          GOTO_IF_NOT(__ IsNull(search, wasm::kWasmStringRef),
-                      search_done_label, search);
+          GOTO_IF_NOT(__ IsNull(search, args[1].type), search_done_label,
+                      search);
           GOTO(search_done_label, LOAD_ROOT(null_string));
           BIND(search_done_label, search_value);
           search = search_value;
@@ -1656,7 +1669,7 @@ class TurboshaftGraphBuildingInterface {
 #if V8_INTL_SUPPORT
         V<String> string = args[0].op;
         if (args[0].type.is_nullable()) {
-          IF (__ IsNull(string, wasm::kWasmStringRef)) {
+          IF (__ IsNull(string, args[0].type)) {
             CallBuiltinThroughJumptable<
                 BuiltinCallDescriptor::ThrowToLowerCaseCalledOnNull>(decoder,
                                                                      {});
@@ -1665,6 +1678,7 @@ class TurboshaftGraphBuildingInterface {
           END_IF
         }
         result = CallStringToLowercase(decoder, string);
+        __ AnnotateWasmType(result, kWasmRefString);
         decoder->detected_->Add(kFeature_stringref);
         break;
 #else
@@ -1681,6 +1695,7 @@ class TurboshaftGraphBuildingInterface {
         }
         V<String> string = args[0].op;
         result = CallStringToLowercase(decoder, string);
+        __ AnnotateWasmType(result, kWasmRefExternString);
         decoder->detected_->Add(kFeature_imported_strings);
         break;
 #else
@@ -3347,7 +3362,7 @@ class TurboshaftGraphBuildingInterface {
   V<Tagged> StringNewWtf8ArrayImpl(FullDecoder* decoder,
                                    const unibrow::Utf8Variant variant,
                                    const Value& array, const Value& start,
-                                   const Value& end) {
+                                   const Value& end, ValueType result_type) {
     // Special case: shortcut a sequence "array from data segment" + "string
     // from wtf8 array" to directly create a string from the segment.
     V<Tagged> call;
@@ -3379,16 +3394,20 @@ class TurboshaftGraphBuildingInterface {
           {start.op, end.op, V<WasmArray>::Cast(NullCheck(array)),
            __ SmiConstant(Smi::FromInt(static_cast<int32_t>(variant)))});
     }
-    bool null_on_invalid = variant == unibrow::Utf8Variant::kUtf8NoTrap;
-    return __ AnnotateWasmType(
-        call, null_on_invalid ? kWasmStringRef : kWasmRefString);
+    DCHECK_IMPLIES(variant == unibrow::Utf8Variant::kUtf8NoTrap,
+                   result_type.is_nullable());
+    // The builtin returns a WasmNull for kUtf8NoTrap, so nullable values in
+    // combination with extern strings are not supported.
+    DCHECK_NE(result_type, wasm::kWasmExternRef);
+    return AnnotateAsString(call, result_type);
   }
 
   void StringNewWtf8Array(FullDecoder* decoder,
                           const unibrow::Utf8Variant variant,
                           const Value& array, const Value& start,
                           const Value& end, Value* result) {
-    result->op = StringNewWtf8ArrayImpl(decoder, variant, array, start, end);
+    result->op = StringNewWtf8ArrayImpl(decoder, variant, array, start, end,
+                                        result->type);
   }
 
   void StringNewWtf16(FullDecoder* decoder, const MemoryIndexImmediate& imm,
diff --git a/src/wasm/value-type.h b/src/wasm/value-type.h
@@ -66,6 +66,9 @@ class HeapType {
     kArray,                   // shorthand: g
     kAny,                     //
     kExtern,                  // shorthand: a.
+    kExternString,            // Internal type for optimization purposes.
+                              // Subtype of extern.
+                              // Used by the js-builtin-strings proposal.
     kExn,                     //
     kString,                  // shorthand: w.
     kStringViewWtf8,          // shorthand: x.
@@ -172,6 +175,8 @@ class HeapType {
         return std::string("array");
       case kExtern:
         return std::string("extern");
+      case kExternString:
+        return std::string("<extern_string>");
       case kAny:
         return std::string("any");
       case kString:
@@ -745,6 +750,10 @@ constexpr ValueType kWasmStructRef = ValueType::RefNull(HeapType::kStruct);
 constexpr ValueType kWasmArrayRef = ValueType::RefNull(HeapType::kArray);
 constexpr ValueType kWasmStringRef = ValueType::RefNull(HeapType::kString);
 constexpr ValueType kWasmRefString = ValueType::Ref(HeapType::kString);
+constexpr ValueType kWasmRefNullExternString =
+    ValueType::RefNull(HeapType::kExternString);
+constexpr ValueType kWasmRefExternString =
+    ValueType::Ref(HeapType::kExternString);
 constexpr ValueType kWasmStringViewWtf8 =
     ValueType::RefNull(HeapType::kStringViewWtf8);
 constexpr ValueType kWasmStringViewWtf16 =
diff --git a/src/wasm/wasm-subtyping.cc b/src/wasm/wasm-subtyping.cc
diff --git a/test/unittests/wasm/subtyping-unittest.cc b/test/unittests/wasm/subtyping-unittest.cc
