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

Allow BDWGC to be linked dynamically #112

Merged
merged 1 commit into from
Mar 4, 2024

Conversation

jacob-hughes
Copy link
Collaborator

This is useful for debugging, where profilers intercept and override dynamically linked symbols (e.g. malloc and free).

As of this commit, there is no way to build a custom BDWGC fork and have that dynamically link to a program compiled with Alloy automatically. This is because cargo does not support bubbling up linker args to the final linked binary, so there is no way to programmatically set the rpath for BDWGC to the OUT directory in library/bdwgc 1.

Alloy programs must therefore set the LD_PRELOAD environment variable to prevent it from linking against the system libgc.

This is useful for debugging, where profilers intercept and override
dynamically linked symbols (e.g. `malloc` and `free`).

As of this commit, there is no way to build a custom BDWGC fork and have
that dynamically link to a program compiled with Alloy automatically.
This is because `cargo` does not support bubbling up linker args to the
final linked binary, so there is no way to programmatically set the
rpath for BDWGC to the OUT directory in `library/bdwgc` [1].

Alloy programs must therefore set the `LD_PRELOAD` environment variable
to prevent it from linking against the system libgc.

[1]: rust-lang/cargo#9554
@ltratt
Copy link
Member

ltratt commented Mar 4, 2024

Could we make use of rust-lang/cargo#9554 (comment) DEP_ so that an Alloy project could call a function of ours in its build.rs and we set the correct flag for it? Because LD_PRELOAD is really a big usability problem that we want to avoid if possible...

@jacob-hughes
Copy link
Collaborator Author

Because LD_PRELOAD is really a big usability problem that we want to avoid if possible...

I agree. This is really less than ideal. Fortunately though dynamic linking should never really be used unless you're trying to debug the collector.

Could we make use of rust-lang/cargo#9554 (comment) DEP_ so that an Alloy project could call a function of ours in its build.rs

The problem with this as I see it is that it must also be transitive, library/standard and library/alloc which re-export library/bdwgc must do it in their build.rs, and then the user program must supply a build.rs script that does it. To me this feels less user-friendly than just setting LD_PRELOAD.

I've even tried using __attribute__(weak)__ to remove dynamic linking altogether and instead try and allow profiling tools to intercept and override static symbols (see https://stackoverflow.com/questions/15966978/attribute-weak-and-ld-preload). Unfortunately this didn't seem to work.

Personally I think this is the least worst option.

@ltratt
Copy link
Member

ltratt commented Mar 4, 2024

OK let's revisit this at another point and see if we can improve things. This will have to do for now.

@ltratt ltratt added this pull request to the merge queue Mar 4, 2024
Merged via the queue into softdevteam:master with commit 5b00d25 Mar 4, 2024
2 checks passed
@ltratt
Copy link
Member

ltratt commented Mar 5, 2024

@jacob-hughes I think we should have added docs to the README for this change. Can we do a new PR for that please?

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.

2 participants