Skip to content
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

Build a .drectve section in COFF objects with export directives #423

Merged
merged 1 commit into from
Jan 17, 2022

Conversation

Amanieu
Copy link
Contributor

@Amanieu Amanieu commented Jan 14, 2022

This is needed to export symbols with SymbolScope::Dynamic from a DLL.

@Amanieu
Copy link
Contributor Author

Amanieu commented Jan 14, 2022

I'm not super happy about this, but I'd like some initial feedback first.

Some issues:

  • This works poorly if there already is a .drectve section provided by the user.
  • The format of the directives is different for GNU (-export:FOO) and MSVC (/EXPORT:FOO). LLD supports both formats. Currently this PR only generates the MSVC format.

@philipc
Copy link
Contributor

philipc commented Jan 15, 2022

This works poorly if there already is a .drectve section provided by the user.

I think this is okay. If the user needs to provide their own linker options, then we should extend the API to support that in a general way for ELF (SHT_LLVM_LINKER_OPTIONS extension) and Mach-O (LC_LINKER_OPTION) too.

The format of the directives is different for GNU (-export:FOO) and MSVC (/EXPORT:FOO). LLD supports both formats. Currently this PR only generates the MSVC format.

We'll need to add more configuration settings if we need to differentiate. Using MSVC is correct for now.

@philipc
Copy link
Contributor

philipc commented Jan 15, 2022

https://docs.microsoft.com/en-us/windows/win32/debug/pe-format#the-drectve-section-object-only talks about a UTF-8 marker. Do we need to emit it? I couldn't see anywhere that LLVM adds this.

@bjorn3
Copy link
Contributor

bjorn3 commented Jan 15, 2022

The format of the directives is different for GNU (-export:FOO) and MSVC (/EXPORT:FOO). LLD supports both formats. Currently this PR only generates the MSVC format.

We'll need to add more configuration settings if we need to differentiate. Using MSVC is correct for now.

MinGW is currently the only windows environment for which cg_clif works. I would prefer if it doesn't break.

@philipc
Copy link
Contributor

philipc commented Jan 15, 2022

How does cg_clif currently handle SymbolScope::Dynamic?

@bjorn3
Copy link
Contributor

bjorn3 commented Jan 15, 2022

Rustc knows which symbols need to be exported and then tells the linker about this when it runs it. In many cases not all symbols with SymbolScope::Dynamic are exported. When compiling the object files it isn't known in advance which symbols need to be exported. If the same rlib is linked into a cdylib most symbols aren't exported, only #[no_mangle]. If it is linked into a dylib all non-local symbols are exported.

@philipc
Copy link
Contributor

philipc commented Jan 15, 2022

That sounds like this is something we shouldn't be doing automatically. So the user needs to explicitly request it, and we may as well do that by creating or appending to a regular section when they request it.

@Amanieu
Copy link
Contributor Author

Amanieu commented Jan 15, 2022

So what would the API look like in that case?

@philipc
Copy link
Contributor

philipc commented Jan 16, 2022

Something like pub fn add_coff_msvc_exports(&mut self) on Object.

@Amanieu
Copy link
Contributor Author

Amanieu commented Jan 17, 2022

Done.

Copy link
Contributor

@philipc philipc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@philipc philipc merged commit eea6c45 into gimli-rs:master Jan 17, 2022
@Amanieu
Copy link
Contributor Author

Amanieu commented Jan 18, 2022

Thanks! Could you publish a new version of the object crate?

@philipc
Copy link
Contributor

philipc commented Jan 19, 2022

Released 0.28.3

mcbegamerxx954 pushed a commit to mcbegamerxx954/object that referenced this pull request Jun 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants