Generate native gems with -fvisibility=hidden
#6541
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
casperisfine
force-pushed
the
c-exts-hide-symbols
branch
from
25005a9
to
b40f120
Compare
|
I seem to recall a few gems that compile vendored code, then use ffi to access it. Will they be affected by this? |
deivid-rodriguez
approved these changes
Mar 24, 2023
|
@MSP-Greg This is just the boilerplate code created by |
|
@deivid-rodriguez @casperisfine
Duh. This isn't mkmf. Sorry, not enough coffee yet... |
simi
reviewed
Mar 24, 2023
simi
approved these changes
Mar 24, 2023
casperisfine
force-pushed
the
c-exts-hide-symbols
branch
from
b40f120
to
075391e
Compare
simi
reviewed
Mar 24, 2023
bundler/lib/bundler/templates/newgem/ext/newgem/extconf-c.rb.tt
Outdated
Show resolved
Hide resolved
I recently ran into very nasty issues with dynamic symbols clashing between two native gems. I believe the overwhelming majority of native gems don't want to export their symbols, so hidding them by default would make sense to me.
auto-merge was automatically disabled
March 24, 2023 20:13
Head branch was pushed to by a user without write access
casperisfine
force-pushed
the
c-exts-hide-symbols
branch
from
075391e
to
449624a
Compare
To be fair, I think we should make this change in mkmf, we'll probably bring this up on ruby-lang at some point, but I thought bundler's templates was a good start. |
nobu
reviewed
Mar 26, 2023
| # Makes all symbols private by default to avoid unintended conflict | ||
| # with other gems. To explicitly export symbols you can use RUBY_FUNC_EXPORTED | ||
| # selectively, or entirely remove this flag. | ||
| $CFLAGS << " -fvisibility=hidden " |
There was a problem hiding this comment.
Of course this option is compiler dependent.
Should be append_cflags("-fvisibility=hidden").
4 tasks
hsbt
added a commit
that referenced
this pull request
Mar 28, 2023
Partly reverted rubocop rule at GH-6541
matzbot
pushed a commit
to ruby/ruby
that referenced
this pull request
Mar 28, 2023
deivid-rodriguez
pushed a commit
that referenced
this pull request
Apr 10, 2023
Generate native gems with `-fvisibility=hidden` (cherry picked from commit 7305d11)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I recently ran into very nasty issues with dynamic symbols clashing between two native gems.
I believe the overwhelming majority of native gems don't want to export their symbols, so hiding them by default would make sense to me.