Uh oh!
There was an error while loading. Please reload this page.
Generate synthetic object file to ensure all exported and used symbols participate in the linking - #95604
Conversation
petrochenkov
commented
Apr 2, 2022
One reason why I like this approach more than #95363 is that I have no idea how widely linker script files like A "root" object file, on another hand, is as portable as it gets, with the same logic being reusable for very different targets. |
This comment has been minimized.
This comment has been minimized.
carbotaniuman
commented
Apr 3, 2022
Yeah this seems like a cleaner approach. This should fix the This doesn't fix #47384 fully however as it still requires |
nbdd0121
commented
Apr 3, 2022
What do you mean? This certainly fixes the issue described by the OP in #47384. |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
carbotaniuman
commented
Apr 3, 2022
nbdd0121
commented
Apr 3, 2022
Ah, the rust-audit case does require a custom linker script or |
This comment has been minimized.
This comment has been minimized.
#47384 discusses so many different things that I'd like to set some agreements on what specifically this PR is supposed to assume and fix.
|
petrochenkov
commented
Apr 9, 2022
@nbdd0121
I'll be able to approve the first PR almost immediately, and it will then unblock #95818. |
nbdd0121
commented
Apr 9, 2022
No. This PR does not change the meaning of
No. Neither of these are exported. The export symbol list is not changed at all.
See above. |
petrochenkov
commented
Apr 9, 2022
As I understand the |
nbdd0121
commented
Apr 9, 2022
The |
nbdd0121
commented
Apr 9, 2022
There is an "exported_symbols" query and a "exported_symbols" function in linker.rs that produces the final exported symbols. They are actually different things; the first contains all reachables, latter does a filter to the former to get an actual list for export. |
petrochenkov
commented
Apr 9, 2022
Yes, but they shouldn't have any exported level at all (unless they are exported for other reasons unrelated to
The list of symbols to filter in |
nbdd0121
commented
Apr 9, 2022
"Rust" level export will be exported in I don't see why #[used]staticA:u32 = 1;shouldn't be exported while staticA:u32 = 1;#[inline]pubfnfoo() -> &u32{&A}should. |
nbdd0121
commented
Apr 9, 2022
In a sense I don't treat "Rust" export level as truly being exports; it's basically just an export level for "everything that might get used" and I view it as an implementation detail to make dylibs work. |
Ok, point understood. I agree that in practice it's not too important to prune such used symbols from dylib exports, and we can skip it if it helps to simplify the implementation or fix other more important issues sooner. I'd still consider it as a rough edge of the implementation, because when spec / mental model say one thing and the implementation do another it often leads to misunderstanding and mess. |
petrochenkov
commented
Apr 9, 2022
The issue 1 ( |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
petrochenkov
commented
Apr 9, 2022
Otherwise LGTM, no need to split into two PRs (#95604 (comment)) anymore. |
nbdd0121
commented
Apr 26, 2022
I left it empty because it isn't currently needed. |
Fix#50007 and #47384
This is the synthetic object file approach that I described in #95363 (comment), allowing all exported and used symbols to be linked while still allowing them to be GCed.
Related #93791, #95363
r? @petrochenkov
cc @carbotaniuman