Merge pull request swiftlang#252 from kateinoigakukun/katei/fix-keypath

[WASM] Fix KeyPath on WebAssembly
kateinoigakukun · Feb 27, 2020 · a9fb8ae · a9fb8ae
2 parents e9f0bda + 60186b6
commit a9fb8ae
Show file tree

Hide file tree

Showing 4 changed files with 51 additions and 9 deletions.
diff --git a/lib/IRGen/GenKeyPath.cpp b/lib/IRGen/GenKeyPath.cpp
@@ -222,7 +222,8 @@ getAccessorForComputedComponent(IRGenModule &IGM,
       componentArgsBuf = params.claimNext();
       // Pass the argument pointer down to the underlying function, if it
       // wants it.
-      if (hasSubscriptIndices) {
+      // Always forward extra argument to match callee and caller signature on WebAssembly
+      if (hasSubscriptIndices || IGM.TargetInfo.OutputObjectFormat == llvm::Triple::Wasm) {
         forwardedArgs.add(componentArgsBuf);
       }
       break;

diff --git a/lib/SIL/SILVerifier.cpp b/lib/SIL/SILVerifier.cpp
@@ -277,7 +277,7 @@ void verifyKeyPathComponent(SILModule &M,
                 SILFunctionTypeRepresentation::Thin,
               "getter should be a thin function");
 
-      require(substGetterType->getNumParameters() == 1 + hasIndices,
+      require(substGetterType->getNumParameters() == 1 + (hasIndices || C.LangOpts.Target.isOSBinFormatWasm()),
               "getter should have one parameter");
       auto baseParam = substGetterType->getParameters()[0];
       require(baseParam.getConvention() == normalArgConvention,
@@ -325,7 +325,7 @@ void verifyKeyPathComponent(SILModule &M,
                 SILFunctionTypeRepresentation::Thin,
               "setter should be a thin function");
 
-      require(substSetterType->getNumParameters() == 2 + hasIndices,
+      require(substSetterType->getNumParameters() == 2 + (hasIndices || C.LangOpts.Target.isOSBinFormatWasm()),
               "setter should have two parameters");
 
       auto newValueParam = substSetterType->getParameters()[0];

diff --git a/lib/SILGen/SILGenExpr.cpp b/lib/SILGen/SILGenExpr.cpp
@@ -2680,6 +2680,7 @@ static SILFunction *getOrCreateKeyPathGetter(SILGenModule &SGM,
     }
   }
 
+  auto Target = SGM.getASTContext().LangOpts.Target;
   auto genericSig =
       genericEnv ? genericEnv->getGenericSignature().getCanonicalSignature()
                  : nullptr;
@@ -2688,6 +2689,14 @@ static SILFunction *getOrCreateKeyPathGetter(SILGenModule &SGM,
     genericEnv = nullptr;
   }
 
+  // Add empty generic type parameter to match function signature on WebAssembly
+  if (!genericSig && Target.isOSBinFormatWasm()) {
+    auto param = GenericTypeParamType::get(0, 0, SGM.getASTContext());
+    auto sig = GenericSignature::get(param, { });
+    genericSig = CanGenericSignature(sig);
+    genericEnv = sig->getGenericEnvironment();
+  }
+
   // Build the signature of the thunk as expected by the keypath runtime.
   auto signature = [&]() {
     CanType loweredBaseTy, loweredPropTy;
@@ -2703,7 +2712,8 @@ static SILFunction *getOrCreateKeyPathGetter(SILGenModule &SGM,
     SmallVector<SILParameterInfo, 2> params;
     params.push_back({loweredBaseTy, paramConvention});
     auto &C = SGM.getASTContext();
-    if (!indexes.empty())
+    // Always take indexes parameter to match callee and caller signature on WebAssembly
+    if (!indexes.empty() || C.LangOpts.Target.isOSBinFormatWasm())
       params.push_back({C.getUnsafeRawPointerDecl()->getDeclaredType()
                                                    ->getCanonicalType(),
                         ParameterConvention::Direct_Unowned});
@@ -2759,7 +2769,8 @@ static SILFunction *getOrCreateKeyPathGetter(SILGenModule &SGM,
   auto resultArg = entry->createFunctionArgument(resultArgTy);
   auto baseArg = entry->createFunctionArgument(baseArgTy);
   SILValue indexPtrArg;
-  if (!indexes.empty()) {
+  // Always take indexes parameter to match callee and caller signature on WebAssembly
+  if (!indexes.empty() || Target.isOSBinFormatWasm()) {
     auto indexArgTy = signature->getParameters()[1].getSILStorageType(
         SGM.M, signature);
     indexPtrArg = entry->createFunctionArgument(indexArgTy);
@@ -2821,6 +2832,7 @@ static SILFunction *getOrCreateKeyPathSetter(SILGenModule &SGM,
     }
   }
 
+  auto Target = SGM.getASTContext().LangOpts.Target;
   auto genericSig =
       genericEnv ? genericEnv->getGenericSignature().getCanonicalSignature()
                  : nullptr;
@@ -2829,6 +2841,14 @@ static SILFunction *getOrCreateKeyPathSetter(SILGenModule &SGM,
     genericEnv = nullptr;
   }
 
+  // Add empty generic type parameter to match function signature on WebAssembly
+  if (!genericSig && Target.isOSBinFormatWasm()) {
+    auto param = GenericTypeParamType::get(0, 0, SGM.getASTContext());
+    auto sig = GenericSignature::get(param, { });
+    genericSig = CanGenericSignature(sig);
+    genericEnv = sig->getGenericEnvironment();
+  }
+
   // Build the signature of the thunk as expected by the keypath runtime.
   auto signature = [&]() {
     CanType loweredBaseTy, loweredPropTy;
@@ -2854,7 +2874,8 @@ static SILFunction *getOrCreateKeyPathSetter(SILGenModule &SGM,
                         ? ParameterConvention::Indirect_Inout
                         : paramConvention});
     // indexes
-    if (!indexes.empty())
+    // Always take indexes parameter to match callee and caller signature on WebAssembly
+    if (!indexes.empty() || C.LangOpts.Target.isOSBinFormatWasm())
       params.push_back({C.getUnsafeRawPointerDecl()->getDeclaredType()
                                                    ->getCanonicalType(),
                         ParameterConvention::Direct_Unowned});
@@ -2910,7 +2931,8 @@ static SILFunction *getOrCreateKeyPathSetter(SILGenModule &SGM,
   auto baseArg = entry->createFunctionArgument(baseArgTy);
   SILValue indexPtrArg;
 
-  if (!indexes.empty()) {
+  // Always take indexes parameter to match callee and caller signature on WebAssembly
+  if (!indexes.empty() || Target.isOSBinFormatWasm()) {
     auto indexArgTy = signature->getParameters()[2].getSILStorageType(
         SGM.M, signature);
     indexPtrArg = entry->createFunctionArgument(indexArgTy);
@@ -2992,6 +3014,7 @@ getOrCreateKeyPathEqualsAndHash(SILGenModule &SGM,
     return;
   }
 
+  auto Target = SGM.getASTContext().LangOpts.Target;
   auto genericSig =
       genericEnv ? genericEnv->getGenericSignature().getCanonicalSignature()
                  : nullptr;
@@ -3001,6 +3024,14 @@ getOrCreateKeyPathEqualsAndHash(SILGenModule &SGM,
     genericEnv = nullptr;
   }
 
+  // Add empty generic type parameter to match function signature on WebAssembly
+  if (!genericSig && Target.isOSBinFormatWasm()) {
+    auto param = GenericTypeParamType::get(0, 0, SGM.getASTContext());
+    auto sig = GenericSignature::get(param, { });
+    genericSig = CanGenericSignature(sig);
+    genericEnv = sig->getGenericEnvironment();
+  }
+
   auto &C = SGM.getASTContext();
   auto unsafeRawPointerTy = C.getUnsafeRawPointerDecl()->getDeclaredType()
                                                        ->getCanonicalType();

diff --git a/stdlib/public/core/KeyPath.swift b/stdlib/public/core/KeyPath.swift
@@ -2543,9 +2543,19 @@ internal protocol KeyPathPatternVisitor {
 
 internal func _resolveRelativeAddress(_ base: UnsafeRawPointer,
                                       _ offset: Int32) -> UnsafeRawPointer {
+  #if arch(wasm32)
+  // FIXME: If offset is 0, it means the pointer is null.
+  // For real relative pointer, it always calculates valid non-null address
+  // because the base address is always valid.
+  // But hacked absolute pointer can be null pointer.
+  // The return type doesn't allow nil, so return given base temporarily.
+  if offset == 0 { return base }
+  return UnsafeRawPointer(bitPattern: Int(offset)).unsafelyUnwrapped
+  #else
   // Sign-extend the offset to pointer width and add with wrap on overflow.
   return UnsafeRawPointer(bitPattern: Int(bitPattern: base) &+ Int(offset))
     .unsafelyUnwrapped
+  #endif
 }
 internal func _resolveRelativeIndirectableAddress(_ base: UnsafeRawPointer,
                                                   _ offset: Int32)
@@ -3189,7 +3199,7 @@ internal struct InstantiateKeyPathBuffer: KeyPathPatternVisitor {
 
     case .pointer:
       // Resolve the sign-extended relative reference.
-      var absoluteID: UnsafeRawPointer? = idValueBase + Int(idValue)
+      var absoluteID: UnsafeRawPointer? = _resolveRelativeAddress(idValueBase, idValue)
 
       // If the pointer ID is unresolved, then it needs work to get to
       // the final value.
@@ -3287,7 +3297,7 @@ internal struct InstantiateKeyPathBuffer: KeyPathPatternVisitor {
       for i in externalArgs.indices {
         let base = externalArgs.baseAddress.unsafelyUnwrapped + i
         let offset = base.pointee
-        let metadataRef = UnsafeRawPointer(base) + Int(offset)
+        let metadataRef = _resolveRelativeAddress(UnsafeRawPointer(base), offset)
         let result = _resolveKeyPathGenericArgReference(
                        metadataRef,
                        genericEnvironment: genericEnvironment,