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.

Sign up for GitHub

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

Jump to bottom

Makefile: Add support for symbol versioning (ON by default) #49012

Merged
merged 6 commits into from
Jun 29, 2023
Merged

Makefile: Add support for symbol versioning (ON by default) #49012

merged 6 commits into from
Jun 29, 2023

Conversation

topolarity
Copy link
Member

@topolarity topolarity commented Mar 15, 2023 •
edited
Loading

This change enables symbol versioning by default and also adds SYMBOL_VERSION_SUFFIX as a Makefile option to allow appending a "salt" to the SONAME and all symbols' versions.

@topolarity topolarity force-pushed the sym-version branch 5 times, most recently from 5673534 to f6c8719 Compare March 15, 2023 20:22
@topolarity topolarity requested a review from staticfloat March 15, 2023 21:06
staticfloat
staticfloat previously requested changes Mar 15, 2023
View reviewed changes
Make.inc Outdated Show resolved Hide resolved
cli/Makefile Outdated Show resolved Hide resolved
@topolarity topolarity requested a review from staticfloat March 16, 2023 13:25
@topolarity topolarity force-pushed the sym-version branch 3 times, most recently from 90b843d to f89b147 Compare March 16, 2023 15:06
@ChrisRackauckas
Copy link
Member

Could we get a backport to v1.9.x here?

@staticfloat staticfloat added the backport 1.9 Change should be backported to release-1.9 label Mar 27, 2023
@topolarity
Copy link
Member Author

@staticfloat @gbaraldi What do you think about turning this on by default?

If we go ahead and do that, I'm guessing we'll want a separate change that we won't backport to 1.9?

vtjnash
vtjnash reviewed Mar 29, 2023
View reviewed changes
Copy link
Member

@vtjnash vtjnash left a comment

Choose a reason for hiding this comment

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

Can you simplify this PR to only have the version script change for the Makefile?

vtjnash
vtjnash reviewed Mar 29, 2023
View reviewed changes
cli/Makefile Outdated Show resolved Hide resolved
@topolarity
Copy link
Member Author

Can you simplify this PR to only have the version script change for the Makefile?

Sorry, I should have mentioned this in the OP, but f89b147 is the only commit specific to this PR.

The others are carried from the dependency on #49010

@KristofferC KristofferC mentioned this pull request Mar 30, 2023
Merged
52 tasks
@topolarity topolarity mentioned this pull request Mar 31, 2023
Merged
@KristofferC KristofferC mentioned this pull request Apr 9, 2023
Merged
26 tasks
@topolarity
Copy link
Member Author

Alright, I've added cli/julia.expmap and made the requested changes to #49010

This PR can now be reviewed independently 🙂

@topolarity topolarity requested a review from vtjnash April 11, 2023 17:46
@topolarity topolarity self-assigned this Apr 12, 2023
vtjnash
vtjnash reviewed Apr 18, 2023
View reviewed changes
cli/julia.expmap Outdated Show resolved Hide resolved
@KristofferC KristofferC mentioned this pull request May 8, 2023
Merged
51 tasks
@gbaraldi
Copy link
Member

Bump!

vtjnash
vtjnash reviewed May 10, 2023
View reviewed changes
Copy link
Member

@vtjnash vtjnash left a comment

Choose a reason for hiding this comment

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

LGTM. Can you make sure this is on-by-default, so that release builds have this available? I think you also need to specify -fno-gnu-unique, which disables a GNU extension to ELF as a feature to sometimes miscompile your C++ programs (https://bugs.llvm.org/show_bug.cgi?id=48221), and also check with @vchuravy that this won't break his fix in d8fa3c8. IIUC, that symbol may need to be tagged with the LLVM symbol version, not the Julia symbol version info.

@topolarity
Copy link
Member Author

Can you make sure this is on-by-default, so that release builds have this available?

@vtjnash @staticfloat Sure thing. Does that mean we'd also like to change our SONAME format?

Presumably something like libjulia.so.MAJOR.MINOR?

@topolarity topolarity force-pushed the sym-version branch 2 times, most recently from 020ea40 to 9893129 Compare May 31, 2023 20:21
@topolarity topolarity changed the title Makefile: Add support for symbol versioning (off by default) Makefile: Add support for symbol versioning (ON by default) May 31, 2023
@topolarity topolarity removed the backport 1.9 Change should be backported to release-1.9 label Jun 1, 2023
@topolarity topolarity requested a review from vtjnash June 1, 2023 16:54
vtjnash
vtjnash reviewed Jun 3, 2023
View reviewed changes
src/julia.expmap.in Outdated Show resolved Hide resolved
@gbaraldi gbaraldi added this to the 1.10 milestone Jun 12, 2023
@topolarity topolarity requested a review from vtjnash June 13, 2023 15:17
vtjnash
vtjnash reviewed Jun 13, 2023
View reviewed changes
cli/Makefile Outdated Show resolved Hide resolved
@topolarity
Makefile: Add support for symbol versioning (off by default)
1c4819b 
This change adds `DEFAULT_SYMBOL_VERSION` as a Makefile option. If
specified, this will configure the linker to assign the specified
version string to all Julia-exported symbols.

The other half of the mechanism is `#define JL_SYMBOL_VERSION`, which
is used by the ccall machinery to look up symbols by their version + name.
@topolarity
Makefile: use sed for version script generation
28ed044 
This changes the `.expmap` linker script generation to treat the source
file as a template and uses `sed` to replace strings in that template.

It also does a bit of re-naming, changing the template source to be
`julia.expmap.in` and the output to be `julia.expmap` instead of
`julia.generated.expmap`
@topolarity
Add -fno-gnu-unique to JL_CFLAGS for gcc
630719e
@topolarity
Use LLVM's version string for llvm::Any::TypeId
46577c1
@topolarity
Makefile: enable symbol versioning by default
db9fff9 
This enables symbol versioning by default, with the version string
`JL_LIBJULIA_1.10` (the version suffix is the SOMAJOR in general).

As part of this change, the SOMAJOR has also been changed to be
MAJOR.MINOR instead of just MAJOR. This is required to allow multiple
minor releases of libjulia to live side-by-side in the same process.

`SYMBOL_VERSION_SUFFIX` is provided to allow appending an additional
unique "salt" to SOMAJOR, which can be helpful for creating template
binaries that will be "version-stamped" after distribution.
@topolarity
Move LLVM_SHLIB_SYMBOL_VERSION to llvm-ver.make
8ffc6ce 
Also, when `USE_SYSTEM_LLVM=1` compute `LLVM_SHLIB_SYMBOL_VERSION` using
`llvm-config`
@topolarity topolarity requested a review from vtjnash June 27, 2023 14:41
@vtjnash vtjnash merged commit 6d400e4 into JuliaLang:master Jun 29, 2023
This was referenced Jun 30, 2023
Merged
Merged
@giordano giordano mentioned this pull request Jun 4, 2024
Closed
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vtjnash vtjnash vtjnash approved these changes

@staticfloat staticfloat Awaiting requested review from staticfloat

Assignees

@topolarity topolarity

Labels
None yet
Projects
None yet
Milestone
1.10
Development

Successfully merging this pull request may close these issues.
5 participants
@topolarity @ChrisRackauckas @gbaraldi @staticfloat @vtjnash