Skip to content

Conversation

Xazax-hun
Copy link
Contributor

@Xazax-hun Xazax-hun commented Sep 16, 2025

Large trivial types were copied via memcpy instead of doing a field-wise copy. This is incorrect for types with reference fields where we also need to bump the corresponding refcounts.

rdar://160315343

@Xazax-hun Xazax-hun added the c++ interop Feature: Interoperability with C++ label Sep 16, 2025
Copy link
Contributor

@egorzhdan egorzhdan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@rjmccall
Copy link
Contributor

We're not modeling these types as POD in any way in the Swift compiler, right? You're saying "POD" because C++ says they're POD, but we're modeling them as non-trivial types?

@Xazax-hun Xazax-hun force-pushed the unbalanced-refcount-in-pods branch from 1823a1e to 75ec1fd Compare September 17, 2025 07:42
@Xazax-hun Xazax-hun changed the title [cxx-interop] Fix over-releasing foreign reference members of PODs [cxx-interop] Fix over-releasing reference members of trival C++ types Sep 17, 2025
@Xazax-hun
Copy link
Contributor Author

Indeed, I updated the PR title and the comments of the code to not confuse POD types in C++ with trivial types in Swift.

@Xazax-hun Xazax-hun force-pushed the unbalanced-refcount-in-pods branch from 75ec1fd to 965f15f Compare September 17, 2025 14:11
@Xazax-hun Xazax-hun requested a review from a team as a code owner September 19, 2025 08:35
@Xazax-hun Xazax-hun force-pushed the unbalanced-refcount-in-pods branch from b467982 to 502646a Compare September 19, 2025 10:29
Large trivial types were copied via memcpy instead of doing a field-wise
copy. This is incorrect for types with reference fields where we also
need to bump the corresponding refcounts.

rdar://160315343
@Xazax-hun Xazax-hun force-pushed the unbalanced-refcount-in-pods branch from 502646a to e4e5423 Compare September 19, 2025 13:28
@Xazax-hun
Copy link
Contributor Author

@swift-ci please test

@Xazax-hun Xazax-hun enabled auto-merge September 19, 2025 16:30
@Xazax-hun Xazax-hun merged commit ac26e28 into swiftlang:main Sep 19, 2025
5 checks passed
Xazax-hun added a commit to Xazax-hun/swift that referenced this pull request Sep 22, 2025
… types

Explanation: Large trivial types were copied via memcpy instead of doing a
field-wise copy. This is incorrect for types with reference fields where we also
need to bump the corresponding refcounts. This PR makes sure that trival
C++ types with reference members are not trivial in Swift.
Issues: rdar://160315343
Original PRs: swiftlang#84321
Risk: There is a low chance of breaking source compatibility as some
types that used to be BitwiseCopyable are no longer considered as such.
However, code relying on that probably has latent memory safety bugs.
Testing: Added a compiler test.
Reviewers: @egorzhdan, @j-hui
Xazax-hun added a commit to Xazax-hun/swift that referenced this pull request Sep 22, 2025
… types

Explanation: Large trivial types were copied via memcpy instead of doing a
field-wise copy. This is incorrect for types with reference fields where we also
need to bump the corresponding refcounts. This PR makes sure that trival
C++ types with reference members are not trivial in Swift.
Issues: rdar://160315343
Original PRs: swiftlang#84321
Risk: There is a low chance of breaking source compatibility as some
types that used to be BitwiseCopyable are no longer considered as such.
However, code relying on that probably has latent memory safety bugs.
Testing: Added a compiler test.
Reviewers: @egorzhdan, @j-hui
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ interop Feature: Interoperability with C++
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants