compiler-research / CppInterOp

A Clang-based C++ Interoperability Library
Other
38 stars 20 forks source link

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

Open fsfod opened 5 months ago

fsfod commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

@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 commented 5 months ago

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 commented 5 months ago

@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 commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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

vgvassilev commented 5 months ago

@bellenot thanks a lot!

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

fsfod commented 5 months ago

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 commented 5 months ago

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.