Mark Bundle(_dsoHandle:) call in macro expansion with unsafe

[WindowsMEMZ]

#bundle macro generates a warning when Strict Memory Safety is enabled.

[jmschonfeld]

Thanks for posting this! We should definitely come up with a solution here that resolves warnings caused when using #bundle with strict memory safety enabled.

That being said, I think instead of annotating this expression as unsafe, it might be better to annotate Bundle(_dsoHandle:) as @safe instead. Bundle(_dsoHandle:) is always safe - we never try to dereference the pointer, we just lookup what binary contains this pointer which doesn't come with any memory unsafety (and we gracefully handle cases where it's not a pointer to any binary). Marking Bundle(_dsoHandle:) as @safe would eliminate the need for unsafe here and would signify that this API isn't actually unsafe. What do you think about that approach?

side note: Bundle(_dsoHandle:) isn't actually in this repo since Bundle isn't in swift-foundation yet, so changing that wouldn't be a change here but we can still make that happen if it's the right approach.

cc @Tantalum73 in case you have any thoughts

[WindowsMEMZ]

Thanks for your reply! I didn't found the declaration of Bundle.init(_dsoHandle) in any repo so I think it might not be open-sourced yet. Thanks to the @_alwaysEmitIntoClient attribute on it, I found this from its swift interface:

@available(macOS 12, iOS 15, tvOS 15, watchOS 8, visionOS 1, *)
@_alwaysEmitIntoClient convenience public init?(_dsoHandle: Swift.UnsafeRawPointer) {
  if #available(macOS 16, iOS 19, tvOS 19, watchOS 9, visionOS 3, *) {
    self.init(__dsoHandle: _dsoHandle)
  } else {
    let zeroBytes = "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0" as StaticString
    let pointer = UnsafeRawPointer(zeroBytes.utf8Start).alignedUp(toMultipleOf: MemoryLayout<Int>.size)
    self.init(for: unsafeBitCast(pointer, to: AnyClass.self))
  }
}

I can't continue to inspect how init(__dsoHandle:) works. However, in the else code block of #available expression, unsafeBitCast(_:to:) is called, which is unsafe. So I think Bundle(_dsoHandle:) is also unsafe.

[glessard]

I don't think this unsafeBitCast is more unsafe than getting and operating on the pointer itself, to be honest. This being said, I am not convinced that we should apply the @safe attribute to Bundle.init(__dsoHandle) at this time. I think it's prudent to use the unsafe marker as introduced in this PR, as it means there is no rush to audit this Bundle initializer. If there were a later decision to mark the Bundle initializer as safe in the SDK, we can then remove the marker from the macro.

[jmschonfeld]

Thanks all yeah after thinking longer on it I agree with @glessard - lets go with adding this unsafe for now since while the underlying calls that Foundation makes (effectively dladdr) should be safe, it's not a formal guarantee and I also haven't evaluated if that'd be the case on other platforms too.

@WindowsMEMZ have you tried running the unit tests against this change? I suspect that the macro unit test for #bundle needs to be updated to include this unsafe keyword in the expected expansion for it to pass (but if it did pass for you we can go ahead and kick off CI to confirm)

[WindowsMEMZ]

Thanks for pointing it out. I've add unsafe to expected expansion in the latest commit.

[jmschonfeld]

@swift-ci please test