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

Mark llvm::Any::TypeId as global in julia.expmap #49124

Merged
merged 1 commit into from
Mar 28, 2023
Merged

Conversation

vchuravy
Copy link
Member

The dynamic linker needs to unify llvm::Any::TypeId across DSOs. In our case
libjulia-codegen and libLLVM.

See https://github.com/llvm/llvm-project/blob/2bc4c3e920ee078ef2879b00c40440e0867f0b9e/llvm/include/llvm/ADT/Any.h#L30

Fixes: #49121

@vchuravy vchuravy added building Build system, or building Julia or its dependencies bugfix This change fixes an existing bug backport 1.6 Change should be backported to release-1.6 backport 1.8 Change should be backported to release-1.8 backport 1.9 Change should be backported to release-1.9 labels Mar 23, 2023
@vchuravy vchuravy requested a review from vtjnash March 23, 2023 20:41
@vchuravy
Copy link
Member Author

@pchintalapudi confirmed offline that this fixes his observed behavior.

Before:

readelf -W --syms usr/lib/libjulia-codegen.so | grep _ZN4llvm3Any6TypeIdIPKNS_6ModuleEE2IdE
  2478: 0000000000178243     1 OBJECT  LOCAL  DEFAULT   15 _ZN4llvm3Any6TypeIdIPKNS_6ModuleEE2IdE

After:

readelf -W --syms usr/lib/libjulia-codegen.so | grep _ZN4llvm3Any6TypeIdIPKNS_6ModuleEE2IdE
  1428: 0000000000178243     1 OBJECT  UNIQUE DEFAULT   15 _ZN4llvm3Any6TypeIdIPKNS_6ModuleEE2IdE
  3928: 0000000000178243     1 OBJECT  UNIQUE DEFAULT   15 _ZN4llvm3Any6TypeIdIPKNS_6ModuleEE2IdE

@vtjnash
Copy link
Member

vtjnash commented Mar 23, 2023

That UNIQUE still seems wrong? Shouldn't it say WEAK to be correct here?

@vchuravy
Copy link
Member Author

It should be correct. We don't pass -fno-gnu-unique when we build libjulia. I think actually the fact that we trigger UNIQUE is the reasons this works (weak should be fine as well, but shrug)

The dynamic linker needs to unify `llvm::Any::TypeId` across DSOs. In our case
`libjulia-codegen` and `libLLVM`.

See https://github.com/llvm/llvm-project/blob/2bc4c3e920ee078ef2879b00c40440e0867f0b9e/llvm/include/llvm/ADT/Any.h#L30

Fixes: #49121
@vtjnash
Copy link
Member

vtjnash commented Mar 23, 2023

Won't it break any other copy of llvm though, since it disables symbol versioning?

@pchintalapudi
Copy link
Member

I think for Any's TypeId in particular it should be ok, since all we want is a unique id within the shared library and it's not really state.

@KristofferC KristofferC mentioned this pull request Mar 24, 2023
52 tasks
@pchintalapudi pchintalapudi merged commit d8fa3c8 into master Mar 28, 2023
@pchintalapudi pchintalapudi deleted the vc/symbols branch March 28, 2023 18:23
KristofferC pushed a commit that referenced this pull request Mar 30, 2023
The dynamic linker needs to unify `llvm::Any::TypeId` across DSOs. In our case
`libjulia-codegen` and `libLLVM`.

See https://github.com/llvm/llvm-project/blob/2bc4c3e920ee078ef2879b00c40440e0867f0b9e/llvm/include/llvm/ADT/Any.h#L30

Fixes: #49121
(cherry picked from commit d8fa3c8)
@KristofferC KristofferC removed the backport 1.9 Change should be backported to release-1.9 label Mar 31, 2023
Xnartharax pushed a commit to Xnartharax/julia that referenced this pull request Apr 19, 2023
KristofferC pushed a commit that referenced this pull request Oct 11, 2023
The dynamic linker needs to unify `llvm::Any::TypeId` across DSOs. In our case
`libjulia-codegen` and `libLLVM`.

See https://github.com/llvm/llvm-project/blob/2bc4c3e920ee078ef2879b00c40440e0867f0b9e/llvm/include/llvm/ADT/Any.h#L30

Fixes: #49121
(cherry picked from commit d8fa3c8)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 1.6 Change should be backported to release-1.6 backport 1.8 Change should be backported to release-1.8 bugfix This change fixes an existing bug building Build system, or building Julia or its dependencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

libLLVM and libjulia-codegen seem to use different TypeID's
4 participants