diff --git a/include/swift/RemoteInspection/BitMask.h b/include/swift/RemoteInspection/BitMask.h index f64f2126996a1..866e0e5f24a7e 100644 --- a/include/swift/RemoteInspection/BitMask.h +++ b/include/swift/RemoteInspection/BitMask.h @@ -74,7 +74,16 @@ class BitMask { BitMask(unsigned sizeInBytes, uint64_t sourceMask): size(sizeInBytes) { mask = (uint8_t *)calloc(1, sizeInBytes); - memcpy(mask, &sourceMask, sizeInBytes); + if (!mask) { + assert(false && "Failed to allocate Bitmask"); + size = 0; + return; + } + size_t toCopy = sizeInBytes; + if (toCopy > sizeof(sourceMask)) { + toCopy = sizeof(sourceMask); + } + memcpy(mask, &sourceMask, toCopy); } // Construct a bitmask of the appropriate number of bytes diff --git a/stdlib/public/RemoteInspection/TypeLowering.cpp b/stdlib/public/RemoteInspection/TypeLowering.cpp index 8a2d36dfeedda..2bdb4c85b85b2 100644 --- a/stdlib/public/RemoteInspection/TypeLowering.cpp +++ b/stdlib/public/RemoteInspection/TypeLowering.cpp @@ -251,7 +251,7 @@ BuiltinTypeInfo::BuiltinTypeInfo(unsigned Size, unsigned Alignment, // Builtin.Int is mangled as 'Bi' N '_' // Returns 0 if this isn't an Int -static unsigned isIntType(std::string name) { +static unsigned intTypeBitSize(std::string name) { llvm::StringRef nameRef(name); if (nameRef.starts_with("Bi") && nameRef.ends_with("_")) { llvm::StringRef naturalRef = nameRef.drop_front(2).drop_back(); @@ -272,8 +272,22 @@ bool BuiltinTypeInfo::readExtraInhabitantIndex( *extraInhabitantIndex = -1; return true; } - unsigned intSize = isIntType(Name); - if (intSize > 0 && intSize < 64 && getSize() <= 8 && intSize < getSize() * 8) { + // If it has extra inhabitants, it could be an integer type with extra + // inhabitants (such as a bool) or a pointer. + unsigned intSize = intTypeBitSize(Name); + if (intSize > 0) { + // This is an integer type + + // If extra inhabitants are impossible, return early... + // (assert in debug builds) + assert(intSize < getSize() * 8 + && "Standard-sized int cannot have extra inhabitants"); + if (intSize > 64 || getSize() > 8 || intSize >= getSize() * 8) { + *extraInhabitantIndex = -1; + return true; + } + + // Compute range of extra inhabitants uint64_t maxValidValue = (((uint64_t)1) << intSize) - 1; uint64_t maxAvailableValue = (((uint64_t)1) << (getSize() * 8)) - 1; uint64_t computedExtraInhabitants = maxAvailableValue - maxValidValue; @@ -281,14 +295,13 @@ bool BuiltinTypeInfo::readExtraInhabitantIndex( computedExtraInhabitants = ValueWitnessFlags::MaxNumExtraInhabitants; } assert(getNumExtraInhabitants() == computedExtraInhabitants && - "Unexpected number of extra inhabitants in an odd-sized integer"); + "Unexpected number of extra inhabitants in an odd-sized integer"); + // Example: maxValidValue is 1 for a 1-bit bool, so any larger value + // is an extra inhabitant. uint64_t rawValue; if (!reader.readInteger(address, getSize(), &rawValue)) return false; - - // Example: maxValidValue is 1 for a 1-bit bool, so any larger value - // is an extra inhabitant. if (maxValidValue < rawValue) { *extraInhabitantIndex = rawValue - maxValidValue - 1; } else { @@ -307,7 +320,7 @@ bool BuiltinTypeInfo::readExtraInhabitantIndex( } BitMask BuiltinTypeInfo::getSpareBits(TypeConverter &TC, bool &hasAddrOnly) const { - unsigned intSize = isIntType(Name); + unsigned intSize = intTypeBitSize(Name); if (intSize > 0) { // Odd-sized integers export spare bits // In particular: bool fields are Int1 and export 7 spare bits @@ -315,6 +328,7 @@ BitMask BuiltinTypeInfo::getSpareBits(TypeConverter &TC, bool &hasAddrOnly) cons mask.keepOnlyMostSignificantBits(getSize() * 8 - intSize); return mask; } else if ( + Name == "ypXp" || // Any.Type Name == "yyXf" // 'yyXf' = @thin () -> Void function ) { // Builtin types that expose pointer spare bits @@ -578,7 +592,9 @@ class NoPayloadEnumTypeInfo: public EnumTypeInfo { } BitMask getSpareBits(TypeConverter &TC, bool &hasAddrOnly) const override { - return BitMask::zeroMask(getSize()); + auto mask = BitMask(getSize(), maskForCount(getNumCases())); + mask.complement(); + return mask; } bool projectEnumValue(remote::MemoryReader &reader, @@ -648,7 +664,17 @@ class SinglePayloadEnumTypeInfo: public EnumTypeInfo { } BitMask getSpareBits(TypeConverter &TC, bool &hasAddrOnly) const override { - return BitMask::zeroMask(getSize()); + FieldInfo PayloadCase = getCases()[0]; + size_t payloadSize = PayloadCase.TI.getSize(); + if (getSize() <= payloadSize) { + return BitMask::zeroMask(getSize()); + } + size_t tagSize = getSize() - payloadSize; + auto mask = BitMask::oneMask(getSize()); + mask.keepOnlyMostSignificantBits(tagSize * 8); // Clear payload bits + auto tagMaskUsedBits = BitMask(getSize(), maskForCount(getNumCases())); + mask.andNotMask(tagMaskUsedBits, payloadSize); // Clear used tag bits + return mask; } // Think of a single-payload enum as being encoded in "pages". @@ -1961,6 +1987,15 @@ class EnumTypeInfoBuilder { default: Kind = EnumKind::MultiPayloadEnum; break; } + // Sanity: Ignore any enum that claims to have a size more than 1MiB + // This avoids allocating lots of memory for spare bit mask calculations + // when clients try to interpret random chunks of memory as type descriptions. + if (Size > (1024ULL * 1024)) { + unsigned Stride = ((Size + Alignment - 1) & ~(Alignment - 1)); + return TC.makeTypeInfo( + Size, Alignment, Stride, NumExtraInhabitants, BitwiseTakable, Kind, Cases); + } + if (Cases.size() == 1) { if (EffectivePayloadCases == 0) { // Zero-sized enum with only one empty case @@ -2046,11 +2081,10 @@ class EnumTypeInfoBuilder { // // Do we have a fixed layout? - // TODO: Test whether a missing FixedDescriptor is actually relevant. auto FixedDescriptor = TC.getBuilder().getBuiltinTypeDescriptor(TR); if (!FixedDescriptor || GenericPayloadCases > 0) { // This is a "dynamic multi-payload enum". For example, - // this occurs with: + // this occurs with generics such as: // ``` // class ClassWithEnum { // enum E { @@ -2060,6 +2094,12 @@ class EnumTypeInfoBuilder { // var e: E? // } // ``` + // and when we have a resilient inner enum, such as: + // ``` + // enum E2 { + // case y(E1_resilient) + // case z(Int) + // } auto tagCounts = getEnumTagCounts(Size, EffectiveNoPayloadCases, EffectivePayloadCases); Size += tagCounts.numTagBytes; @@ -2091,7 +2131,6 @@ class EnumTypeInfoBuilder { unsigned Stride = ((Size + Alignment - 1) & ~(Alignment - 1)); if (Stride == 0) Stride = 1; - auto PayloadSize = EnumTypeInfo::getPayloadSizeForCases(Cases); // Compute the spare bit mask and determine if we have any address-only fields auto localSpareBitMask = BitMask::oneMask(Size); @@ -2103,44 +2142,11 @@ class EnumTypeInfoBuilder { } } - // See if we have MPE bit mask information from the compiler... - // TODO: drop this? - - // Uncomment the following line to dump the MPE section every time we come through here... - //TC.getBuilder().dumpMultiPayloadEnumSection(std::cerr); // DEBUG helper - - auto MPEDescriptor = TC.getBuilder().getMultiPayloadEnumDescriptor(TR); - if (MPEDescriptor && MPEDescriptor->usesPayloadSpareBits()) { - // We found compiler-provided spare bit data... - auto PayloadSpareBitMaskByteCount = MPEDescriptor->getPayloadSpareBitMaskByteCount(); - auto PayloadSpareBitMaskByteOffset = MPEDescriptor->getPayloadSpareBitMaskByteOffset(); - auto SpareBitMask = MPEDescriptor->getPayloadSpareBits(); - BitMask compilerSpareBitMask(PayloadSize, SpareBitMask, - PayloadSpareBitMaskByteCount, PayloadSpareBitMaskByteOffset); - - if (compilerSpareBitMask.isZero() || hasAddrOnly) { - // If there are no spare bits, use the "simple" tag-only implementation. - return TC.makeTypeInfo( - Size, Alignment, Stride, NumExtraInhabitants, - BitwiseTakable, Cases, EffectivePayloadCases); - } - -#if 0 // TODO: This should be !defined(NDEBUG) - // Verify that compiler provided and local spare bit info agree... - // TODO: If we could make this actually work, then we wouldn't need the - // bulky compiler-provided info, would we? - assert(localSpareBitMask == compilerSpareBitMask); -#endif - - // Use compiler-provided spare bit information - return TC.makeTypeInfo( - Size, Alignment, Stride, NumExtraInhabitants, - BitwiseTakable, Cases, compilerSpareBitMask, - EffectivePayloadCases); - } - if (localSpareBitMask.isZero() || hasAddrOnly) { - // Simple case that does not use spare bits + // Simple tag-only layout does not use spare bits. + // Either: + // * There are no spare bits, or + // * We can't copy it to strip spare bits. return TC.makeTypeInfo( Size, Alignment, Stride, NumExtraInhabitants, BitwiseTakable, Cases, EffectivePayloadCases); diff --git a/validation-test/Reflection/Inputs/reflect_Enum_values_resilient_enums.swift b/validation-test/Reflection/Inputs/reflect_Enum_values_resilient_enums.swift new file mode 100644 index 0000000000000..b41e6fa45a864 --- /dev/null +++ b/validation-test/Reflection/Inputs/reflect_Enum_values_resilient_enums.swift @@ -0,0 +1,10 @@ + +public enum E1_resilient { +case a +case b +} + +public enum E2_resilient { +case c(E1_resilient) +case d(E1_resilient) +} diff --git a/validation-test/Reflection/reflect_Enum_values_resilient.swift b/validation-test/Reflection/reflect_Enum_values_resilient.swift new file mode 100644 index 0000000000000..814339169be2d --- /dev/null +++ b/validation-test/Reflection/reflect_Enum_values_resilient.swift @@ -0,0 +1,75 @@ +// RUN: %empty-directory(%t) + +// RUN: %target-build-swift-dylib(%t/%target-library-name(resilient_enums)) -enable-library-evolution %S/Inputs/reflect_Enum_values_resilient_enums.swift -emit-module -emit-module-path %t/resilient_enums.swiftmodule -module-name resilient_enums +// RUN: %target-codesign %t/%target-library-name(resilient_enums) + +// RUN: %target-build-swift -lswiftSwiftReflectionTest %s -L %t -I %t -lresilient_enums -o %t/reflect_Enum_values_resilient %target-rpath(%t) +// RUN: %target-codesign %t/reflect_Enum_values_resilient + +// RUN: %target-run %target-swift-reflection-test %t/reflect_Enum_values_resilient | tee /dev/stderr | %FileCheck %s --dump-input=fail + +// REQUIRES: executable_test +// REQUIRES: objc_interop, OS=macosx +// UNSUPPORTED: use_os_stdlib + +import SwiftReflectionTest + +import resilient_enums + +// Non-resilient enum wrapping a resilient enum +// This doesn't use spare bits of the inner enum +enum E2 { +case y(E1_resilient) +case z(E1_resilient) +} + +// Contrast: +// E2_resilient is a resilient enum wrapping a resilient enum +// This does use spare bits of the inner enum + + +// CHECK: Reflecting an enum value. +// CHECK-NEXT: Type reference: +// CHECK-NEXT: (enum reflect_Enum_values_resilient.E2) +// CHECK-NEXT: Value: .y(.a) + +reflect(enumValue: E2.y(.a)) + +// CHECK: Reflecting an enum value. +// CHECK-NEXT: Type reference: +// CHECK-NEXT: (enum reflect_Enum_values_resilient.E2) +// CHECK-NEXT: Value: .z(.b) + +reflect(enumValue: E2.z(.b)) + +// CHECK: Reflecting an enum value. +// CHECK-NEXT: Type reference: +// CHECK-NEXT: (enum resilient_enums.E1_resilient) +// CHECK-NEXT: Value: .a + +reflect(enumValue: E1_resilient.a) + +// CHECK: Reflecting an enum value. +// CHECK-NEXT: Type reference: +// CHECK-NEXT: (enum resilient_enums.E1_resilient) +// CHECK-NEXT: Value: .b + +reflect(enumValue: E1_resilient.b) + +// CHECK: Reflecting an enum value. +// CHECK-NEXT: Type reference: +// CHECK-NEXT: (enum resilient_enums.E2_resilient) +// CHECK-NEXT: Value: .c(.a) + +reflect(enumValue: E2_resilient.c(.a)) + +// CHECK: Reflecting an enum value. +// CHECK-NEXT: Type reference: +// CHECK-NEXT: (enum resilient_enums.E2_resilient) +// CHECK-NEXT: Value: .d(.b) + +reflect(enumValue: E2_resilient.d(.b)) + +doneReflecting() + +// CHECK: Done.