Skip to content

IRGen: do not mark AFP as const #40292

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
merged 1 commit into from
Dec 5, 2021
Merged

Conversation

compnerd
Copy link
Member

@compnerd compnerd commented Nov 24, 2021

We were not marking the async function pointer as constant but adding it
to the .rdata section which the Windows linker helped identify as a
problem: we were adding data with "rw" to a section that is expected to
be "rd". Marking the data as constant results in error propagation
breaking (presumably due to failure to materialize parameters properly
as bindiff'ing the Concurrency library indicates that the functions are
nearly identical with some argument loads elided). This repairs the
emission of read/write data into the readonly section.

@compnerd
Copy link
Member Author

@swift-ci please test Windows platform

We were not marking the async function pointer as constant but adding it
to the `.rdata` section which the Windows linker helped identify as a
problem: we were adding data with "rw" to a section that is expected to
be "rd".  Marking the data as constant results in error propagation
breaking (presumably due to failure to materialize parameters properly
as bindiff'ing the Concurrency library indicates that the functions are
nearly identical with some argument loads elided).  This repairs the
emission of read/write data into the readonly section.
@compnerd compnerd changed the title IRGen: make setTrueConst mark the variable as constant IRGen: do not mark AFP as const Nov 25, 2021
@compnerd
Copy link
Member Author

@swift-ci please test

@compnerd
Copy link
Member Author

compnerd commented Nov 25, 2021

Okay, further investigation results: the problem is that marking the AFP as constant allows LLVM to peer through the structure. Then, when IRGen'ing it will inline all the context to 32-bytes unconditionally dropping the AFP checks even on the generic parameter. The failing tests fortunately helped here - they happened to have a larger context size (e.g. async_taskgroup_throw_recover has 48 bytes). However, because we only allocated 32-bytes for the task, we now get corruption.

@compnerd
Copy link
Member Author

CC: @aschwaighofer

@compnerd compnerd merged commit 19e2664 into swiftlang:main Dec 5, 2021
@compnerd compnerd deleted the constantinople branch December 5, 2021 18:18
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.

1 participant