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

Add a way to generate a list of symbols for dynamic linkage #916

Merged
merged 1 commit into from
Jan 15, 2025

Conversation

TheElectronWill
Copy link
Contributor

@TheElectronWill TheElectronWill commented Jan 8, 2024

This resolves #907 by adding the ability to generate the list of exportable symbols (i.e. functions and globals), in a format that the linker (ld) can understand. Like depfile, this new feature is available both as a CLI option (--symfile) and as a method in Bindings. Therefore, it's easy to use it in a build script.

Most of the new files are tests on the output of the new option --symfile.

@TheElectronWill TheElectronWill changed the title Add a way to generate a list of symbols for dynamic linkage, resolves… Add a way to generate a list of symbols for dynamic linkage Jan 8, 2024
Copy link
Contributor

@jschwe jschwe left a comment

Choose a reason for hiding this comment

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

Nice - I might need exactly something like this in a project of mine in the next few months.

in a format that the linker (ld) can understand.

Which linkers specifically support this option? Is the --dynamic-list option of the gold linker compatible with the format, or did you only test with ldd? Is there any documentation on the exact format?

src/bindgen/bindings.rs Outdated Show resolved Hide resolved
@TheElectronWill
Copy link
Contributor Author

TheElectronWill commented Jan 9, 2024

Which linkers specifically support this option? Is the --dynamic-list option of the gold linker compatible with the format, or did you only test with ldd? Is there any documentation on the exact format?

The manual doesn't clearly explain the format. Gnu linker's manual reads "The format of the dynamic list is the same as the version node without scope and node name. See VERSION", and that "version" thing is introduced here.

I would expect at least GNU ld, lld and gold to work with that (I have only tested ld for the moment, but I'm confident that it works with lld too since it's been implemented with the same format in 2016). It would be weird for a linker to offer the same option but with a different format IMO.

@TheElectronWill
Copy link
Contributor Author

rebased

Copy link
Collaborator

@emilio emilio left a comment

Choose a reason for hiding this comment

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

Thanks, this looks good. Probably needs a rebase (can get to it if you can't) and I have one suggestion to avoid a couple intermediary allocations. But lmk if it's unreasonable.

src/bindgen/bindings.rs Outdated Show resolved Hide resolved
src/bindgen/bindings.rs Outdated Show resolved Hide resolved
@TheElectronWill TheElectronWill force-pushed the symbols-files branch 2 times, most recently from f961663 to c604d2e Compare September 20, 2024 12:09
@TheElectronWill
Copy link
Contributor Author

Sorry for the (very) late reply, it's been a busy year! I've rebased the branch and applied your suggestions.

@TheElectronWill TheElectronWill requested a review from emilio October 6, 2024 17:35
@TheElectronWill
Copy link
Contributor Author

@emilio Is there something else that should be modified? This feature would be great for alumet-dev/alumet#58. Thanks for the review :)

Copy link
Collaborator

@emilio emilio left a comment

Choose a reason for hiding this comment

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

No, looks good!

@emilio emilio added this pull request to the merge queue Jan 15, 2025
Merged via the queue into mozilla:master with commit 87afbf9 Jan 15, 2025
0 of 3 checks passed
@emilio
Copy link
Collaborator

emilio commented Jan 15, 2025

Ugh, the merge queue is... not blocking on things passing, that really sucks. I'll fix up the CI tho, I don't think I'll need to revert this.

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.

Allow access to bindings symbols names
3 participants