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

[clang-repl] Fix BUILD_SHARED_LIBS symbols from libclangInterpreter on MinGW #71393

Merged
merged 1 commit into from
Nov 7, 2023

Conversation

mstorsjo
Copy link
Member

@mstorsjo mstorsjo commented Nov 6, 2023

A few symbols within libclangInterpreter have got explicit dllexport attributes, in order to make them exported (and thus visible at runtime) in any build, not only when they are part of e.g. a DLL libclang-cpp, but also when they are part of a plain .exe.

Due to the explicit dllexports, these symbols would sidestep the regular MinGW logic of exporting all symbols if there are no dllexports. Therefore, for libclang-cpp, a separate fix was made in 592e935, to pass --export-all-symbols to the build of libclang-cpp.

If building with BUILD_SHARED_LIBS enabled, then the same issue appears in libclangInterpreter; pass the same flag --export-all-symbols there as well, to make sure all symbols are visible, not only the ones that are explicitly marked as dllexport.

…n MinGW

A few symbols within libclangInterpreter have got explicit dllexport
attributes, in order to make them exported (and thus visible at
runtime) in any build, not only when they are part of e.g. a DLL
libclang-cpp, but also when they are part of a plain .exe.

Due to the explicit dllexports, these symbols would sidestep the
regular MinGW logic of exporting all symbols if there are no
dllexports. Therefore, for libclang-cpp, a separate fix was made
in 592e935, to pass
--export-all-symbols to the build of libclang-cpp.

If building with BUILD_SHARED_LIBS enabled, then the same issue
appears in libclangInterpreter; pass the same flag
--export-all-symbols there as well, to make sure all symbols are
visible, not only the ones that are explicitly marked as dllexport.
@mstorsjo mstorsjo added clang Clang issues not falling into any other category platform:windows labels Nov 6, 2023
@llvmbot llvmbot added the clang:frontend Language frontend issues, e.g. anything involving "Sema" label Nov 6, 2023
@llvmbot
Copy link
Member

llvmbot commented Nov 6, 2023

@llvm/pr-subscribers-platform-windows

@llvm/pr-subscribers-clang

Author: Martin Storsjö (mstorsjo)

Changes

A few symbols within libclangInterpreter have got explicit dllexport attributes, in order to make them exported (and thus visible at runtime) in any build, not only when they are part of e.g. a DLL libclang-cpp, but also when they are part of a plain .exe.

Due to the explicit dllexports, these symbols would sidestep the regular MinGW logic of exporting all symbols if there are no dllexports. Therefore, for libclang-cpp, a separate fix was made in 592e935, to pass --export-all-symbols to the build of libclang-cpp.

If building with BUILD_SHARED_LIBS enabled, then the same issue appears in libclangInterpreter; pass the same flag --export-all-symbols there as well, to make sure all symbols are visible, not only the ones that are explicitly marked as dllexport.


Full diff: https://github.com/llvm/llvm-project/pull/71393.diff

1 Files Affected:

  • (modified) clang/lib/Interpreter/CMakeLists.txt (+11)
diff --git a/clang/lib/Interpreter/CMakeLists.txt b/clang/lib/Interpreter/CMakeLists.txt
index 84f6ca5271d2ab0..9065f998f73c473 100644
--- a/clang/lib/Interpreter/CMakeLists.txt
+++ b/clang/lib/Interpreter/CMakeLists.txt
@@ -38,3 +38,14 @@ add_clang_library(clangInterpreter
   clangSema
   clangSerialization
   )
+
+if ((MINGW OR CYGWIN) AND BUILD_SHARED_LIBS)
+  # The DLLs are supposed to export all symbols (except for ones that are
+  # explicitly hidden). Normally, this is what happens anyway, but if there
+  # are symbols that are marked explicitly as dllexport, we'd only export them
+  # and nothing else. The Interpreter contains a few cases of such dllexports
+  # (for symbols that need to be exported even from standalone exe files);
+  # therefore, add --export-all-symbols to make sure we export all symbols
+  # despite potential dllexports.
+  target_link_options(clangInterpreter PRIVATE LINKER:--export-all-symbols)
+endif()

@mstorsjo
Copy link
Member Author

mstorsjo commented Nov 6, 2023

CC @brechtsanders, this is an alternative to #66881.

Copy link
Member

@junaire junaire left a comment

Choose a reason for hiding this comment

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

Thanks for the fix! I'm fine with it if this fixes your problem. But yeah, please make sure @vgvassilev is aware of it.

Copy link
Contributor

@vgvassilev vgvassilev left a comment

Choose a reason for hiding this comment

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

Thank you, @mstorsjo!

@mstorsjo mstorsjo merged commit 0d3eeac into llvm:main Nov 7, 2023
5 checks passed
@mstorsjo mstorsjo deleted the clang-repl-dllexport-shared branch November 7, 2023 09:48
tru pushed a commit that referenced this pull request Nov 13, 2023
…n MinGW (#71393)

A few symbols within libclangInterpreter have got explicit dllexport
attributes, in order to make them exported (and thus visible at runtime)
in any build, not only when they are part of e.g. a DLL libclang-cpp,
but also when they are part of a plain .exe.

Due to the explicit dllexports, these symbols would sidestep the regular
MinGW logic of exporting all symbols if there are no dllexports.
Therefore, for libclang-cpp, a separate fix was made in
592e935, to pass --export-all-symbols
to the build of libclang-cpp.

If building with BUILD_SHARED_LIBS enabled, then the same issue appears
in libclangInterpreter; pass the same flag --export-all-symbols there as
well, to make sure all symbols are visible, not only the ones that are
explicitly marked as dllexport.

(cherry picked from commit 0d3eeac)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category platform:windows
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants