-
Notifications
You must be signed in to change notification settings - Fork 1
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Fixes #1
- Loading branch information
Showing
2 changed files
with
2 additions
and
2 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
8f0e3b4
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 confirm that this change is working in Chromium, where we're linking the rust rlibs with C++ with lld.
It appears that this did not work in the tests you have in this repo though? Does this perhaps need to be configurable depending on the linker choice?
I wonder if the tests there would pass with
-Clinker=clang
or-Clinker=clang -Clink-arg=-fuse-ld=lld
8f0e3b4
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.
What relocation model is used when compiling the tests?
https://doc.rust-lang.org/rustc/codegen-options/index.html#relocation-model
I see that when relocation model is static for MachO, LLVM expects to put them in a different place:
https://github.com/llvm/llvm-project/blob/4b3eaee2701afa2549c9cd0f78692598e9bfd44e/llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp#L1191-L1203
FWIW, looking at otool I can see that clang puts C++ static initializers in __DATA_CONST,__mod_term_func, which is not strictly what the code above says, so that must end up modified somehow.
8f0e3b4
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.
We are investigating more here: https://bugs.chromium.org/p/chromium/issues/detail?id=1466674
8f0e3b4
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 will look into this now. For some reason when I ran this locally on my machine it passed but apparently it's now failing there too, even for the most basic test of compiling and running the example.