From 50ef384f884e21f24f002de3d492a09d7060f164 Mon Sep 17 00:00:00 2001 From: Hiroshi Yamauchi Date: Thu, 8 Jun 2023 12:55:23 -0700 Subject: [PATCH] Fix the swift-inspect dump-generic-metadata operation. ReflectionContext::allocationMetadataPointer() was reading the metadata pointer from a wrong offset because of the out-of-sync struct layouts and dump-generic-metadata was not working correctly. This change resync's the layouts and adds a static_assert to verify that the offsets match between GenericMetadataCacheEntry and GenericCacheEntry. --- .../GenericMetadataCacheEntry.h | 40 +++++++++++++++++++ .../RemoteInspection/ReflectionContext.h | 15 ++----- stdlib/public/runtime/Metadata.cpp | 11 +++++ stdlib/public/runtime/MetadataCache.h | 2 + 4 files changed, 57 insertions(+), 11 deletions(-) create mode 100644 include/swift/RemoteInspection/GenericMetadataCacheEntry.h diff --git a/include/swift/RemoteInspection/GenericMetadataCacheEntry.h b/include/swift/RemoteInspection/GenericMetadataCacheEntry.h new file mode 100644 index 0000000000000..3e2f377511ad9 --- /dev/null +++ b/include/swift/RemoteInspection/GenericMetadataCacheEntry.h @@ -0,0 +1,40 @@ +//===--- GenericMetadataCacheEntry.h ----------------------------*- C++ -*-===// +// +// This source file is part of the Swift.org open source project +// +// Copyright (c) 2014 - 2023 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See https://swift.org/LICENSE.txt for license information +// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors +// +//===----------------------------------------------------------------------===// +// +// Declares a struct that mirrors the layout of GenericCacheEntry in +// Metadata.cpp and use a static assert to check that the offset of +// the member Value match between the two. +// +//===----------------------------------------------------------------------===// + +#ifndef SWIFT_REFLECTION_GENERICMETADATACACHEENTRY_H +#define SWIFT_REFLECTION_GENERICMETADATACACHEENTRY_H + +#include + +namespace swift { + +template +struct GenericMetadataCacheEntry { + StoredPointer TrackingInfo; + uint16_t NumKeyParameters; + uint16_t NumWitnessTables; + uint16_t NumPacks; + uint16_t NumShapeClasses; + StoredPointer PackShapeDescriptors; + uint32_t Hash; + StoredPointer Value; +}; + +} // namespace swift + +#endif // SWIFT_REFLECTION_GENERICMETADATACACHEENTRY_H diff --git a/include/swift/RemoteInspection/ReflectionContext.h b/include/swift/RemoteInspection/ReflectionContext.h index bd1492f64dab0..2d057c5133c95 100644 --- a/include/swift/RemoteInspection/ReflectionContext.h +++ b/include/swift/RemoteInspection/ReflectionContext.h @@ -30,6 +30,7 @@ #include "swift/Concurrency/Actor.h" #include "swift/Remote/MemoryReader.h" #include "swift/Remote/MetadataReader.h" +#include "swift/RemoteInspection/GenericMetadataCacheEntry.h" #include "swift/RemoteInspection/Records.h" #include "swift/RemoteInspection/RuntimeInternals.h" #include "swift/RemoteInspection/TypeLowering.h" @@ -1289,22 +1290,14 @@ class ReflectionContext StoredPointer allocationMetadataPointer( MetadataAllocation Allocation) { if (Allocation.Tag == GenericMetadataCacheTag) { - struct GenericMetadataCacheEntry { - StoredPointer LockedStorage; - uint8_t LockedStorageKind; - uint8_t TrackingInfo; - uint16_t NumKeyParameters; - uint16_t NumWitnessTables; - uint32_t Hash; - StoredPointer Value; - }; auto AllocationBytes = getReader().readBytes(RemoteAddress(Allocation.Ptr), Allocation.Size); if (!AllocationBytes) return 0; - auto Entry = reinterpret_cast( - AllocationBytes.get()); + auto Entry = + reinterpret_cast *>( + AllocationBytes.get()); return Entry->Value; } return 0; diff --git a/stdlib/public/runtime/Metadata.cpp b/stdlib/public/runtime/Metadata.cpp index cae6faac16b74..d69a602e3640d 100644 --- a/stdlib/public/runtime/Metadata.cpp +++ b/stdlib/public/runtime/Metadata.cpp @@ -28,6 +28,7 @@ #include "swift/Basic/Range.h" #include "swift/Basic/STLExtras.h" #include "swift/Demangling/Demangler.h" +#include "swift/RemoteInspection/GenericMetadataCacheEntry.h" #include "swift/Runtime/Casting.h" #include "swift/Runtime/EnvironmentVariables.h" #include "swift/Runtime/ExistentialContainer.h" @@ -427,6 +428,16 @@ namespace { }; } // end anonymous namespace +namespace swift { + struct StaticAssertGenericMetadataCacheEntryValueOffset { + static_assert( + offsetof(GenericCacheEntry, Value) == + offsetof(swift::GenericMetadataCacheEntry, + Value), + "The generic metadata cache entry layout mismatch"); + }; +} + namespace { class GenericMetadataCache : public MetadataCache { diff --git a/stdlib/public/runtime/MetadataCache.h b/stdlib/public/runtime/MetadataCache.h index 2d67f5350ee7a..2b6397b3f0726 100644 --- a/stdlib/public/runtime/MetadataCache.h +++ b/stdlib/public/runtime/MetadataCache.h @@ -1583,6 +1583,8 @@ class VariadicMetadataCacheEntryBase : bool matchesKey(const MetadataCacheKey &key) const { return key == getKey(); } + + friend struct StaticAssertGenericMetadataCacheEntryValueOffset; }; template