-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[Autolink] Autolinking on COFF for Cygwin/MinGW #3887
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
Conversation
Please add the |
I still think that we should figure out why you can't use the existing linker support on these targets. LLVM can generate objects which conform to binutils' limitations. If this is a workaround, we should at least have a clear understanding of what it is working around and test for the case that shows what breaks down. |
(For other readers, some of this discussion is overlapped with #3841, because the compiler part of autolinking is there) The root cause is to We can confirm all the possible |
Ah, right, that was a local extension that I had toyed with. Id say that we want to base the decision on the linker |
|
Right, but the MinGW folks have been playing with lld for Windows, and are expecting to use it because I didn't want to upstream my patches for binutils to support Windows ARM :-). So, I imagine that they will at some point try to converge. Thats much less likely for cygwin, but in both scenarios, the check over the value of |
Is 'MinGW with lld for Windows' possible for MinGW applications which is liking with glibc? I'm waiting someone implement it. (llvm-dev: lld for MinGW) |
For the most part, yes, it is possible. The arguments just need to be mapped into the appropriate spelling. There are only a couple of misfeatures which aren't necessary for swift which I can see as being contentious. |
I now understand your opinion. I agree it is possible to use But we can not use the lld for autolinking. In MinGW, gcc/clang and |
|
Does swift project have a plan for covering its own linker? If we now put If you patched the The enhance-lld-or-ld-work will takes some time which is not dependent to the patch size, but rather dependent to llvm-lld development community and ld development community. If we success these (patch ld/lld and widely deployed), we can apply same thing to ELF format with proper section name, and the swift-autolink-extract can be obsoleted. |
8210762
to
0112e86
Compare
@gribozavr, I updated test related files. |
elif run_os == 'linux-gnu' or run_os == 'linux-gnueabihf' or run_os == 'freebsd': | ||
# Linux/FreeBSD | ||
if run_os == 'freebsd': | ||
elif run_os == 'linux-gnu' or run_os == 'linux-gnueabihf' or run_os == 'freebsd' or run_os == 'windows-cygnus' or run_os == 'windows-gnu': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you mind using a more compact form?
elif run_os in ['linux-gnu', 'linux-gnueabihf', 'freebsd', 'windows-cygnus', 'windows-gnu']:
@tinysun212 Thank you, this looks great! Only a few minor comments, and we should be good to go! Let's get a CI run anyway: @swift-ci Please test |
Cygwin and MinGW should use the autolink feature in the sameway of Linux due to the linker's limit. Now swift-autolink-extract recognizes the COFF format file for Cygwin/MinGW.
0112e86
to
0747503
Compare
@gribozavr, I applied your comments. |
@swift-ci Please test |
for (auto &Section : ObjectFile->sections()) { | ||
llvm::StringRef SectionName; | ||
Section.getName(SectionName); | ||
if (SectionName == ".swift1_autolink_entries") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't valid for COFF. COFF has a 8-character limit on section names.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I quoted the description of the name field in 3. Section Table (Section Headers)
from MS PECOFF.
... For longer names, this field contains a slash (/) that is followed by an ASCII
representation of a decimal number that is an offset into the string table.
Executable images do not use a string table and do not support section names longer
than 8 characters. Long names in object files are truncated if they are emitted to
an executable file.
Because the section '.swift1_autolink_entries' will not be emitted to an executable file, we can use the long name for autolink feature. But we can not use the long names '.swift2_protocol_conformances' and '.swift2_type_metadata' instead of '.sw2prtc' or '.sw2tymd', because they are emitted to an executable and loaded into the system memory.
@swift-ci Please test Linux platform |
Testing Linux manually. |
Tests on Linux passed. @tinysun212 Would you mind submitting the simplification around |
What's in this pull request?
Resolved bug number: (SR-1128)
Before merging this pull request to apple/swift repository:
Triggering Swift CI
The swift-ci is triggered by writing a comment on this PR addressed to the GitHub user @swift-ci. Different tests will run depending on the specific comment that you use. The currently available comments are:
Smoke Testing
A smoke test on macOS does the following:
device standard libraries are not built.
version of these tests are not run.
A smoke test on Linux does the following:
tests are not run.
Validation Testing
Lint Testing
Note: Only members of the Apple organization can trigger swift-ci.
Cygwin and MinGW should use the autolink feature in the sameway of Linux
due to the linker's limit. Now swift-autolink-extract recognizes the
COFF format file for Cygwin/MinGW.