OpenBSD support. #349

3405691582 · 2025-04-05T18:57:47Z

OpenBSD support.

Checklist

I've run tests to see all new and existing tests pass
I've followed the code style of the rest of the project
I've read the Contribution Guidelines
I've updated the documentation if necessary (not necessary)

If you've made changes to `gyb` files

I've run .script/generate_boilerplate_files_with_gyb and included updated generated files in a commit of this pull request (not applicable)

Motivation:

swift-crypto is a required dependency for swift-package-manager, and must build and run on OpenBSD.

Modifications:

OpenBSD support for cmake/swiftpm.

* Add to patterns for CMakeLists. Also provide a helpful error so future
  porters have a slightly better idea of what might need to be updated.

* Proactively reverse sense in some system name checks so they read as
  "if not Darwin". This is likely safer to do here than elsewhere since
  there are likely not going to be other platforms for which swift-crypto
  already exists.

* Add to Package.swift

ManagedBuffer.capacity is unavailable on OpenBSD.

ManagedBuffer.capacity depends on malloc introspection (e.g.,
`malloc_size`), which is not available on this platform -- and is thus
marked as such.

Here, SecureBytes uses a ManagedBuffer in Backing. We have capacity
tracked in the BackingHeader, and most of the references to the
ManagedBuffer.capacity property can be replaced with references to the
capacity in the header. However, since capacity in the header is marked
as internal which conflicts with some of the @inline functions, so we
must change those to @usableFromInline for the platform as well.

Add conditional for RandomBytes.

Required for tests to build on OpenBSD.

Result:

swift-crypto builds and runs on OpenBSD.

* Add to patterns for CMakeLists. Also provide a helpful error so future porters have a slightly better idea of what might need to be updated. * Proactively reverse sense in some system name checks so they read as "if not Darwin". This is likely safer to do here than elsewhere since there are likely not going to be other platforms for which swift-crypto already exists. * Add to Package.swift.

@inline

ManagedBuffer.capacity depends on malloc introspection (e.g., `malloc_size`), which is not available on this platform -- and is thus marked as such. Here, SecureBytes uses a ManagedBuffer in Backing. We have capacity tracked in the BackingHeader, and most of the references to the ManagedBuffer.capacity property can be replaced with references to the capacity in the header. However, since capacity in the header is marked as internal which conflicts with some of the @inline functions, so we must change those to @usableFromInline for the platform as well.

As in the prior commit, ManagedBuffer.capacity is unavailable, therefore to get the capacity here, we need to inspect the header, which is marked as internal.

Lukasa · 2025-04-09T12:36:28Z

Sources/CCryptoBoringSSL/CMakeLists.txt

@@ -334,6 +334,8 @@ elseif(CMAKE_SYSTEM_NAME MATCHES "Linux|Android|FreeBSD" AND CMAKE_SYSTEM_PROCES
    gen/bcm/vpaes-armv8-linux.S
    gen/crypto/chacha-armv8-linux.S
    gen/crypto/chacha20_poly1305_armv8-linux.S)
+else()


These cmakelists.txt files are autogenerated, so you'll need to modify the generation script and then re-run it to produce this output.

I don't understand. The scripts don't seem to modify the CMakeLists, only the underlying source, which I'm not touching. If it helps, I can leave off the extra helpful error here. Could you elaborate a bit more?

Sources/Crypto/Util/SecureBytes.swift

Lukasa · 2025-04-10T09:52:03Z

Sources/Crypto/Util/SecureBytes.swift

