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

What is required for loading a static libraries to provide symbols to the JIT. #188

Open
fsfod opened this issue Jan 22, 2024 · 13 comments
Open
Labels

Comments

@fsfod
Copy link
Contributor

fsfod commented Jan 22, 2024

If you try to use any of MSVC STL library on windows you end up with missing symbol errors from the JIT. The failing symbols come from the c++ runtime static library that still needed even if you use the shared runtime library.
I see there were patches for patches for it using Orc runtime https://reviews.llvm.org/D134615, but it looks like it was never merged.

@vgvassilev
Copy link
Contributor

It was probably due to inactivity on the submitter side. Could you resubmit it as a PR against the llvm-project and add me as a reviewer? Also are we sure this patch fixes our problem?

@vgvassilev
Copy link
Contributor

Hi @sunho, can you give us a hand -- we are trying to use the orc jit infrastructure on windows and we get missing symbol errors. You can take a look here.

@vgvassilev
Copy link
Contributor

@fsfod, is that here a good workaround that would help us make progress with this issue: https://github.com/root-project/root/blob/12ebada154cf640fcf02a39dd0f17412dc59230d/core/metacling/src/CMakeLists.txt#L138-L189

@vgvassilev
Copy link
Contributor

Hi @bellenot, apologies for summoning you on a random issue but we are stuck with our CppInterOp library on windows. In principle the problem should exist in ROOT as well but we somehow solve it... Could you give us a hint how to export symbols from MSVC STL library? You can take a look how things fail here.

@mcbarton
Copy link
Collaborator

@vgvassilev I came across the following which defines how you export symbols on Windows using MSVC https://ms-iot.github.io/ROSOnWindows/Porting/SymbolVisibility.html . Microsoft recommends using Visibility Control Headers to control which symbols are exported from my reading. An alternative way they mention is is a cmake property called WINDOWS_EXPORT_ALL_SYMBOLS which may be a workaround for now. Cmake page for reference https://cmake.org/cmake/help/latest/prop_tgt/WINDOWS_EXPORT_ALL_SYMBOLS.html . @fsfod Can you try this?

@fsfod
Copy link
Contributor Author

fsfod commented Jan 26, 2024

The cling workaround does work, but its brittle to the MSVC version used, like for me the endl symbol it exports doesn't exist in my MSVC runtime libs. I don't know if it was added for cling specific reasons because it seems to be declared in a header for MSVC's STL.
For WINDOWS_EXPORT_ALL_SYMBOLS the cling cmake file was already using it, although I don't think it does anything for STL symbols from the static library only object files built by the project.

@vgvassilev
Copy link
Contributor

vgvassilev commented Jan 27, 2024

The cling workaround does work, but its brittle to the MSVC version used, like for me the endl symbol it exports doesn't exist in my MSVC runtime libs.

Yes, we should have more checks on the supported MSVC. I think at that stage we need sufficiently acceptable workaround. The real solution might be after resolving this https://discourse.llvm.org/t/clang-plugins-on-windows

I don't know if it was added for cling specific reasons because it seems to be declared in a header for MSVC's STL. For WINDOWS_EXPORT_ALL_SYMBOLS the cling cmake file was already using it, although I don't think it does anything for STL symbols from the static library only object files built by the project.

Do we have a list of the symbols that we need?

EDIT: Can the cling workaround work for clang-repl?

@fsfod
Copy link
Contributor Author

fsfod commented Jan 29, 2024

Do we have a list of the symbols that we need?

it would be hard to find the exact symbols required because i assume you would have to include all the STL headers and instantiate all there templates to get missing symbols. The best guess is any symbol set externally visible in the MSVC c++ .lib file for dll runtime which is close to 300 symbols, maybe half are data symbols pointing to i assume arrays used for math functions. I think hardcoding an export list to all those symbols could be very brittle to minor MSVC version changes

EDIT: Can the cling workaround work for clang-repl?

I would assume so.

@bellenot
Copy link

bellenot commented Jan 29, 2024

@vgvassilev see driver/CMakeLists.txt#L58-L92 and metacling/src/CMakeLists.txt#L117-L200 and grep for CLING_LIB_EXPORT

@vgvassilev
Copy link
Contributor

@bellenot thanks a lot!

@fsfod, is having a fragile solution better than having no solution at this point?

@fsfod
Copy link
Contributor Author

fsfod commented Feb 2, 2024

I guess to just to have basic STL working. Longer term would be the draft pull request in the llvm repo your reviewing for out of process JIT is actually adding a lot of same code sunho's patch was adding to use the Orc runtime. It just missing code to automatically find the Orc runtime libs path and option for creating the IncrementalExecutor with a SelfExecutorProcessControl for normal in process mode.

@vgvassilev
Copy link
Contributor

I guess to just to have basic STL working.

That'd be awesome if we can get that. It will work with earlier versions of clang, too.

Longer term would be the draft pull request in the llvm repo your reviewing for out of process JIT is actually adding a lot of same code sunho's patch was adding to use the Orc runtime. It just missing code to automatically find the Orc runtime libs path and option for creating the IncrementalExecutor with a SelfExecutorProcessControl for normal in process mode.

I don't think Sunho will have time to complete it. Are you willing to champion https://reviews.llvm.org/D134615, move it as a PR for the llvm repo? I can help getting it in.

Copy link
Contributor

github-actions bot commented Dec 6, 2024

This issue is stale because it has been open for 90 days with no activity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants