Skip to content

[5.9] IRGen: alloc_global and global_addr instructions need to agree on the storage #66560

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

aschwaighofer
Copy link
Contributor

@aschwaighofer aschwaighofer commented Jun 12, 2023

The ABI used for some typed globals is similar to existential values. We reserve an "inline" 3 word buffer, query type metadata's size, and if the size is bigger allocate storage on the heap.

The global_addr instruction needs to follow this logic: If the storage is opaque we need to project to the underlying buffer.

Risk: Low. The change should only affect opaque types (some types).

Without the change any some typed globals bigger than 3 words might clobber storage in memory immediately after it.

e.g. the following code is broken

public protocol Some {}

 public struct Implementer : Some {
   var w = (0, 1, 2, 3, 4)

   public init() { }
 }

 let g : some Some = Implementer()

Original PR: #66507
rdar://109636344

…on the storage

The ABI used for `some` typed globals is similar to existential values.
We reserve an "inline" 3 word buffer, query type metadata's size, and if
the size is bigger allocate storage on the heap.

The `global_addr` instruction needs to follow this logic: If the storage is
opaque we need to project to the underlying buffer.

Risk: Low. The change should only affect opaque types (`some` types).

Without the change any `some` typed globals bigger than 3 words might
clobber storage in memory immediately after it.

rdar://109636344
@aschwaighofer aschwaighofer requested a review from a team as a code owner June 12, 2023 16:39
@aschwaighofer
Copy link
Contributor Author

@swift-ci test

@aschwaighofer aschwaighofer merged commit bfef7f3 into swiftlang:release/5.9 Jun 13, 2023
nate-chandler added a commit to nate-chandler/swift that referenced this pull request Aug 18, 2023
In swiftlang#66560 , a bug in the lowering of
`global_addr` was fixed.  Part of that fix was to postpone mapping the
type of the global into context until getting the address of the global
and projecting the buffer for the out-of-line value; at that point, the
type is mapped into context and the address is cast.

It introduced an issue for fixed-size globals, however: the type of such
globals was not mapped into context; the result was that the lowered
value set for the corresponding SIL value would have the wrong type.

Fix that by extracting the code which mapped the type into context and
cast the address to the appropriate lowered type into a lambda and call
that lambda both in both the fixed-size (newly) and non-fixed-size (as
before) cases.

rdar://114013709
nate-chandler added a commit to nate-chandler/swift that referenced this pull request Aug 18, 2023
In swiftlang#66560 , a bug in the lowering of
`global_addr` was fixed.  Part of that fix was to postpone mapping the
type of the global into context until getting the address of the global
and projecting the buffer for the out-of-line value; at that point, the
type is mapped into context and the address is cast.

It introduced an issue for fixed-size globals, however: the type of such
globals was not mapped into context; the result was that the lowered
value set for the corresponding SIL value would have the wrong type.

Fix that by extracting the code which mapped the type into context and
cast the address to the appropriate lowered type into a lambda and call
that lambda both in both the fixed-size (newly) and non-fixed-size (as
before) cases.

rdar://114013709
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants