Merge pull request #64547 from slavapestov/metadata-cache-key-oops

Runtime: Fix MetadataCacheKey::installInto() for variadic generics

Author: slavapestov

stdlib/public/runtime/Metadata.cpp

@@ -155,28 +155,20 @@ Metadata *TargetSingletonMetadataInitialization<InProcess>::allocate(
   return metadata;
 }
 
-/// Copy the generic arguments into place in a newly-allocated metadata.
-static void installGenericArguments(Metadata *metadata,
-                                    const TypeContextDescriptor *description,
-                                    const void *arguments) {
-  const auto &genericContext = *description->getGenericContext();
-  const auto &header = genericContext.getGenericContextHeader();
-
-  auto dst = (reinterpret_cast<const void **>(metadata) +
-              description->getGenericArgumentOffset());
-  memcpy(dst,
-         reinterpret_cast<const void * const *>(arguments),
-         header.NumKeyArguments * sizeof(void *));
+void MetadataCacheKey::installGenericArguments(
+    uint16_t numKeyArguments,
+    uint16_t numPacks,
+    const GenericPackShapeDescriptor *PackShapeDescriptors,
+    const void **dst, const void * const *src) {
+  memcpy(dst, src, numKeyArguments * sizeof(void *));
 
   // If we don't have any pack arguments, there is nothing more to do.
-  auto packShapeHeader = genericContext.getGenericPackShapeHeader();
-  if (packShapeHeader.NumPacks == 0)
+  if (numPacks == 0)
     return;
 
   // Heap-allocate all installed metadata and witness table packs.
-  for (auto pack : genericContext.getGenericPackShapeDescriptors()) {
-    assert(pack.ShapeClass < packShapeHeader.NumShapeClasses);
-
+  for (unsigned i = 0; i < numPacks; ++i) {
+    auto pack = PackShapeDescriptors[i];
     size_t count = reinterpret_cast<size_t>(dst[pack.ShapeClass]);
 
     switch (pack.Kind) {
@@ -195,6 +187,26 @@ static void installGenericArguments(Metadata *metadata,
   }
 }
 
+/// Copy the generic arguments into place in a newly-allocated metadata.
+static void installGenericArguments(Metadata *metadata,
+                                    const TypeContextDescriptor *description,
+                                    const void *arguments) {
+  const auto &genericContext = *description->getGenericContext();
+  const auto &header = genericContext.getGenericContextHeader();
+
+  auto dst = (reinterpret_cast<const void **>(metadata) +
+              description->getGenericArgumentOffset());
+  auto src = reinterpret_cast<const void * const *>(arguments);
+
+  auto packShapeHeader = genericContext.getGenericPackShapeHeader();
+
+  MetadataCacheKey::installGenericArguments(
+      header.NumKeyArguments,
+      packShapeHeader.NumPacks,
+      genericContext.getGenericPackShapeDescriptors().data(),
+      dst, src);
+}
+
 #if SWIFT_OBJC_INTEROP
 static ClassMetadataBounds computeMetadataBoundsForObjCClass(Class cls) {
   cls = swift_getInitializedObjCClass(cls);

stdlib/public/runtime/MetadataCache.h

@@ -521,6 +521,10 @@ class MetadataCacheKey {
   }
 
 public:
+  static void installGenericArguments(uint16_t numKeyArguments, uint16_t numPacks,
+                                      const GenericPackShapeDescriptor *packShapeDescriptors,
+                                      const void **dst, const void * const *src);
+
   /// Compare two conformance descriptors, checking their contents if necessary.
   static bool areConformanceDescriptorsEqual(
       const ProtocolConformanceDescriptor *aDescription,
@@ -548,7 +552,8 @@ class MetadataCacheKey {
     MetadataPackPointer lhs(lhsPtr);
     MetadataPackPointer rhs(rhsPtr);
 
-    assert(lhs.getLifetime() == PackLifetime::OnHeap);
+    // lhs is the user-supplied key, which might be on the stack.
+    // rhs is the stored key in the cache.
     assert(rhs.getLifetime() == PackLifetime::OnHeap);
 
     auto *lhsElt = lhs.getElements();
@@ -568,7 +573,8 @@ class MetadataCacheKey {
     WitnessTablePackPointer lhs(lhsPtr);
     WitnessTablePackPointer rhs(rhsPtr);
 
-    assert(lhs.getLifetime() == PackLifetime::OnHeap);
+    // lhs is the user-supplied key, which might be on the stack.
+    // rhs is the stored key in the cache.
     assert(rhs.getLifetime() == PackLifetime::OnHeap);
 
     auto *lhsElt = lhs.getElements();
@@ -591,7 +597,7 @@ class MetadataCacheKey {
                    const void *const *data, uint32_t hash)
       : Data(data), Layout(layout), Hash(hash) {}
 
-  bool operator==(MetadataCacheKey rhs) const {
+  bool operator==(const MetadataCacheKey &rhs) const {
     // Compare the hashes.
     if (hash() != rhs.hash()) return false;
 
@@ -682,8 +688,11 @@ class MetadataCacheKey {
   unsigned size() const { return Layout.sizeInWords(); }
 
   void installInto(const void **buffer) const {
-    // FIXME: variadic-parameter-packs
-    memcpy(buffer, Data, size() * sizeof(const void *));
+    MetadataCacheKey::installGenericArguments(
+        Layout.sizeInWords(),
+        Layout.NumPacks,
+        Layout.PackShapeDescriptors,
+        buffer, Data);
   }
 
 private:

test/Interpreter/variadic_generic_types.swift

@@ -26,6 +26,22 @@ public struct Outer<each U> {
   public struct InnerSameShape<each V> where (repeat (each U, each V)): Any {}
 }
 
+func makeMetatype<each T>(_: repeat (each T).Type) -> Any.Type {
+  return Outer<repeat each T>.self
+}
+
+func blackHole<T>(_: T) {}
+
+types.test("OuterRepeated") {
+  // Instantiate a type containing type parameters to avoid caching
+  blackHole(makeMetatype())
+  blackHole(makeMetatype())
+  blackHole(makeMetatype(Int.self))
+  blackHole(makeMetatype(Int.self))
+  blackHole(makeMetatype(String.self, Substring.self))
+  blackHole(makeMetatype(String.self, Substring.self))
+}
+
 types.test("Outer") {
   expectEqual("main.Outer<Pack{}>", _typeName(Outer< >.self))
   expectEqual("main.Outer<Pack{Swift.Int}>", _typeName(Outer<Int>.self))