-    @inlinable
-    /* private but inlinable */ func _withVeryUnsafeMutableBytes<T>(_ body: (UnsafeMutableRawBufferPointer) throws -> T) rethrows -> T {
-        let capacity = self.capacity
+    @usableFromInline


Why were these dropped from @inlinable?

I had to change these from @inlinable to @usableFromInline because the new allocatedCapacity computed property refers to BackingHeader which is internal; otherwise I would get a compiler error. If you mean the comments, then I missed there was a similar commenting convention for "private but usableFromInline" and added that back in now.

Ah, so let's push the computed property one layer out. We'd like the body of this method to be inlinable, so we can add another computed property in the outer type that is @usableFromInline, which can keep this method accessible.

I'm not sure I follow, sorry. _withVeryUnsafeMutableBytes and _copyBytes are extensions on SecureBytes.Backing and right now allocatedCapacity is defined on SecureBytes.Backing. If I set _copyBytes and _withVeryUnsafeMutableBytes back to @inlinable and add an extra computed property on SecureBacking, I still get errors, so I think I'm misunderstanding something.

Hrm, if the two methods are on SecureBytes.Backing and allocatedCapacity is also on SecureBytes.Backing, merely making allocatedCapacity @usableFromInline should be enough to flip these back to @inlinable.

The error specifically reads something like this (same for _withVeryUnsafeMutableBytes):

/tmp/swift-crypto/Sources/Crypto/Util/SecureBytes.swift:443:51: error: property 'allocatedCapacity' is internal and cannot be referenced from an '@inlinable' function 332 | 333 | @usableFromInline 334 | var allocatedCapacity: Int { | `- note: property 'allocatedCapacity' is not '@usableFromInline' or public 335 | #if os(OpenBSD) 336 | return self.header.capacity : 441 | /* private but inlinable */ func _copyBytes<C: Collection>(_ bytes: C, at offset: Int) where C.Element == UInt8 { 442 | precondition(offset >= 0) 443 | precondition(offset + bytes.count <= self.allocatedCapacity) | `- error: property 'allocatedCapacity' is internal and cannot be referenced from an '@inlinable' function 444 | 445 | let byteRange = offset..<(offset + bytes.count)

That error seems...entirely wrong. It's @usableFromInline, the annotation is right here.

I found an apparent workaround: adding the property to the same section that _copyBytes is in makes this work somehow.

It may be slightly more performant to special-case but select for readability here.

Required for tests to build on OpenBSD.

Lukasa

Nice one, this is looking good. Thanks!

3405691582 · 2025-04-18T01:18:15Z

As a heads-up, I think this works fine with swiftpm/swift build but not with CMake (e.g. during swiftpm bootstrap). I'll send a followup pr if I can untangle the knot.

In apple#349 we added a new computed property to handle the fact that ManagedBuffer.capacity is unavailable on OpenBSD. In doing this, however we encountered some strange errors about this property being `internal and cannot be referenced from an '@inlinable' function` despite the fact we marked it `@usableFromInline`. Moving the computed property to the nested class extension seemed to help, but in recent versions of swift this caused a different strange compiler error about the computed property having the `wrong linkage`. While I don't really understand what is happening here enough to say whether this is a compiler bug or not, this only seems to be triggering from `_copyBytes` and `_withVeryUnsafeMutableBytes` still. We want these to be inlinable, so the next natural workaround seems to be to plumb the capacity references in by argument instead of referencing `self`. This makes the callsites a bit wordy, but it seems to work.

3405691582 added 3 commits April 5, 2025 14:21

Remove capacity tests for OpenBSD.

b95284b

As in the prior commit, ManagedBuffer.capacity is unavailable, therefore to get the capacity here, we need to inspect the header, which is marked as internal.

Lukasa requested changes Apr 9, 2025

View reviewed changes

3405691582 force-pushed the openbsd branch from 87dc1d6 to 3ac9276 Compare April 9, 2025 20:59

Lukasa reviewed Apr 10, 2025

View reviewed changes

3405691582 force-pushed the openbsd branch from c8a0325 to c5eaa36 Compare April 10, 2025 11:15

3405691582 added 2 commits April 12, 2025 10:04

Use a computed property instead of special-casing.

81cef4f

It may be slightly more performant to special-case but select for readability here.

Add conditional for RandomBytes.

e7dd233

Required for tests to build on OpenBSD.

3405691582 force-pushed the openbsd branch from c5eaa36 to e7dd233 Compare April 12, 2025 14:10

Lukasa added the 🆕 semver/minor Adds new public API. label Apr 17, 2025

Merge branch 'main' into openbsd

739d63c

Lukasa approved these changes Apr 17, 2025

View reviewed changes

Lukasa merged commit 0411996 into apple:main Apr 17, 2025
32 checks passed

3405691582 mentioned this pull request Apr 18, 2025

Fix cmake build. #352

Open

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

OpenBSD support. #349

OpenBSD support. #349

Uh oh!

3405691582 commented Apr 5, 2025 •

edited

Loading

Uh oh!

Lukasa Apr 9, 2025

Uh oh!

3405691582 Apr 9, 2025

Uh oh!

Uh oh!

Uh oh!

Lukasa Apr 10, 2025

Uh oh!

3405691582 Apr 10, 2025

Uh oh!

Lukasa Apr 10, 2025

Uh oh!

3405691582 Apr 10, 2025

Uh oh!

Lukasa Apr 11, 2025

Uh oh!

3405691582 Apr 11, 2025

Uh oh!

Lukasa Apr 11, 2025

Uh oh!

3405691582 Apr 12, 2025

Uh oh!

Lukasa left a comment

Uh oh!

Uh oh!

3405691582 commented Apr 18, 2025

Uh oh!

Uh oh!

OpenBSD support. #349

OpenBSD support. #349

Uh oh!

Conversation

3405691582 commented Apr 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

If you've made changes to gyb files

Motivation:

Modifications:

Result:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Lukasa left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

3405691582 commented Apr 18, 2025

Uh oh!

Uh oh!

3405691582 commented Apr 5, 2025 •

edited

Loading

If you've made changes to `gyb` files