-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
[draft] Raw_dylib codegen #71497
[draft] Raw_dylib codegen #71497
Conversation
r? @estebank (rust_highfive has picked a reviewer for you, use r? to override) |
Your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem. Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
Your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem. Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
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.
Some smaller nits
Your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem. Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
aa847c1
to
92d86ab
Compare
Is this actually needed? AFAIK it will just select the functions by name?
See stack overflow question about it here. You need to declare a specially named constant and then do some computation in IR code. It's also inside the example of #30027. I don't think the computation can be done in the backend as when the IR code is being emitted the info is not available yet, but not an expert.
No idea, I'm sorry.
@ZerothLaw has offered help with this. Optimally, the test would create two dll files (written in Rust, using cdylib), both defining and exporting the same function returning a different i32 constant each. Then you'd specify one of the dlls in the test case and assert that the result matches the dll you named. |
Test case: mod foo1 {
#[link(kind="raw_dylib", name="foo1.dll")]
extern {
fn foo() -> i32;
}
}
mod foo2 {
#[link(kind="raw_dylib", name="foo2.dll")]
extern {
fn foo() -> i32;
}
}
fn main() {
unsafe {
assert_eq!(foo1::foo(), 100);
assert_eq!(foo2::foo(), 200);
}
} Variations include mixing |
i was under the assumption that examples like the one that @retep998 just posted cannot work unless we generate the stub that calls the correct dll as a mangled rust function |
@retep998 how will that be possible? I can't find hints for it to work either in LLVM IR nor in the PE format, but likely I'm missing something. What would the wrappers contain? How would they be able to differ between foo from foo1.dll and foo from foo2.dll? I can't find a method to link a function to the idata tables, thought they'd be linked by name only. |
I was under the impression that an idata entry could provide a mapping from a mangled name to an external name. There's already some of this going on for things like stdcall and cdecl on 32-bit where |
Actually you might be right. The import address table contains the needed data during runtime: the addresses of the loaded symbols. I have no idea how to reference it in the wrapper though. I wonder if EntryPoint of C#'s |
I'm wondering: are you maybe compiling for 64 bit? Because currently the implementation only supports 32 bit... |
rvas are always 32 bit, regardless of platform |
Point. The import lookup table has 64 bit entries on 64 bit but even there the RVAs are 31 bit. Also your code doesn't emit it yet so it can't be the cause of the assertion errors. |
☔ The latest upstream changes (presumably #72458) made this pull request unmergeable. Please resolve the merge conflicts. |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
@tinaun any progress on this? |
@tinaun what's the status of this? |
☔ The latest upstream changes (presumably #75120) made this pull request unmergeable. Please resolve the merge conflicts. |
Closing this due to inactivity |
for #58713
i don't quite know if this is how i should structure the code, but it seems to work
current progress
issues