Skip to content

Conversation

ellishg
Copy link
Contributor

@ellishg ellishg commented Oct 3, 2025

Emit ObjC strings to the __cstring section if willBeRelativelyAddressed is true.

#84300 recently emitted these special strings into their respective sections. However, I recently discovered an error when linking an x86_64 binary.

ld64.lld: error: a.o:(__objc_classname): offset is outside the section

I believe the issue is there is a relocation that has an offset (-4 in this case) outside of the referent section (__objc_classname). After looking at 638e4b0 and 50b5044, I suspect they are a similar issues.

When we relatively address them (which occurs in protocol conformance records), the linker may compute the relative offset before coalescing, leading to an incorrect result.

These cases already set willBeRelativelyAddressed, so if that's the case, also make sure they are emitted into the __cstring section.

@ellishg ellishg requested a review from rjmccall as a code owner October 3, 2025 01:28
Copy link
Contributor

@rjmccall rjmccall left a comment

Choose a reason for hiding this comment

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

We've definitely run into that linker bug before, which is why the flag exists, so I'm fine with this.

@ellishg
Copy link
Contributor Author

ellishg commented Oct 3, 2025

I've requested membership, but I don't have it yet. @rjmccall could you test and merge for me?

@rjmccall
Copy link
Contributor

rjmccall commented Oct 3, 2025

@swift-ci Please test

@ellishg
Copy link
Contributor Author

ellishg commented Oct 4, 2025

@swift-ci Please test macOS platform

@ellishg ellishg enabled auto-merge (squash) October 4, 2025 16:16
@ellishg ellishg force-pushed the fix-objc-cstr-sections branch from ba43f96 to de6e7f0 Compare October 4, 2025 21:29
@ellishg
Copy link
Contributor Author

ellishg commented Oct 4, 2025

@swift-ci Please test

@ellishg ellishg force-pushed the fix-objc-cstr-sections branch from de6e7f0 to 0a95e71 Compare October 5, 2025 17:15
@ellishg
Copy link
Contributor Author

ellishg commented Oct 5, 2025

@swift-ci Please test

@ellishg
Copy link
Contributor Author

ellishg commented Oct 6, 2025

import-as-instance-method.swift keeps failing for macOS, but it passes locally for me. It doesn't seem related to me. @rjmccall is there a way to merge anyway? Or should I keep trying to rebase?

@rjmccall
Copy link
Contributor

rjmccall commented Oct 6, 2025

@ellishg There's a PR up to fix it, #84708. We just need to wait until it's merged and then re-test.

@hamishknight
Copy link
Contributor

@swift-ci please test macOS

@hamishknight
Copy link
Contributor

@swift-ci please test Windows

@ellishg
Copy link
Contributor Author

ellishg commented Oct 6, 2025

@swift-ci please test macOS

Oops, I thought I had to rebase and then retest. I'll trigger the tests again

@ellishg
Copy link
Contributor Author

ellishg commented Oct 6, 2025

@swift-ci Please test

@ellishg
Copy link
Contributor Author

ellishg commented Oct 7, 2025

@swift-ci please test Windows

@ellishg ellishg merged commit 4f25495 into swiftlang:main Oct 7, 2025
5 checks passed
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