You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Object files can only be linked with legacy clang linker on Apple #8730
Hello, thank you very much for all of the incredible software y'all are writing. I'm currently using Cranelift to generate native binaries for my project, and I'm continually impressed at the level of detail and effort that goes into these things. Not to mention the super helpful Zulip chat.
Using the legacy linker, that is, adding -Wl -ld_classic to the clang invocation, does work. However, as the legacy linker will likely be going away someday, I'm filing this issue in the hopes that we can figure out why it doesn't work with the current linker. Note that cg_clif also uses -Wl -ld_classic, so when the legacy linker is dropped, cg_clif could also have issues.
So it appears that cranelift isn't generating the correct relocations for ARM64 on macOS. I'll let those who know more about this stuff fix it properly.
Reacted by Alex Hansen, EdorianDark, Yerkebulan Tulibergenov, Amos Wenger and Samuel Guyah
with the LLVM backend. If I try to link it I get a linker error as expected:
Undefined symbols for architecture arm64:
"_FOO", referenced from:
rust_out::foo::he5a9b1ed9cb33bc0 in rust_out.o
"_bar", referenced from:
rust_out::foo::he5a9b1ed9cb33bc0 in rust_out.o
rust_out::foo::he5a9b1ed9cb33bc0 in rust_out.o
"_main", referenced from:
<initial-undefines>
ld: symbol(s) not found for architecture arm64
This means there has to be a difference somewhere. If I run bingrep on both binaries, the relocations are shown in opposite order for both executable. For LLVM:
What would be the best place to add this relocation reversing code? object or cranelift-object? On the one hand it is something that would affect other projects too, so object makes more sense, on the other hand reversing the relocation order right before writing would require anyone who wants to round trip an object file through the object crate to reverse relocation order again before the add_relocation calls to produce a working object file.
LLVM does the reverse in its object writer. I'm not certain which place is best, for the same reasons you give, but I'm inclined to fix it in object, since its primary use is for generating objects, not for round trips. It already does at least one other change that round trips need to account for (symbol name decorations).
I'm now getting ld: warning: no platform load command found in '/path/to/cranelift/output.o', assuming: macOS. This happens only with the default modern linker, but not the legacy linker. The executable still works, but it's unfortunate to get this warning. It seems like Julia encountered a similar issue and fixed it by including the macOS version number in the OS segment of the triple: JuliaLang/julia#51830. I'm using target_lexicon::Triple::host, so it could be an issue with that crate rather than cranelift?
It's unclear to me if cranelift-object should be exposing some function to set the macho build version or if it should handle it automatically. Do we have enough information in cranelift-object to target the exact version?
Will need some work in target_lexicon to better support iOS/tvOS/watchOS/visionOS here too.
I think you might need to error if the version isn't present in the given triple?
The system tooling (linker, assembler, otool etc.) requires a version here.
At the very least don't emit the LC_BUILD_VERSION command in this case, and warn very strongly.
sdk (SDK version) to 0. This is also what LLVM does by default if no SDK version is given, and the tooling shows n/a in that case.
Note that this will not support older tooling that uses LC_VERSION_MIN_IPHONEOS, LC_VERSION_MIN_MACOSX etc. (we're talking somewhere around Xcode 9 or 10), but that's probably fine, we still support older OS versions the linker is going to patch it up such that you can still run the binaries on older OS versions.
It's unclear to me if cranelift-object should be exposing some function to set the macho build version or if it should handle it automatically. Do we have enough information in cranelift-object to target the exact version?
So to answer this directly, Cranelift should have enough information to set this if it requires the deployment target / minimum OS version to be set in the target triple (e.g. that users pass aarch64-apple-macosx12.0.0, and not just aarch64-apple-macosx).
Cranelift uses target-lexicon for target triples, which aims to math rustc. Rustc target triples do not allow passing in the OS version. Instead rustc has a default for each target and additionally allows setting the same env vars that clang reads. There is no way currently to pass this information to Cranelift.
Cranelift uses target-lexicon for target triples, which aims to math rustc. Rustc target triples do not allow passing in the OS version. Instead rustc has a default for each target and additionally allows setting the same env vars that clang reads. There is no way currently to pass this information to Cranelift.
Yes, but as it turns out target-lexicon is just lenient with parsing to parse most LLVM triples fine too. I'm pretty sure it would parse aarch64-apple-macosx12.0.0 as having the OS field set to macosx12.0.1, which is not an OS known to rustc or target-lexicon and thus would fail parsing.
Yes, but as it turns out target-lexicon is just lenient with parsing to parse most LLVM triples fine too. I'm pretty sure it would parse aarch64-apple-macosx12.0.0 as having the OS field set to macosx12.0.1, which is not an OS known to rustc or target-lexicon and thus would fail parsing.
In any case, I have opened bytecodealliance/target-lexicon#111 for figuring out how to resolve the rustc/LLVM discrepancy in target-lexicon.
But if it is as you say, that Cranelift wants to accept rustc target triples, then it needs some way to specify the deployment target on Apple platforms too.
A simple way to do this would be to add ObjectBuilder::apple_deployment_target(&mut self, major: u16, minor: u8, patch: u8) -> &mut Self, but I'd argue that it might be worthwhile to add such a setter to the Triple itself directly? At least, if Cranelift wants to at some point do an optimization based on the deployment target (which LLVM does all the time, though mostly for Objective-C code), then it'd be nice to have available earlier in the compilation pipeline.
Activity
philipc commentedon Jun 2, 2024
I've reproduced the issue with arm64. If I change to use x86_64 it works with either linker.
The linker error is:
clang generates relocations like this:
cranelift generates:
If I hack cranelift to generate this:
then the linker error changes to:
If I hack cranelift to generate this:
then it links and runs successfully.
Complete hack:
So it appears that cranelift isn't generating the correct relocations for ARM64 on macOS. I'll let those who know more about this stuff fix it properly.
bjorn3 commentedon Jun 27, 2024
A simple
produces
with the LLVM backend. If I try to link it I get a linker error as expected:
If I apply the following patch to Cranelift:
Relocations that print identical by
objdump -drare generated by Cranelift:Yet the object file produced by LLVM gives a regular linker error due to undefined symbols, while the one produced by Cranelift crashes the linker:
This means there has to be a difference somewhere. If I run bingrep on both binaries, the relocations are shown in opposite order for both executable. For LLVM:
and for Cranelift:
As hack I tried to reverse the order of relocations emitted by Cranelift too, which worked.
bjorn3 commentedon Jun 27, 2024
Looks like the change to cranelift/codegen/src/isa/aarch64/abi.rs isn't even necessary at all. Only the relocation order reversing is required.
philipc commentedon Jun 27, 2024
That sounds better. I was comparing clang output instead of looking at what rustc with llvm backend did, so you can probably ignore my results.
bjorn3 commentedon Jun 27, 2024
What would be the best place to add this relocation reversing code? object or cranelift-object? On the one hand it is something that would affect other projects too, so object makes more sense, on the other hand reversing the relocation order right before writing would require anyone who wants to round trip an object file through the object crate to reverse relocation order again before the
add_relocationcalls to produce a working object file.philipc commentedon Jun 27, 2024
LLVM does the reverse in its object writer. I'm not certain which place is best, for the same reasons you give, but I'm inclined to fix it in
object, since its primary use is for generating objects, not for round trips. It already does at least one other change that round trips need to account for (symbol name decorations).bjorn3 commentedon Jun 27, 2024
Opened gimli-rs/object#702
bjorn3 commentedon Jun 29, 2024
This has been fixed in object 0.36.1.
sezna commentedon Jun 29, 2024
Thank you all who looked into this!
bjorn3 commentedon Jun 29, 2024
This issue can be closed now, right?
camelid commentedon Jun 29, 2024
I'm now getting
ld: warning: no platform load command found in '/path/to/cranelift/output.o', assuming: macOS. This happens only with the default modern linker, but not the legacy linker. The executable still works, but it's unfortunate to get this warning. It seems like Julia encountered a similar issue and fixed it by including the macOS version number in the OS segment of the triple: JuliaLang/julia#51830. I'm usingtarget_lexicon::Triple::host, so it could be an issue with that crate rather than cranelift?bjorn3 commentedon Jun 29, 2024
We will need to call set_macho_build_version in cranelift-object to emit the LC_BUILD_VERSION load command.
camelid commentedon Jun 30, 2024
It seems like rustc has implemented a fix for the same issue: rust-lang/rust#111384.
camelid commentedon Jun 30, 2024
It's unclear to me if cranelift-object should be exposing some function to set the macho build version or if it should handle it automatically. Do we have enough information in cranelift-object to target the exact version?
madsmtm commentedon Sep 28, 2024
I've looked into this recently, see rust-lang/rust#129432 for the broader Rust issue of deployment targets and SDKs.
I'm pretty sure the correct behaviour for Cranelift would be to set the values on
MachOBuildVersionas follows:platform, depending on the target triple'sOperatingSystemandEnvironment.minos(deployment target), extracted from target_lexicon::OperatingSystem::MacOSX.target_lexiconto better support iOS/tvOS/watchOS/visionOS here too.otooletc.) requires a version here.LC_BUILD_VERSIONcommand in this case, and warn very strongly.sdk(SDK version) to0. This is also what LLVM does by default if no SDK version is given, and the tooling showsn/ain that case.Note that this will not support older tooling that uses
LC_VERSION_MIN_IPHONEOS,LC_VERSION_MIN_MACOSXetc. (we're talking somewhere around Xcode 9 or 10), but that's probably fine, we still support older OS versions the linker is going to patch it up such that you can still run the binaries on older OS versions.madsmtm commentedon Sep 28, 2024
So to answer this directly, Cranelift should have enough information to set this if it requires the deployment target / minimum OS version to be set in the target triple (e.g. that users pass
aarch64-apple-macosx12.0.0, and not justaarch64-apple-macosx).bjorn3 commentedon Sep 28, 2024
Cranelift uses target-lexicon for target triples, which aims to math rustc. Rustc target triples do not allow passing in the OS version. Instead rustc has a default for each target and additionally allows setting the same env vars that clang reads. There is no way currently to pass this information to Cranelift.
madsmtm commentedon Sep 28, 2024
Hmm, isn't
rustcpassing the LLVM target to Cranelift? https://github.com/rust-lang/rust/blob/612796c42077605fdd3c6f7dda05745d8f4dc4d8/compiler/rustc_codegen_cranelift/src/lib.rs#L262bjorn3 commentedon Sep 28, 2024
Yes, but as it turns out target-lexicon is just lenient with parsing to parse most LLVM triples fine too. I'm pretty sure it would parse aarch64-apple-macosx12.0.0 as having the OS field set to macosx12.0.1, which is not an OS known to rustc or target-lexicon and thus would fail parsing.
madsmtm commentedon Sep 29, 2024
Nope, they do parse the version number, see https://github.com/bytecodealliance/target-lexicon/blob/7c80d459a9fdd121e9f23feb680c3db13c1baa39/src/targets.rs#L1393-L1421.
In any case, I have opened bytecodealliance/target-lexicon#111 for figuring out how to resolve the
rustc/LLVM discrepancy intarget-lexicon.But if it is as you say, that Cranelift wants to accept
rustctarget triples, then it needs some way to specify the deployment target on Apple platforms too.A simple way to do this would be to add
ObjectBuilder::apple_deployment_target(&mut self, major: u16, minor: u8, patch: u8) -> &mut Self, but I'd argue that it might be worthwhile to add such a setter to theTripleitself directly? At least, if Cranelift wants to at some point do an optimization based on the deployment target (which LLVM does all the time, though mostly for Objective-C code), then it'd be nice to have available earlier in the compilation pipeline.bjorn3 commentedon Dec 11, 2024
We are using target-lexicon 0.13 so we should now be able to call
set_macho_build_versionin cranelift-object and update https://github.com/rust-lang/rustc_codegen_cranelift/blob/3ebaf047071afae07229ff8fb5e05527fa6d9645/src/lib.rs#L247 to fix this issue.