From 1ac8e2c291a605a46b56ac83a8a294f02302b98c Mon Sep 17 00:00:00 2001 From: Alejandro Alonso Date: Thu, 17 Jul 2025 16:09:37 -0700 Subject: [PATCH 01/14] Handle arguments with alignment larger than a word in KeyPath --- stdlib/public/core/KeyPath.swift | 205 +++++++++++++++++++++---------- 1 file changed, 140 insertions(+), 65 deletions(-) diff --git a/stdlib/public/core/KeyPath.swift b/stdlib/public/core/KeyPath.swift index cda7e2cc4db5c..be54d6b4d3d21 100644 --- a/stdlib/public/core/KeyPath.swift +++ b/stdlib/public/core/KeyPath.swift @@ -1119,6 +1119,44 @@ internal enum KeyPathComputedIDResolution { case functionCall } +internal struct ComputedArgumentSize { + let value: UInt + + static var sizeMask: UInt { +#if _pointerBitWidth(_64) + 0x7FFF_FFFF_FFFF_FFFF +#elseif _pointerBitWidth(_32) + 0x3FFF_FFFF +#else +#error("Unsupported platform") +#endif + } + + static var paddingShift: UInt { +#if _pointerBitWidth(_64) + 63 +#elseif _pointerBitWidth(_32) + 30 +#else +#error("Unsupported platform") +#endif + } + + // The total size of the argument buffer is in the bottom 30/63 bits. + var size: Int { + Int(truncatingIfNeeded: value & Self.sizeMask) + } + + // The number of padding bytes required so that the argument is properly + // aligned in the keypath buffer. This is in the top 2 bits for 32 bit + // platforms (because they need to represent 8 and 16 overaligned types) and + // top 1 bit for 64 bit platforms (because they only need to represent 16 + // alignment). + var padding: Int { + Int(truncatingIfNeeded: value &>> Self.paddingShift) &* MemoryLayout.size + } +} + @_unavailableInEmbedded @safe internal struct RawKeyPathComponent { @@ -1534,7 +1572,7 @@ internal struct RawKeyPathComponent { // two words for argument header: size, witnesses total &+= ptrSize &* 2 // size of argument area - total &+= _computedArgumentSize + total &+= _computedArgumentSize.size if header.isComputedInstantiatedFromExternalWithArguments { total &+= Header.externalWithArgumentsExtraSize } @@ -1592,8 +1630,8 @@ internal struct RawKeyPathComponent { (header.isComputedSettable ? 3 : 2) } - internal var _computedArgumentSize: Int { - return unsafe _computedArgumentHeaderPointer.load(as: Int.self) + internal var _computedArgumentSize: ComputedArgumentSize { + return unsafe _computedArgumentHeaderPointer.load(as: ComputedArgumentSize.self) } internal var _computedArgumentWitnesses: ComputedArgumentWitnessesPtr { @@ -1611,6 +1649,10 @@ internal struct RawKeyPathComponent { if header.isComputedInstantiatedFromExternalWithArguments { unsafe base += Header.externalWithArgumentsExtraSize } + + // If the argument needed padding to be properly aligned, skip that. + unsafe base += _computedArgumentSize.padding + return unsafe base } internal var _computedMutableArguments: UnsafeMutableRawPointer { @@ -1648,7 +1690,7 @@ internal struct RawKeyPathComponent { if header.hasComputedArguments { unsafe argument = unsafe KeyPathComponent.ArgumentRef( data: UnsafeRawBufferPointer(start: _computedArguments, - count: _computedArgumentSize), + count: _computedArgumentSize.size &- _computedArgumentSize.padding), witnesses: _computedArgumentWitnesses, witnessSizeAdjustment: _computedArgumentWitnessSizeAdjustment) } else { @@ -1688,7 +1730,7 @@ internal struct RawKeyPathComponent { if header.hasComputedArguments, let destructor = unsafe _computedArgumentWitnesses.destroy { unsafe destructor(_computedMutableArguments, - _computedArgumentSize &- _computedArgumentWitnessSizeAdjustment) + _computedArgumentSize.size &- _computedArgumentWitnessSizeAdjustment) } case .external: _internalInvariantFailure("should have been instantiated away") @@ -1741,12 +1783,14 @@ internal struct RawKeyPathComponent { componentSize += MemoryLayout.size } + // FIXME: Need to handle if buffer is already aligned when copying + // arguments which would impact the argument size word. if header.hasComputedArguments { let arguments = unsafe _computedArguments let argumentSize = _computedArgumentSize unsafe buffer.storeBytes(of: argumentSize, toByteOffset: componentSize, - as: Int.self) + as: ComputedArgumentSize.self) componentSize += MemoryLayout.size unsafe buffer.storeBytes(of: _computedArgumentWitnesses, toByteOffset: componentSize, @@ -1761,7 +1805,7 @@ internal struct RawKeyPathComponent { as: Int.self) componentSize += MemoryLayout.size } - let adjustedSize = argumentSize - _computedArgumentWitnessSizeAdjustment + let adjustedSize = argumentSize.size - _computedArgumentWitnessSizeAdjustment let argumentDest = unsafe buffer.baseAddress.unsafelyUnwrapped + componentSize unsafe _computedArgumentWitnesses.copy( @@ -1776,7 +1820,7 @@ internal struct RawKeyPathComponent { size: UInt(_computedArgumentWitnessSizeAdjustment)) } - componentSize += argumentSize + componentSize += argumentSize.size } case .external: @@ -3527,46 +3571,44 @@ internal struct GetKeyPathClassAndInstanceSizeFromPattern if settable { unsafe size += MemoryLayout.size } - - // ...and the arguments, if any. - let argumentHeaderSize = MemoryLayout.size * 2 - switch unsafe (arguments, externalArgs) { - case (nil, nil): - break - case (let arguments?, nil): - unsafe size += argumentHeaderSize - // If we have arguments, calculate how much space they need by invoking - // the layout function. - let (addedSize, addedAlignmentMask) = unsafe arguments.getLayout(patternArgs) - // TODO: Handle over-aligned values - _internalInvariant(addedAlignmentMask < MemoryLayout.alignment, - "overaligned computed property element not supported") - unsafe size += addedSize - - case (let arguments?, let externalArgs?): - // If we're referencing an external declaration, and it takes captured - // arguments, then we have to build a bit of a chimera. The canonical - // identity and accessors come from the descriptor, but the argument - // handling is still as described in the local candidate. - unsafe size += argumentHeaderSize - let (addedSize, addedAlignmentMask) = unsafe arguments.getLayout(patternArgs) - // TODO: Handle over-aligned values - _internalInvariant(addedAlignmentMask < MemoryLayout.alignment, - "overaligned computed property element not supported") - unsafe size += addedSize + + // Handle local arguments. + if let arguments = arguments { + // Argument size and witnesses ptr. + unsafe size &+= MemoryLayout.size &* 2 + + let (typeSize, typeAlignMask) = unsafe arguments.getLayout(patternArgs) + + // We are known to be pointer aligned at this point in the KeyPath buffer. + // However, for types who have an alignment large than pointers, we need + // to determine if the current position is suitable for the argument, or + // if we need to add padding bytes to align ourselves. + let misalign = unsafe size & typeAlignMask + + if misalign != 0 { + let typeAlignment = typeAlignMask &+ 1 + let padding = Swift.max(0, typeAlignment &- MemoryLayout.alignment) + unsafe size &+= padding + } + + unsafe size &+= typeSize + unsafe roundUpToPointerAlignment() + } + + // Handle external arguments. + if let externalArgs = externalArgs { + // Argument size and witnesses ptr if we didn't have local arguments. + if arguments == nil { + unsafe size &+= MemoryLayout.size &* 2 + } + // We also need to store the size of the local arguments so we can // find the external component arguments. - unsafe roundUpToPointerAlignment() - unsafe size += RawKeyPathComponent.Header.externalWithArgumentsExtraSize - unsafe size += MemoryLayout.size * externalArgs.count + if arguments != nil { + unsafe size &+= RawKeyPathComponent.Header.externalWithArgumentsExtraSize + } - case (nil, let externalArgs?): - // If we're instantiating an external property with a local - // candidate that has no arguments, then things are a little - // easier. We only need to instantiate the generic - // arguments for the external component's accessors. - unsafe size += argumentHeaderSize - unsafe size += MemoryLayout.size * externalArgs.count + unsafe size &+= MemoryLayout.size &* externalArgs.count } } @@ -3599,8 +3641,7 @@ internal struct GetKeyPathClassAndInstanceSizeFromPattern } mutating func finish() { - unsafe sizeWithMaxSize = unsafe size - unsafe sizeWithMaxSize = unsafe MemoryLayout._roundingUpToAlignment(sizeWithMaxSize) + unsafe sizeWithMaxSize = unsafe MemoryLayout._roundingUpToAlignment(size) unsafe sizeWithMaxSize &+= MemoryLayout.size } } @@ -3891,22 +3932,13 @@ internal struct InstantiateKeyPathBuffer: KeyPathPatternVisitor { } if let arguments = unsafe arguments { - // Instantiate the arguments. - let (baseSize, alignmentMask) = unsafe arguments.getLayout(patternArgs) - _internalInvariant(alignmentMask < MemoryLayout.alignment, - "overaligned computed arguments not implemented yet") - - // The real buffer stride will be rounded up to alignment. - var totalSize = (baseSize + alignmentMask) & ~alignmentMask - - // If an external property descriptor also has arguments, they'll be - // added to the end with pointer alignment. - if let externalArgs = unsafe externalArgs { - totalSize = MemoryLayout._roundingUpToAlignment(totalSize) - totalSize += MemoryLayout.size * externalArgs.count - } - + // Record a placeholder for the total size of the arguments. We need to + // check after pushing preliminary data if the resulting argument will + // require padding bytes to be properly aligned. + let totalSizeAddress = unsafe destData.baseAddress._unsafelyUnwrappedUnchecked + var totalSize: UInt = 0 unsafe pushDest(totalSize) + unsafe pushDest(arguments.witnesses) // A nonnull destructor in the witnesses file indicates the instantiated @@ -3922,15 +3954,58 @@ internal struct InstantiateKeyPathBuffer: KeyPathPatternVisitor { unsafe pushDest(externalArgs.count * MemoryLayout.size) } + let (typeSize, typeAlignMask) = unsafe arguments.getLayout(patternArgs) + let typeAlignment = typeAlignMask &+ 1 + let padding = Swift.max(0, typeAlignment &- MemoryLayout.alignment) + + totalSize = UInt(truncatingIfNeeded: typeSize) + + // We are known to be pointer aligned at this point in the KeyPath buffer. + // However, for types who have an alignment large than pointers, we need + // to determine if the current position is suitable for the argument, or + // if we need to add padding bytes to align ourselves. + let argumentAddress = unsafe destData.baseAddress._unsafelyUnwrappedUnchecked + let misalign = Int(bitPattern: argumentAddress) & typeAlignMask + + if misalign != 0 { + totalSize &+= UInt(truncatingIfNeeded: padding) + + // Go ahead and append the padding. + for _ in 0 ..< padding { + unsafe pushDest(UInt8(0)) + } + } + + // If an external property descriptor also has arguments, they'll be + // added to the end with pointer alignment. + if let externalArgs = unsafe externalArgs { + totalSize = MemoryLayout._roundingUpToAlignment(totalSize) + totalSize += UInt(truncatingIfNeeded: MemoryLayout.size * externalArgs.count) + } + + // The argument total size contains the padding bytes required for the + // argument in the top 4 bits, so ensure the size of the argument buffer + // is small enough to account for that. + _precondition( + totalSize <= ComputedArgumentSize.sizeMask, + "keypath arguments are too large" + ) + + totalSize |= UInt(truncatingIfNeeded: padding / MemoryLayout.size) &<< ComputedArgumentSize.paddingShift + + // Ok, we've fully calculated totalSize, go ahead and update the + // placeholder. + unsafe totalSizeAddress.storeBytes(of: totalSize, as: UInt.self) + // Initialize the local candidate arguments here. - unsafe _internalInvariant(Int(bitPattern: destData.baseAddress) & alignmentMask == 0, + unsafe _internalInvariant(Int(bitPattern: destData.baseAddress) & typeAlignMask == 0, "argument destination not aligned") unsafe arguments.initializer(patternArgs, destData.baseAddress._unsafelyUnwrappedUnchecked) unsafe destData = unsafe UnsafeMutableRawBufferPointer( - start: destData.baseAddress._unsafelyUnwrappedUnchecked + baseSize, - count: destData.count - baseSize) + start: destData.baseAddress._unsafelyUnwrappedUnchecked + typeSize, + count: destData.count - typeSize) } if let externalArgs = unsafe externalArgs { From f95e9e4ee4238bb2794f714c7b79c80403bca0bd Mon Sep 17 00:00:00 2001 From: Alejandro Alonso Date: Tue, 19 Aug 2025 14:22:40 -0700 Subject: [PATCH 02/14] Correctly handle alignment and appending keypaths Add test for overaligned keypath arguments --- stdlib/public/core/KeyPath.swift | 261 +++++++++++++++++----- stdlib/public/core/ReflectionMirror.swift | 6 +- test/stdlib/KeyPath.swift | 112 ++++++++++ 3 files changed, 324 insertions(+), 55 deletions(-) diff --git a/stdlib/public/core/KeyPath.swift b/stdlib/public/core/KeyPath.swift index be54d6b4d3d21..03571f95576f1 100644 --- a/stdlib/public/core/KeyPath.swift +++ b/stdlib/public/core/KeyPath.swift @@ -1126,7 +1126,17 @@ internal struct ComputedArgumentSize { #if _pointerBitWidth(_64) 0x7FFF_FFFF_FFFF_FFFF #elseif _pointerBitWidth(_32) - 0x3FFF_FFFF + 0x1FFF_FFFF +#else +#error("Unsupported platform") +#endif + } + + static var paddingMask: UInt { +#if _pointerBitWidth(_64) + 0x8000_0000_0000_0000 +#elseif _pointerBitWidth(_32) + 0x6000_0000 #else #error("Unsupported platform") #endif @@ -1136,14 +1146,34 @@ internal struct ComputedArgumentSize { #if _pointerBitWidth(_64) 63 #elseif _pointerBitWidth(_32) - 30 + 29 #else #error("Unsupported platform") #endif } - // The total size of the argument buffer is in the bottom 30/63 bits. - var size: Int { + static var alignmentShift: UInt { +#if _pointerBitWidth(_64) + // On 64 bit platforms, we don't need to record alignment in this structure. + 0 +#elseif _pointerBitWidth(_32) + 31 +#else +#error("Unsupported platform") +#endif + } + + init(_ argSize: Int) { + value = UInt(truncatingIfNeeded: argSize) + } + + // The size of just the arguments only, ignoring padding. + var argumentSize: Int { + totalSize &- padding + } + + // The total size of the argument buffer is in the bottom 29/63 bits. + var totalSize: Int { Int(truncatingIfNeeded: value & Self.sizeMask) } @@ -1153,7 +1183,56 @@ internal struct ComputedArgumentSize { // top 1 bit for 64 bit platforms (because they only need to represent 16 // alignment). var padding: Int { - Int(truncatingIfNeeded: value &>> Self.paddingShift) &* MemoryLayout.size + get { + let mask = value & Self.paddingMask + let shift = mask &>> Self.paddingShift + return Int(truncatingIfNeeded: shift) &* MemoryLayout.size + } + + set { + let reduced = newValue / MemoryLayout.size + let shift = reduced &<< Self.paddingShift + value &= ~Self.paddingMask + value |= shift + } + } + + // If the arguments have an alignment greater than the platform's pointer + // alignment, then this structure records the argument's alignment. On 64 bit + // platforms, we don't actually record this in the bit pattern anywhere, but + // on 32 bit platforms we use the top bit to indicate either 8 or 16 byte + // alignment. This returns 0 if no padding was recorded. + var alignment: Int { + get { + // If we didn't record a padding value, then this argument's alignment was + // either equal to or less than pointer value alignment. + guard padding != 0 else { + return 0 + } + + #if _pointerBitWidth(_64) + // On 64 bit, the only higher alignment a type could have is 16 byte. + return 16 + #elseif _pointerBitWidth(_32) + // 0 = 8 byte + // 1 = 16 byte + let shift = value &>> Self.alignmentShift + return shift == 1 ? 16 : 8 + #else + #error("Unsupported platform") + #endif + } + + // The setter should only be called when there is or will be padding. + set { +#if _pointerBitWidth(_64) + // Nop on 64 bit. + return +#elseif _pointerBitWidth(_32) + let reduced = newValue == 16 ? 1 : 0 + let shift = reduced &<< Self.alignmentShift + value |= shift + } } } @@ -1572,7 +1651,7 @@ internal struct RawKeyPathComponent { // two words for argument header: size, witnesses total &+= ptrSize &* 2 // size of argument area - total &+= _computedArgumentSize.size + total &+= _computedArgumentSize.totalSize if header.isComputedInstantiatedFromExternalWithArguments { total &+= Header.externalWithArgumentsExtraSize } @@ -1690,7 +1769,7 @@ internal struct RawKeyPathComponent { if header.hasComputedArguments { unsafe argument = unsafe KeyPathComponent.ArgumentRef( data: UnsafeRawBufferPointer(start: _computedArguments, - count: _computedArgumentSize.size &- _computedArgumentSize.padding), + count: _computedArgumentSize.argumentSize), witnesses: _computedArgumentWitnesses, witnessSizeAdjustment: _computedArgumentWitnessSizeAdjustment) } else { @@ -1730,15 +1809,18 @@ internal struct RawKeyPathComponent { if header.hasComputedArguments, let destructor = unsafe _computedArgumentWitnesses.destroy { unsafe destructor(_computedMutableArguments, - _computedArgumentSize.size &- _computedArgumentWitnessSizeAdjustment) + _computedArgumentSize.argumentSize &- _computedArgumentWitnessSizeAdjustment) } case .external: _internalInvariantFailure("should have been instantiated away") } } - internal func clone(into buffer: inout UnsafeMutableRawBufferPointer, - endOfReferencePrefix: Bool) { + internal func clone( + into buffer: inout UnsafeMutableRawBufferPointer, + endOfReferencePrefix: Bool, + adjustForAlignment: Bool + ) { var newHeader = header newHeader.endOfReferencePrefix = endOfReferencePrefix @@ -1783,15 +1865,27 @@ internal struct RawKeyPathComponent { componentSize += MemoryLayout.size } - // FIXME: Need to handle if buffer is already aligned when copying - // arguments which would impact the argument size word. if header.hasComputedArguments { let arguments = unsafe _computedArguments let argumentSize = _computedArgumentSize - unsafe buffer.storeBytes(of: argumentSize, - toByteOffset: componentSize, - as: ComputedArgumentSize.self) + // Remember the argument size location, we may need to adjust the value + // for alignment. + let argumentSizePtr = unsafe buffer.baseAddress._unsafelyUnwrappedUnchecked + componentSize + + // If we don't need to adjust for alignment, then the current argument + // size is correct. Or, if the arguments themselves don't require any + // padding whatsoever. If alignment is 0, then the arguments had equal + // to or less than alignment to the platform's word alignment. + if !adjustForAlignment || argumentSize.alignment == 0 { + unsafe buffer.storeBytes( + of: argumentSize, + toByteOffset: componentSize, + as: ComputedArgumentSize.self + ) + } + componentSize += MemoryLayout.size + unsafe buffer.storeBytes(of: _computedArgumentWitnesses, toByteOffset: componentSize, as: ComputedArgumentWitnessesPtr.self) @@ -1805,13 +1899,49 @@ internal struct RawKeyPathComponent { as: Int.self) componentSize += MemoryLayout.size } - let adjustedSize = argumentSize.size - _computedArgumentWitnessSizeAdjustment - let argumentDest = + + let adjustedSize = argumentSize.argumentSize &- + _computedArgumentWitnessSizeAdjustment + var argumentDest = unsafe buffer.baseAddress.unsafelyUnwrapped + componentSize + + // We've been asked to adjust for alignment and the original argument + // was overaligned. Check for misalignment at our current destination. + if adjustForAlignment, argumentSize.alignment != 0 { + let argumentAlignmentMask = argumentSize.alignment &- 1 + let misaligned = Int(bitPattern: argumentDest) & argumentAlignmentMask + + // Ok, this is no longer misaligned. We don't need to record any + // padding. + if misaligned == 0 { + unsafe argumentSizePtr.storeBytes( + of: argumentSize.argumentSize, + as: Int.self + ) + } else { + // Otherwise, this is still misaligned and we need to calculate + // how much padding we need. + var newArgumentSize = ComputedArgumentSize(argumentSize.argumentSize) + + newArgumentSize.padding = argumentSize.alignment &- misaligned + newArgumentSize.alignment = argumentSize.alignment + + unsafe argumentSizePtr.storeBytes( + of: newArgumentSize, + as: ComputedArgumentSize.self + ) + + // Push the buffer past the padding. + unsafe argumentDest += padding + componentSize += padding + } + } + unsafe _computedArgumentWitnesses.copy( arguments, argumentDest, adjustedSize) + if header.isComputedInstantiatedFromExternalWithArguments { // The extra information for external property descriptor arguments // can always be memcpy'd. @@ -1820,7 +1950,7 @@ internal struct RawKeyPathComponent { size: UInt(_computedArgumentWitnessSizeAdjustment)) } - componentSize += argumentSize.size + componentSize += argumentSize.argumentSize } case .external: @@ -2766,7 +2896,12 @@ internal func _appendingKeyPaths< unsafe component.clone( into: &destBuilder.buffer, - endOfReferencePrefix: endOfReferencePrefix) + endOfReferencePrefix: endOfReferencePrefix, + + // Root components don't need to adjust for alignment because the + // components should appear at the same offset as the root keypath. + adjustForAlignment: false + ) // Insert our endpoint type between the root and leaf components. if let type = type { unsafe destBuilder.push(type) @@ -2784,7 +2919,13 @@ internal func _appendingKeyPaths< unsafe component.clone( into: &destBuilder.buffer, - endOfReferencePrefix: component.header.endOfReferencePrefix) + endOfReferencePrefix: component.header.endOfReferencePrefix, + + // Leaf components, however, can appear at actual aligned spots that + // they couldn't before. Adjust computed arguments for this + // potential. + adjustForAlignment: true + ) if let type = type { unsafe destBuilder.push(type) @@ -3583,12 +3724,13 @@ internal struct GetKeyPathClassAndInstanceSizeFromPattern // However, for types who have an alignment large than pointers, we need // to determine if the current position is suitable for the argument, or // if we need to add padding bytes to align ourselves. - let misalign = unsafe size & typeAlignMask + let misaligned = unsafe size & typeAlignMask - if misalign != 0 { - let typeAlignment = typeAlignMask &+ 1 - let padding = Swift.max(0, typeAlignment &- MemoryLayout.alignment) - unsafe size &+= padding + if misaligned != 0 { + // We were misaligned, add the padding to the total size of the keypath + // buffer. + let typeAlign = typeAlignMask &+ 1 + unsafe size &+= typeAlign &- misalign } unsafe size &+= typeSize @@ -3936,8 +4078,7 @@ internal struct InstantiateKeyPathBuffer: KeyPathPatternVisitor { // check after pushing preliminary data if the resulting argument will // require padding bytes to be properly aligned. let totalSizeAddress = unsafe destData.baseAddress._unsafelyUnwrappedUnchecked - var totalSize: UInt = 0 - unsafe pushDest(totalSize) + unsafe pushDest(ComputedArgumentSize(0)) unsafe pushDest(arguments.witnesses) @@ -3955,47 +4096,49 @@ internal struct InstantiateKeyPathBuffer: KeyPathPatternVisitor { } let (typeSize, typeAlignMask) = unsafe arguments.getLayout(patternArgs) - let typeAlignment = typeAlignMask &+ 1 - let padding = Swift.max(0, typeAlignment &- MemoryLayout.alignment) + var argumentSize = UInt(truncatingIfNeeded: typeSize) + + // If an external property descriptor also has arguments, they'll be + // added to the end with pointer alignment. + if let externalArgs = unsafe externalArgs { + argumentSize = MemoryLayout._roundingUpToAlignment(totalSize) + argumentSize += UInt(truncatingIfNeeded: MemoryLayout.size * externalArgs.count) + } - totalSize = UInt(truncatingIfNeeded: typeSize) + // The argument total size contains the padding and alignment bits + // required for the argument in the top 1/3 bits, so ensure the size of + // the argument buffer is small enough to account for that. + _precondition( + argumentSize <= ComputedArgumentSize.sizeMask, + "keypath arguments are too large" + ) + + var totalSize = ComputedArgumentSize(argumentSize) // We are known to be pointer aligned at this point in the KeyPath buffer. // However, for types who have an alignment large than pointers, we need // to determine if the current position is suitable for the argument, or // if we need to add padding bytes to align ourselves. let argumentAddress = unsafe destData.baseAddress._unsafelyUnwrappedUnchecked - let misalign = Int(bitPattern: argumentAddress) & typeAlignMask + let misaligned = Int(bitPattern: argumentAddress) & typeAlignMask - if misalign != 0 { - totalSize &+= UInt(truncatingIfNeeded: padding) + if misaligned != 0 { + let typeAlignment = typeAlignMask &+ 1 + totalSize.padding = typeAlignment &- misaligned + totalSize.alignment = typeAlignment // Go ahead and append the padding. - for _ in 0 ..< padding { + for _ in 0 ..< totalSize.padding { unsafe pushDest(UInt8(0)) } } - // If an external property descriptor also has arguments, they'll be - // added to the end with pointer alignment. - if let externalArgs = unsafe externalArgs { - totalSize = MemoryLayout._roundingUpToAlignment(totalSize) - totalSize += UInt(truncatingIfNeeded: MemoryLayout.size * externalArgs.count) - } - - // The argument total size contains the padding bytes required for the - // argument in the top 4 bits, so ensure the size of the argument buffer - // is small enough to account for that. - _precondition( - totalSize <= ComputedArgumentSize.sizeMask, - "keypath arguments are too large" - ) - - totalSize |= UInt(truncatingIfNeeded: padding / MemoryLayout.size) &<< ComputedArgumentSize.paddingShift - // Ok, we've fully calculated totalSize, go ahead and update the // placeholder. - unsafe totalSizeAddress.storeBytes(of: totalSize, as: UInt.self) + unsafe totalSizeAddress.storeBytes( + of: totalSize, + as: ComputedArgumentSize.self + ) // Initialize the local candidate arguments here. unsafe _internalInvariant(Int(bitPattern: destData.baseAddress) & typeAlignMask == 0, @@ -4320,7 +4463,13 @@ public func _createOffsetBasedKeyPath( body: UnsafeRawBufferPointer(start: nil, count: 0) ) - unsafe component.clone(into: &builder.buffer, endOfReferencePrefix: false) + unsafe component.clone( + into: &builder.buffer, + endOfReferencePrefix: false, + + // The keypath will have the same shape, it's just the types that differ. + adjustForAlignment: false + ) } if _MetadataKind(root) == .struct { @@ -4394,7 +4543,11 @@ public func _rerootKeyPath( unsafe rawComponent.clone( into: &builder.buffer, - endOfReferencePrefix: rawComponent.header.endOfReferencePrefix + endOfReferencePrefix: rawComponent.header.endOfReferencePrefix, + + // Simply changing the type of the root type, so the resulting buffer + // will have the same layout. + adjustForAlignment: false ) if componentTy == nil { diff --git a/stdlib/public/core/ReflectionMirror.swift b/stdlib/public/core/ReflectionMirror.swift index 73fa13048c0d9..3a32b38a6a6c8 100644 --- a/stdlib/public/core/ReflectionMirror.swift +++ b/stdlib/public/core/ReflectionMirror.swift @@ -378,7 +378,11 @@ public func _forEachFieldWithKeyPath( body: UnsafeRawBufferPointer(start: nil, count: 0)) unsafe component.clone( into: &destBuilder.buffer, - endOfReferencePrefix: false) + endOfReferencePrefix: false, + + // We are just storing offset components and not computed ones. + adjustForAlignment: false + ) } if let name = unsafe field.name { diff --git a/test/stdlib/KeyPath.swift b/test/stdlib/KeyPath.swift index fb3b0c1e5510c..f34c4f74ceaf6 100644 --- a/test/stdlib/KeyPath.swift +++ b/test/stdlib/KeyPath.swift @@ -1159,5 +1159,117 @@ if #available(SwiftStdlib 5.9, *) { } } +@_alignment(8) +struct EightAligned { + let x = 0 +} + +extension EightAligned: Equatable {} +extension EightAligned: Hashable {} + +@_alignment(16) +struct SixteenAligned { + let x = 0 +} + +extension SixteenAligned: Equatable {} +extension SixteenAligned: Hashable {} + +struct OveralignedForEight { + subscript(getter getter: EightAligned) -> Int { + 128 + } + + subscript(setter setter: EightAligned) -> Int { + get { + 128 + } + + set { + fatalError() + } + } +} + +struct OveralignedForSixteen { + subscript(getter getter: SixteenAligned) -> Int { + 128 + } + + subscript(setter setter: SixteenAligned) -> Int { + get { + 128 + } + + set { + fatalError() + } + } +} + +struct SimpleEight { + let oa = OveralignedForEight() +} + +struct SimpleSixteen { + let oa = OveralignedForSixteen() +} + +#if _pointerBitWidth(_32) +if #available(SwiftStdlib 6.3, *) { + keyPath.test("eight byte overaligned arguments") { + let oa = OveralignedForEight() + let kp = \OveralignedForEight.[getter: EightAligned()] + + let value = oa[keyPath: kp] + expectEqual(value, 128) + + // Test that appends where the argument is no longer overaligned work + let simple = SimpleEight() + let kp11 = \SimpleEight.oa + let kp12 = \OveralignedForEight.[getter: EightAligned()] + let kp1 = kp11.appending(path: kp12) + + let value1 = simple[keyPath: kp1] + expectEqual(value1, 128) + + // Test that appends where the argument is still overaligned works + let kp21 = \SimpleEight.oa + let kp22 = \OveralignedForEight.[setter: EightAligned()] + let kp2 = kp21.appending(path: kp22) + + let value2 = simple[keyPath: kp2] + expectEqual(value2, 128) + } +} +#endif + +if #available(SwiftStdlib 6.3, *) { + keyPath.test("sixteen byte overaligned arguments") { + let oa = OveralignedForSixteen() + let kp = \OveralignedForSixteen.[SixteenAligned()] + + let value = oa[keyPath: kp] + expectEqual(value, 128) + + // Test that appends where the argument is no longer overaligned work + let simple = SimpleSixteen() + let kp11 = \SimpleSixteen.oa + let kp12 = \OveralignedForSixteen.[getter: SixteenAligned()] + let kp1 = kp11.appending(path: kp12) + + let value1 = simple[keyPath: kp1] + expectEqual(value1, 128) + + // Test that appends where the argument is still overaligned works + let kp21 = \SimpleSixteen.oa + let kp22 = \OveralignedForSixteen.[setter: SixteenAligned()] + let kp2 = kp21.appending(path: kp22) + + let value2 = simple[keyPath: kp2] + expectEqual(value2, 128) + } +} + runAllTests() From 52a7051b77900691240147550c18112007244ad3 Mon Sep 17 00:00:00 2001 From: Alejandro Alonso Date: Tue, 26 Aug 2025 10:15:09 -0700 Subject: [PATCH 03/14] Update KeyPath.swift --- stdlib/public/core/KeyPath.swift | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/stdlib/public/core/KeyPath.swift b/stdlib/public/core/KeyPath.swift index 03571f95576f1..bc182978e00d0 100644 --- a/stdlib/public/core/KeyPath.swift +++ b/stdlib/public/core/KeyPath.swift @@ -1210,17 +1210,17 @@ internal struct ComputedArgumentSize { return 0 } - #if _pointerBitWidth(_64) +#if _pointerBitWidth(_64) // On 64 bit, the only higher alignment a type could have is 16 byte. return 16 - #elseif _pointerBitWidth(_32) +#elseif _pointerBitWidth(_32) // 0 = 8 byte // 1 = 16 byte let shift = value &>> Self.alignmentShift return shift == 1 ? 16 : 8 - #else - #error("Unsupported platform") - #endif +#else +#error("Unsupported platform") +#endif } // The setter should only be called when there is or will be padding. @@ -1232,6 +1232,9 @@ internal struct ComputedArgumentSize { let reduced = newValue == 16 ? 1 : 0 let shift = reduced &<< Self.alignmentShift value |= shift +#else +#error("Unsupported platform") +#endif } } } From b7f4bb791824b9814c63a1402f5046775c057c22 Mon Sep 17 00:00:00 2001 From: Alejandro Alonso Date: Tue, 26 Aug 2025 14:46:32 -0700 Subject: [PATCH 04/14] Make ComputedArgumentSize unavailable in embedded --- stdlib/public/core/KeyPath.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/stdlib/public/core/KeyPath.swift b/stdlib/public/core/KeyPath.swift index bc182978e00d0..61bf265b628b1 100644 --- a/stdlib/public/core/KeyPath.swift +++ b/stdlib/public/core/KeyPath.swift @@ -1119,6 +1119,7 @@ internal enum KeyPathComputedIDResolution { case functionCall } +@_unavailableInEmbedded internal struct ComputedArgumentSize { let value: UInt From b6b98b281f797b0954d6fcd9215b4ef9d1ff6327 Mon Sep 17 00:00:00 2001 From: Alejandro Alonso Date: Thu, 4 Sep 2025 14:21:28 -0700 Subject: [PATCH 05/14] Update KeyPath.swift --- stdlib/public/core/KeyPath.swift | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/stdlib/public/core/KeyPath.swift b/stdlib/public/core/KeyPath.swift index 61bf265b628b1..ddd09ce6830cf 100644 --- a/stdlib/public/core/KeyPath.swift +++ b/stdlib/public/core/KeyPath.swift @@ -1121,7 +1121,7 @@ internal enum KeyPathComputedIDResolution { @_unavailableInEmbedded internal struct ComputedArgumentSize { - let value: UInt + var value: UInt static var sizeMask: UInt { #if _pointerBitWidth(_64) @@ -1191,7 +1191,8 @@ internal struct ComputedArgumentSize { } set { - let reduced = newValue / MemoryLayout.size + let new = UInt(truncatingIfNeeded: newValue) + let reduced = new / UInt(truncatingIfNeeded: MemoryLayout.size) let shift = reduced &<< Self.paddingShift value &= ~Self.paddingMask value |= shift @@ -1230,8 +1231,9 @@ internal struct ComputedArgumentSize { // Nop on 64 bit. return #elseif _pointerBitWidth(_32) - let reduced = newValue == 16 ? 1 : 0 + let reduced: UInt = newValue == 16 ? 1 : 0 let shift = reduced &<< Self.alignmentShift + value &= ~(1 &<< Self.alignmentShift) value |= shift #else #error("Unsupported platform") @@ -1936,8 +1938,8 @@ internal struct RawKeyPathComponent { ) // Push the buffer past the padding. - unsafe argumentDest += padding - componentSize += padding + unsafe argumentDest += newArgumentSize.padding + componentSize += newArgumentSize.padding } } @@ -3718,7 +3720,7 @@ internal struct GetKeyPathClassAndInstanceSizeFromPattern } // Handle local arguments. - if let arguments = arguments { + if let arguments = unsafe arguments { // Argument size and witnesses ptr. unsafe size &+= MemoryLayout.size &* 2 @@ -3734,7 +3736,7 @@ internal struct GetKeyPathClassAndInstanceSizeFromPattern // We were misaligned, add the padding to the total size of the keypath // buffer. let typeAlign = typeAlignMask &+ 1 - unsafe size &+= typeAlign &- misalign + unsafe size &+= typeAlign &- misaligned } unsafe size &+= typeSize @@ -3742,15 +3744,15 @@ internal struct GetKeyPathClassAndInstanceSizeFromPattern } // Handle external arguments. - if let externalArgs = externalArgs { + if let externalArgs = unsafe externalArgs { // Argument size and witnesses ptr if we didn't have local arguments. - if arguments == nil { + if unsafe arguments == nil { unsafe size &+= MemoryLayout.size &* 2 } // We also need to store the size of the local arguments so we can // find the external component arguments. - if arguments != nil { + if unsafe arguments != nil { unsafe size &+= RawKeyPathComponent.Header.externalWithArgumentsExtraSize } @@ -4100,13 +4102,13 @@ internal struct InstantiateKeyPathBuffer: KeyPathPatternVisitor { } let (typeSize, typeAlignMask) = unsafe arguments.getLayout(patternArgs) - var argumentSize = UInt(truncatingIfNeeded: typeSize) + var argumentSize = typeSize // If an external property descriptor also has arguments, they'll be // added to the end with pointer alignment. if let externalArgs = unsafe externalArgs { - argumentSize = MemoryLayout._roundingUpToAlignment(totalSize) - argumentSize += UInt(truncatingIfNeeded: MemoryLayout.size * externalArgs.count) + argumentSize = MemoryLayout._roundingUpToAlignment(argumentSize) + argumentSize += MemoryLayout.size * externalArgs.count } // The argument total size contains the padding and alignment bits From 13b5a44d954fdfb23fb2955dd35c5da6266a3c16 Mon Sep 17 00:00:00 2001 From: Alejandro Alonso Date: Mon, 8 Sep 2025 10:10:24 -0700 Subject: [PATCH 06/14] Update KeyPath.swift --- stdlib/public/core/KeyPath.swift | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/stdlib/public/core/KeyPath.swift b/stdlib/public/core/KeyPath.swift index ddd09ce6830cf..d539ef5392213 100644 --- a/stdlib/public/core/KeyPath.swift +++ b/stdlib/public/core/KeyPath.swift @@ -1123,6 +1123,7 @@ internal enum KeyPathComputedIDResolution { internal struct ComputedArgumentSize { var value: UInt + @_unavailableInEmbedded static var sizeMask: UInt { #if _pointerBitWidth(_64) 0x7FFF_FFFF_FFFF_FFFF @@ -1133,6 +1134,7 @@ internal struct ComputedArgumentSize { #endif } + @_unavailableInEmbedded static var paddingMask: UInt { #if _pointerBitWidth(_64) 0x8000_0000_0000_0000 @@ -1143,6 +1145,7 @@ internal struct ComputedArgumentSize { #endif } + @_unavailableInEmbedded static var paddingShift: UInt { #if _pointerBitWidth(_64) 63 @@ -1153,6 +1156,7 @@ internal struct ComputedArgumentSize { #endif } + @_unavailableInEmbedded static var alignmentShift: UInt { #if _pointerBitWidth(_64) // On 64 bit platforms, we don't need to record alignment in this structure. @@ -1204,6 +1208,7 @@ internal struct ComputedArgumentSize { // platforms, we don't actually record this in the bit pattern anywhere, but // on 32 bit platforms we use the top bit to indicate either 8 or 16 byte // alignment. This returns 0 if no padding was recorded. + @_unavailableInEmbedded var alignment: Int { get { // If we didn't record a padding value, then this argument's alignment was From 90fe494825d7d06fb2610164efb5a38af1bafe44 Mon Sep 17 00:00:00 2001 From: Alejandro Alonso Date: Tue, 9 Sep 2025 13:04:50 -0700 Subject: [PATCH 07/14] Update KeyPath.swift --- stdlib/public/core/KeyPath.swift | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/stdlib/public/core/KeyPath.swift b/stdlib/public/core/KeyPath.swift index d539ef5392213..3715cd1bb102d 100644 --- a/stdlib/public/core/KeyPath.swift +++ b/stdlib/public/core/KeyPath.swift @@ -1130,7 +1130,8 @@ internal struct ComputedArgumentSize { #elseif _pointerBitWidth(_32) 0x1FFF_FFFF #else -#error("Unsupported platform") +#warning("Unsupported platform") + fatalError() #endif } @@ -1141,7 +1142,8 @@ internal struct ComputedArgumentSize { #elseif _pointerBitWidth(_32) 0x6000_0000 #else -#error("Unsupported platform") +#warning("Unsupported platform") + fatalError() #endif } @@ -1152,7 +1154,8 @@ internal struct ComputedArgumentSize { #elseif _pointerBitWidth(_32) 29 #else -#error("Unsupported platform") +#warning("Unsupported platform") + fatalError() #endif } @@ -1164,7 +1167,8 @@ internal struct ComputedArgumentSize { #elseif _pointerBitWidth(_32) 31 #else -#error("Unsupported platform") +#warning("Unsupported platform") + fatalError() #endif } @@ -1226,7 +1230,8 @@ internal struct ComputedArgumentSize { let shift = value &>> Self.alignmentShift return shift == 1 ? 16 : 8 #else -#error("Unsupported platform") +#warning("Unsupported platform") + fatalError() #endif } @@ -1241,7 +1246,8 @@ internal struct ComputedArgumentSize { value &= ~(1 &<< Self.alignmentShift) value |= shift #else -#error("Unsupported platform") +#warning("Unsupported platform") + fatalError() #endif } } From 9933d3ccb2e9f727bb596aa1fc9e54ce8921896a Mon Sep 17 00:00:00 2001 From: Alejandro Alonso Date: Tue, 9 Sep 2025 14:35:34 -0700 Subject: [PATCH 08/14] Update KeyPath.swift --- test/stdlib/KeyPath.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/stdlib/KeyPath.swift b/test/stdlib/KeyPath.swift index f34c4f74ceaf6..5da22242f33b7 100644 --- a/test/stdlib/KeyPath.swift +++ b/test/stdlib/KeyPath.swift @@ -1247,7 +1247,7 @@ if #available(SwiftStdlib 6.3, *) { if #available(SwiftStdlib 6.3, *) { keyPath.test("sixteen byte overaligned arguments") { let oa = OveralignedForSixteen() - let kp = \OveralignedForSixteen.[SixteenAligned()] + let kp = \OveralignedForSixteen.[getter: SixteenAligned()] let value = oa[keyPath: kp] expectEqual(value, 128) From 10714518445d61e039650db2ced3fd62facd1671 Mon Sep 17 00:00:00 2001 From: Alejandro Alonso Date: Tue, 16 Sep 2025 10:04:25 -0700 Subject: [PATCH 09/14] Force keypath objects to be 16 byte aligned --- stdlib/public/core/KeyPath.swift | 78 ++++++++++++++++++++++---------- 1 file changed, 54 insertions(+), 24 deletions(-) diff --git a/stdlib/public/core/KeyPath.swift b/stdlib/public/core/KeyPath.swift index 3715cd1bb102d..08e07d5724f4f 100644 --- a/stdlib/public/core/KeyPath.swift +++ b/stdlib/public/core/KeyPath.swift @@ -154,8 +154,37 @@ public class AnyKeyPath: _AppendKeyPath { ) -> Self { _internalInvariant(bytes > 0 && bytes % 4 == 0, "capacity must be multiple of 4 bytes") - let result = Builtin.allocWithTailElems_1(self, (bytes/4)._builtinWordValue, - Int32.self) + + // Subtract the buffer header and any padding it may have from the number of + // bytes we need to allocate. The alloc below has a 0 sized 16 byte aligned + // tail element that will force the compiler to insert buffer header + any + // padding necessary to accomodate. + let bytesWithoutHeader = bytes &- MemoryLayout.size + + let result = Builtin.allocWithTailElems_2( + self, + + // This tail element accomplishes two things: + // 1. Guarantees a 16 byte alignment for the allocated object pointer. + // 2. Forces the compiler to add padding after the kvc string to support + // this 16 byte aligned tail element. + // + // The size of keypath objects before any tail elements is 24 bytes large + // on 64 bit platforms and 12 bytes large on 32 bit platforms. + // (Isa, RC, and KVC). The amount of padding bytes needed after the kvc + // string pointer to support a 16 byte aligned tail element is the exact + // same amount of bytes that the keypath buffer header occupies on both + // 64 & 32 bit platforms (because we always start component headers on + // pointer aligned addresses). That is the purpose for the subtraction + // above. The result of this tail element should just be the 16 byte + // pointer aligned requirement and no extra bytes allocated. + 0._builtinWordValue, + SIMD16.self, + + (bytesWithoutHeader/4)._builtinWordValue, + Int32.self + ) + unsafe result._kvcKeyPathStringPtr = nil let base = UnsafeMutableRawPointer(Builtin.projectTailElems(result, Int32.self)) @@ -3071,7 +3100,7 @@ public func _swift_getKeyPath(pattern: UnsafeMutableRawPointer, // Instantiate a new key path object modeled on the pattern. // Do a pass to determine the class of the key path we'll be instantiating // and how much space we'll need for it. - let (keyPathClass, rootType, size, sizeWithMaxSize, _) + let (keyPathClass, rootType, size, sizeWithMaxSize) = unsafe _getKeyPathClassAndInstanceSizeFromPattern(patternPtr, arguments) var pureStructOffset: UInt32? = nil @@ -3633,6 +3662,9 @@ internal struct GetKeyPathClassAndInstanceSizeFromPattern // start with one word for the header var size: Int = MemoryLayout.size var sizeWithMaxSize: Int = 0 + var sizeWithObjectHeaderAndKvc: Int { + size &+ MemoryLayout.size * 3 + } var capability: KeyPathKind = .value var didChain: Bool = false @@ -3735,13 +3767,24 @@ internal struct GetKeyPathClassAndInstanceSizeFromPattern // Argument size and witnesses ptr. unsafe size &+= MemoryLayout.size &* 2 + // If we also have external arguments, we need to store the size of the + // local arguments so we can find the external component arguments. + if unsafe externalArgs != nil { + unsafe size &+= RawKeyPathComponent.Header.externalWithArgumentsExtraSize + } + let (typeSize, typeAlignMask) = unsafe arguments.getLayout(patternArgs) // We are known to be pointer aligned at this point in the KeyPath buffer. // However, for types who have an alignment large than pointers, we need // to determine if the current position is suitable for the argument, or // if we need to add padding bytes to align ourselves. - let misaligned = unsafe size & typeAlignMask + // + // Note: We need to account for the object header and kvc pointer because + // it's an odd number of words which will affect whether the size visitor + // determines if we need padding. It would cause out of sync answers when + // going to actually instantiate the buffer. + let misaligned = unsafe sizeWithObjectHeaderAndKvc & typeAlignMask if misaligned != 0 { // We were misaligned, add the padding to the total size of the keypath @@ -3761,12 +3804,6 @@ internal struct GetKeyPathClassAndInstanceSizeFromPattern unsafe size &+= MemoryLayout.size &* 2 } - // We also need to store the size of the local arguments so we can - // find the external component arguments. - if unsafe arguments != nil { - unsafe size &+= RawKeyPathComponent.Header.externalWithArgumentsExtraSize - } - unsafe size &+= MemoryLayout.size &* externalArgs.count } } @@ -3813,8 +3850,7 @@ internal func _getKeyPathClassAndInstanceSizeFromPattern( keyPathClass: AnyKeyPath.Type, rootType: Any.Type, size: Int, - sizeWithMaxSize: Int, - alignmentMask: Int + sizeWithMaxSize: Int ) { var walker = unsafe GetKeyPathClassAndInstanceSizeFromPattern(patternArgs: arguments) unsafe _walkKeyPathPattern(pattern, walker: &walker) @@ -3840,12 +3876,12 @@ internal func _getKeyPathClassAndInstanceSizeFromPattern( } let classTy = unsafe _openExistential(walker.root!, do: openRoot) - return unsafe (keyPathClass: classTy, - rootType: walker.root!, - size: walker.size, - sizeWithMaxSize: walker.sizeWithMaxSize, - // FIXME: Handle overalignment - alignmentMask: MemoryLayout._alignmentMask) + return unsafe ( + keyPathClass: classTy, + rootType: walker.root!, + size: walker.size, + sizeWithMaxSize: walker.sizeWithMaxSize + ) } internal func _getTypeSize(_: Type.Type) -> Int { @@ -4279,7 +4315,6 @@ internal struct ValidatingInstantiateKeyPathBuffer: KeyPathPatternVisitor { offset: offset) unsafe checkSizeConsistency() unsafe structOffset = unsafe instantiateVisitor.structOffset - unsafe isPureStruct.append(contentsOf: instantiateVisitor.isPureStruct) } mutating func visitComputedComponent(mutating: Bool, idKind: KeyPathComputedIDKind, @@ -4311,31 +4346,26 @@ internal struct ValidatingInstantiateKeyPathBuffer: KeyPathPatternVisitor { // Note: For this function and the ones below, modification of structOffset // is omitted since these types of KeyPaths won't have a pureStruct // offset anyway. - unsafe isPureStruct.append(contentsOf: instantiateVisitor.isPureStruct) unsafe checkSizeConsistency() } mutating func visitOptionalChainComponent() { unsafe sizeVisitor.visitOptionalChainComponent() unsafe instantiateVisitor.visitOptionalChainComponent() - unsafe isPureStruct.append(contentsOf: instantiateVisitor.isPureStruct) unsafe checkSizeConsistency() } mutating func visitOptionalWrapComponent() { unsafe sizeVisitor.visitOptionalWrapComponent() unsafe instantiateVisitor.visitOptionalWrapComponent() - unsafe isPureStruct.append(contentsOf: instantiateVisitor.isPureStruct) unsafe checkSizeConsistency() } mutating func visitOptionalForceComponent() { unsafe sizeVisitor.visitOptionalForceComponent() unsafe instantiateVisitor.visitOptionalForceComponent() - unsafe isPureStruct.append(contentsOf: instantiateVisitor.isPureStruct) unsafe checkSizeConsistency() } mutating func visitIntermediateComponentType(metadataRef: MetadataReference) { unsafe sizeVisitor.visitIntermediateComponentType(metadataRef: metadataRef) unsafe instantiateVisitor.visitIntermediateComponentType(metadataRef: metadataRef) - unsafe isPureStruct.append(contentsOf: instantiateVisitor.isPureStruct) unsafe checkSizeConsistency() } From c4ab943d55f8ae79224d7b590df89852608ea915 Mon Sep 17 00:00:00 2001 From: Alejandro Alonso Date: Tue, 16 Sep 2025 13:41:31 -0700 Subject: [PATCH 10/14] Prevent KeyPathBuffer.next from being inlined --- stdlib/public/core/KeyPath.swift | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/stdlib/public/core/KeyPath.swift b/stdlib/public/core/KeyPath.swift index 08e07d5724f4f..8abd6a26b1bc9 100644 --- a/stdlib/public/core/KeyPath.swift +++ b/stdlib/public/core/KeyPath.swift @@ -2380,6 +2380,7 @@ internal struct KeyPathBuffer { } } + @inline(never) internal mutating func next() -> (RawKeyPathComponent, Any.Type?) { let header = unsafe _pop(from: &data, as: RawKeyPathComponent.Header.self) // Track if this is the last component of the reference prefix. @@ -3663,7 +3664,7 @@ internal struct GetKeyPathClassAndInstanceSizeFromPattern var size: Int = MemoryLayout.size var sizeWithMaxSize: Int = 0 var sizeWithObjectHeaderAndKvc: Int { - size &+ MemoryLayout.size * 3 + unsafe size &+ MemoryLayout.size &* 3 } var capability: KeyPathKind = .value From 4d622abf39cc131b3c33febf6970d07e5065c0ce Mon Sep 17 00:00:00 2001 From: Alejandro Alonso Date: Thu, 18 Sep 2025 11:38:44 -0700 Subject: [PATCH 11/14] Always set the alignment bit, and accurately calculate appended leaf component sizes --- stdlib/public/core/KeyPath.swift | 268 +++++++++++++++++++++---------- 1 file changed, 183 insertions(+), 85 deletions(-) diff --git a/stdlib/public/core/KeyPath.swift b/stdlib/public/core/KeyPath.swift index 8abd6a26b1bc9..53ae4bd3d7597 100644 --- a/stdlib/public/core/KeyPath.swift +++ b/stdlib/public/core/KeyPath.swift @@ -1155,9 +1155,9 @@ internal struct ComputedArgumentSize { @_unavailableInEmbedded static var sizeMask: UInt { #if _pointerBitWidth(_64) - 0x7FFF_FFFF_FFFF_FFFF + 0x3FFF_FFFF_FFFF_FFFF #elseif _pointerBitWidth(_32) - 0x1FFF_FFFF + 0xFFF_FFFF #else #warning("Unsupported platform") fatalError() @@ -1167,9 +1167,9 @@ internal struct ComputedArgumentSize { @_unavailableInEmbedded static var paddingMask: UInt { #if _pointerBitWidth(_64) - 0x8000_0000_0000_0000 + 0x4000_0000_0000_0000 #elseif _pointerBitWidth(_32) - 0x6000_0000 + 0x3000_0000 #else #warning("Unsupported platform") fatalError() @@ -1179,9 +1179,21 @@ internal struct ComputedArgumentSize { @_unavailableInEmbedded static var paddingShift: UInt { #if _pointerBitWidth(_64) - 63 + 62 #elseif _pointerBitWidth(_32) - 29 + 28 +#else +#warning("Unsupported platform") + fatalError() +#endif + } + + @_unavailableInEmbedded + static var alignmentMask: UInt { +#if _pointerBitWidth(_64) + 0x8000_0000_0000_0000 +#elseif _pointerBitWidth(_32) + 0x6000_0000 #else #warning("Unsupported platform") fatalError() @@ -1191,10 +1203,9 @@ internal struct ComputedArgumentSize { @_unavailableInEmbedded static var alignmentShift: UInt { #if _pointerBitWidth(_64) - // On 64 bit platforms, we don't need to record alignment in this structure. - 0 + 63 #elseif _pointerBitWidth(_32) - 31 + 30 #else #warning("Unsupported platform") fatalError() @@ -1207,18 +1218,18 @@ internal struct ComputedArgumentSize { // The size of just the arguments only, ignoring padding. var argumentSize: Int { - totalSize &- padding + Int(truncatingIfNeeded: value & Self.sizeMask) } - // The total size of the argument buffer is in the bottom 29/63 bits. + // The total size of the argument buffer. var totalSize: Int { - Int(truncatingIfNeeded: value & Self.sizeMask) + argumentSize &+ padding } // The number of padding bytes required so that the argument is properly - // aligned in the keypath buffer. This is in the top 2 bits for 32 bit - // platforms (because they need to represent 8 and 16 overaligned types) and - // top 1 bit for 64 bit platforms (because they only need to represent 16 + // aligned in the keypath buffer. This is in the top 3rd and 4th bits for 32 + // bit platforms (because they need to represent 8 and 16 overaligned types) + // and top 2nd bit for 64 bit platforms (because they only need to represent 16 // alignment). var padding: Int { get { @@ -1237,47 +1248,59 @@ internal struct ComputedArgumentSize { } // If the arguments have an alignment greater than the platform's pointer - // alignment, then this structure records the argument's alignment. On 64 bit - // platforms, we don't actually record this in the bit pattern anywhere, but - // on 32 bit platforms we use the top bit to indicate either 8 or 16 byte - // alignment. This returns 0 if no padding was recorded. + // alignment, then this structure records the argument's alignment. Even if + // the argument didn't require padding, we still record the argument's + // alignment here to prevent overallocation on keypath appends (because a leaf + // component may suddenly need padding or lose padding). This returns 0 if the + // argument had less than or equal to the platform's pointer alignment. The + // only valid values of this on 64 bit are 16 and 0, while on 32 it is 16, 8, + // and 0. Top bit on 64 bit platforms and top 2 bits on 32 bit platforms. @_unavailableInEmbedded var alignment: Int { get { - // If we didn't record a padding value, then this argument's alignment was - // either equal to or less than pointer value alignment. - guard padding != 0 else { - return 0 - } + let mask = value & Self.alignmentMask + let shift = mask &>> Self.alignmentShift #if _pointerBitWidth(_64) // On 64 bit, the only higher alignment a type could have is 16 byte. - return 16 + return shift == 1 ? 16 : 0 #elseif _pointerBitWidth(_32) - // 0 = 8 byte - // 1 = 16 byte - let shift = value &>> Self.alignmentShift - return shift == 1 ? 16 : 8 + switch shift { + case 1: + return 8 + case 2: + return 16 + default: + return 0 + } #else #warning("Unsupported platform") fatalError() #endif } - // The setter should only be called when there is or will be padding. set { #if _pointerBitWidth(_64) - // Nop on 64 bit. - return -#elseif _pointerBitWidth(_32) let reduced: UInt = newValue == 16 ? 1 : 0 - let shift = reduced &<< Self.alignmentShift - value &= ~(1 &<< Self.alignmentShift) - value |= shift +#elseif _pointerBitWidth(_32) + let reduced: UInt + + switch newValue { + case 8: + reduced = 1 + case 16: + reduced = 2 + default: + reduced = 0 + } #else #warning("Unsupported platform") fatalError() #endif + + let shift = reduced &<< Self.alignmentShift + value &= ~Self.alignmentMask + value |= shift } } } @@ -1919,8 +1942,7 @@ internal struct RawKeyPathComponent { let argumentSizePtr = unsafe buffer.baseAddress._unsafelyUnwrappedUnchecked + componentSize // If we don't need to adjust for alignment, then the current argument - // size is correct. Or, if the arguments themselves don't require any - // padding whatsoever. If alignment is 0, then the arguments had equal + // size is correct. If alignment is 0, then the arguments had equal // to or less than alignment to the platform's word alignment. if !adjustForAlignment || argumentSize.alignment == 0 { unsafe buffer.storeBytes( @@ -1951,24 +1973,31 @@ internal struct RawKeyPathComponent { var argumentDest = unsafe buffer.baseAddress.unsafelyUnwrapped + componentSize + // If we're not adjusting for alignment, but we had existing padding, we + // need to mimic the exact padding. + if !adjustForAlignment, argumentSize.padding != 0 { + unsafe argumentDest += argumentSize.padding + componentSize &+= argumentSize.padding + } + // We've been asked to adjust for alignment and the original argument // was overaligned. Check for misalignment at our current destination. if adjustForAlignment, argumentSize.alignment != 0 { + var newArgumentSize = ComputedArgumentSize(argumentSize.argumentSize) let argumentAlignmentMask = argumentSize.alignment &- 1 let misaligned = Int(bitPattern: argumentDest) & argumentAlignmentMask - // Ok, this is no longer misaligned. We don't need to record any - // padding. if misaligned == 0 { + newArgumentSize.padding = 0 + newArgumentSize.alignment = argumentSize.alignment + unsafe argumentSizePtr.storeBytes( - of: argumentSize.argumentSize, - as: Int.self + of: newArgumentSize, + as: ComputedArgumentSize.self ) } else { // Otherwise, this is still misaligned and we need to calculate // how much padding we need. - var newArgumentSize = ComputedArgumentSize(argumentSize.argumentSize) - newArgumentSize.padding = argumentSize.alignment &- misaligned newArgumentSize.alignment = argumentSize.alignment @@ -2831,6 +2860,100 @@ internal func _tryToAppendKeyPaths( return _openExistential(rootRoot, do: open) } +internal func calculateAppendedKeyPathSize( + _ root: AnyKeyPath, + _ leaf: AnyKeyPath, + _ rootBuffer: KeyPathBuffer, + _ leafBuffer: KeyPathBuffer +) -> (Int, Int, Int, Int, Int, Int) { + var result = MemoryLayout.size // Header size (padding if needed) + var resultWithObjectHeaderAndKVC: Int { + result &+ MemoryLayout.size &* 3 + } + + // Result buffer has room for both key paths' components, plus the + // header, plus space for the middle type. + // Align up the root so that we can put the component type after it. + result &+= unsafe MemoryLayout._roundingUpToAlignment(rootBuffer.data.count) + result &+= MemoryLayout.size // Middle type + + var leafIter = unsafe leafBuffer + while true { + let (component, nextType) = unsafe leafIter.next() + let isLast = nextType == nil + + result &+= MemoryLayout.size + + if component.header.kind == .computed, + component.header.hasComputedArguments, + component._computedArgumentSize.alignment != 0 { + result = MemoryLayout._roundingUpToAlignment(result) + + // Id and getter + result &+= MemoryLayout.size &* 2 + + if component.header.isComputedSettable { + result &+= MemoryLayout.size + } + + // Argument size and witness table + result &+= MemoryLayout.size &* 2 + + // If we also have external arguments, we need to store the size of the + // local arguments so we can find the external component arguments. + if component.header.isComputedInstantiatedFromExternalWithArguments { + result &+= RawKeyPathComponent.Header.externalWithArgumentsExtraSize + } + + let alignmentMask = component._computedArgumentSize.alignment &- 1 + let misaligned = resultWithObjectHeaderAndKVC & alignmentMask + + if misaligned != 0 { + result &+= component._computedArgumentSize.alignment &- misaligned + } + + result &+= component._computedArgumentSize.argumentSize + } else { + result &+= component.bodySize + } + + if isLast { + break + } + } + + // Size of just our components is equal to root + middle + leaf (get rid of + // the header and any padding needed). + let componentSize = result &- MemoryLayout.size + + // The first member after the components is the maxSize of the keypath. + result = MemoryLayout._roundingUpToAlignment(result) + result &+= MemoryLayout.size + + // Reserve room for the appended KVC string, if both key paths are + // KVC-compatible. + let appendedKVCLength: Int, rootKVCLength: Int, leafKVCLength: Int + + if root.getOffsetFromStorage() == nil, leaf.getOffsetFromStorage() == nil, + let rootPtr = unsafe root._kvcKeyPathStringPtr, + let leafPtr = unsafe leaf._kvcKeyPathStringPtr { + rootKVCLength = unsafe Int(_swift_stdlib_strlen(rootPtr)) + leafKVCLength = unsafe Int(_swift_stdlib_strlen(leafPtr)) + // root + "." + leaf + appendedKVCLength = rootKVCLength &+ 1 &+ leafKVCLength &+ 1 + } else { + rootKVCLength = 0 + leafKVCLength = 0 + appendedKVCLength = 0 + } + + // Immediately following is the tail-allocated space for the KVC string. + let totalResultSize = MemoryLayout._roundingUpToAlignment(result &+ appendedKVCLength) + + return (result, totalResultSize, componentSize, appendedKVCLength, + rootKVCLength, leafKVCLength) +} + @usableFromInline @_unavailableInEmbedded internal func _appendingKeyPaths< @@ -2855,44 +2978,19 @@ internal func _appendingKeyPaths< return unsafe unsafeDowncast(leaf, to: Result.self) } - // Reserve room for the appended KVC string, if both key paths are - // KVC-compatible. - let appendedKVCLength: Int, rootKVCLength: Int, leafKVCLength: Int - - if root.getOffsetFromStorage() == nil, leaf.getOffsetFromStorage() == nil, - let rootPtr = unsafe root._kvcKeyPathStringPtr, - let leafPtr = unsafe leaf._kvcKeyPathStringPtr { - rootKVCLength = unsafe Int(_swift_stdlib_strlen(rootPtr)) - leafKVCLength = unsafe Int(_swift_stdlib_strlen(leafPtr)) - // root + "." + leaf - appendedKVCLength = rootKVCLength + 1 + leafKVCLength + 1 - } else { - rootKVCLength = 0 - leafKVCLength = 0 - appendedKVCLength = 0 - } - - // Result buffer has room for both key paths' components, plus the - // header, plus space for the middle type. - // Align up the root so that we can put the component type after it. - let rootSize = unsafe MemoryLayout._roundingUpToAlignment(rootBuffer.data.count) - var resultSize = unsafe rootSize + // Root component size - leafBuffer.data.count + // Leaf component size - MemoryLayout.size // Middle type - - // Size of just our components is equal to root + leaf + middle - let componentSize = resultSize - - resultSize += MemoryLayout.size // Header size (padding if needed) - - // The first member after the components is the maxSize of the keypath. - resultSize = MemoryLayout._roundingUpToAlignment(resultSize) - resultSize += MemoryLayout.size - - // Immediately following is the tail-allocated space for the KVC string. - let totalResultSize = MemoryLayout - ._roundingUpToAlignment(resultSize + appendedKVCLength) - + let ( + resultSize, + totalResultSize, + componentSize, + appendedKVCLength, + rootKVCLength, + leafKVCLength + ) = unsafe calculateAppendedKeyPathSize( + root, + leaf, + rootBuffer, + leafBuffer + ) var kvcStringBuffer: UnsafeMutableRawPointer? = nil let result = unsafe resultTy._create(capacityInBytes: totalResultSize) { @@ -3408,7 +3506,7 @@ internal func _walkKeyPathPattern( default: unsafe offset = unsafe .inline(header.storedOffsetPayload) } - let kind: KeyPathStructOrClass = header.kind == .struct + let kind: KeyPathStructOrClass = header.kind == .struct ? .struct : .class unsafe walker.visitStoredComponent(kind: kind, mutable: header.isStoredMutable, @@ -4168,6 +4266,8 @@ internal struct InstantiateKeyPathBuffer: KeyPathPatternVisitor { ) var totalSize = ComputedArgumentSize(argumentSize) + let typeAlignment = typeAlignMask &+ 1 + totalSize.alignment = typeAlignment // We are known to be pointer aligned at this point in the KeyPath buffer. // However, for types who have an alignment large than pointers, we need @@ -4177,9 +4277,7 @@ internal struct InstantiateKeyPathBuffer: KeyPathPatternVisitor { let misaligned = Int(bitPattern: argumentAddress) & typeAlignMask if misaligned != 0 { - let typeAlignment = typeAlignMask &+ 1 totalSize.padding = typeAlignment &- misaligned - totalSize.alignment = typeAlignment // Go ahead and append the padding. for _ in 0 ..< totalSize.padding { From 465ed1d36de4f4645295b190ac56ad8d9f496524 Mon Sep 17 00:00:00 2001 From: Alejandro Alonso Date: Thu, 18 Sep 2025 13:33:20 -0700 Subject: [PATCH 12/14] Update KeyPath.swift --- stdlib/public/core/KeyPath.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/stdlib/public/core/KeyPath.swift b/stdlib/public/core/KeyPath.swift index 53ae4bd3d7597..a3779d201a1d3 100644 --- a/stdlib/public/core/KeyPath.swift +++ b/stdlib/public/core/KeyPath.swift @@ -2860,6 +2860,7 @@ internal func _tryToAppendKeyPaths( return _openExistential(rootRoot, do: open) } +@_unavailableInEmbedded internal func calculateAppendedKeyPathSize( _ root: AnyKeyPath, _ leaf: AnyKeyPath, From aed77472e18a22cf58918573af6a65d4ecd5adc9 Mon Sep 17 00:00:00 2001 From: Alejandro Alonso Date: Mon, 22 Sep 2025 17:27:48 -0700 Subject: [PATCH 13/14] Update KeyPath.swift --- stdlib/public/core/KeyPath.swift | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/stdlib/public/core/KeyPath.swift b/stdlib/public/core/KeyPath.swift index a3779d201a1d3..0e46df526e3cd 100644 --- a/stdlib/public/core/KeyPath.swift +++ b/stdlib/public/core/KeyPath.swift @@ -1280,11 +1280,11 @@ internal struct ComputedArgumentSize { } set { + var reduced: UInt = 0 + #if _pointerBitWidth(_64) - let reduced: UInt = newValue == 16 ? 1 : 0 + reduced = newValue == 16 ? 1 : 0 #elseif _pointerBitWidth(_32) - let reduced: UInt - switch newValue { case 8: reduced = 1 From ac376e97f2648d13e7e37ba79c3a1fc9e95f7e1e Mon Sep 17 00:00:00 2001 From: Alejandro Alonso Date: Wed, 24 Sep 2025 09:55:12 -0700 Subject: [PATCH 14/14] Don't use SIMD because it may not exist --- stdlib/public/core/KeyPath.swift | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/stdlib/public/core/KeyPath.swift b/stdlib/public/core/KeyPath.swift index 0e46df526e3cd..251e95467086c 100644 --- a/stdlib/public/core/KeyPath.swift +++ b/stdlib/public/core/KeyPath.swift @@ -24,6 +24,11 @@ internal func _abstract( #endif } +@_alignment(16) +struct _SixteenAligned { + let x: UInt8 +} + // MARK: Type-erased abstract base classes // NOTE: older runtimes had Swift.AnyKeyPath as the ObjC name. @@ -179,7 +184,7 @@ public class AnyKeyPath: _AppendKeyPath { // above. The result of this tail element should just be the 16 byte // pointer aligned requirement and no extra bytes allocated. 0._builtinWordValue, - SIMD16.self, + _SixteenAligned.self, (bytesWithoutHeader/4)._builtinWordValue, Int32.self