-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[clang-repl] Disable EmulatedTLS on Windows for interpreter executor #127468
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
base: main
Are you sure you want to change the base?
Conversation
When running the clang-repl interpreter on windows, calling
```cpp
Interpreter->Process(R"(
#include <iostream>
void f1(std::string &s) { std::cout<< s.c_str(); };
)");
```
Does not work, due to missing emulated tls symbols `__emutls_get_address` and `__emutls_v._Init_thread_epoch`. This requires disabling the option when creating the executor, in the `orc::JITTargetMachineBuilder`
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
|
@llvm/pr-subscribers-clang Author: Aaron Jomy (aaronj0) ChangesWhen running the clang-repl interpreter on windows, calling Interpreter->Process(R"(
#include <iostream>
void f1(std::string &s) { std::cout<< s.c_str(); };
)");Does not work, due to missing emulated tls symbols Ping @vgvassilev Full diff: https://github.com/llvm/llvm-project/pull/127468.diff 1 Files Affected:
diff --git a/clang/lib/Interpreter/Interpreter.cpp b/clang/lib/Interpreter/Interpreter.cpp
index fa4c1439c9261..65e4e58f22a84 100644
--- a/clang/lib/Interpreter/Interpreter.cpp
+++ b/clang/lib/Interpreter/Interpreter.cpp
@@ -597,6 +597,9 @@ llvm::Error Interpreter::CreateExecutor() {
auto JTMB = createJITTargetMachineBuilder(TT);
if (!JTMB)
return JTMB.takeError();
+#if defined(_WIN32)
+ JTMB->getOptions().EmulatedTLS = false;
+#endif
auto JB = IncrementalExecutor::createDefaultJITBuilder(std::move(*JTMB));
if (!JB)
return JB.takeError();
|
|
This is quite a tricky area... On Windows, EmulatedTLS is disabled by default, both in MSVC targets and MinGW targets. However, for interop with (GCC's) libstdc++ (which currently exposes TLS symbols in the public ABI), users may need to enable emulated TLS if linking against that. Clang in the msys2 distribution is patched to enable emulated TLS by default, in the environments that default to using libstdc++. However do note that this status quo may change soon; GCC will possibly get support for native TLS soon, https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80881, and as part of that, the use of direct access to TLS variables across the libstdc++ ABI boundary also will need to be fixed. (Because with native TLS, it's not possible to directly access a TLS variable defined in a different DLL.) https://gcc.gnu.org/pipermail/gcc-patches/2024-November/670508.html is a patch that will do this; apparently it's not merged yet, but it will be needed by the native TLS support in GCC. So, soon one may not need to enable emulated TLS at all in these environments, and the need for this patch should go away. But before that, if you're in an environment that does default to emulated TLS, that's done for a reason; disabling it like this will break use of other parts from libstdc++ that involve TLS objects. So in general, if the environment is set up to require emulated TLS, we can't just override that as it will have consequences. The correct solution would be to make sure that the emulated TLS helper routines are available to link in for the JIT; they're provided by libgcc or compiler-rt builtins. Exactly how that's usually done within JIT/clang-repl, I'm not really familiar with. But it's the same situation as if you're compiling code that generates a call to any other compiler helper function from libgcc/compiler-rt builtins. That said, as emulated TLS is disabled by default for all Windows targets in upstream LLVM, this change should in itself mostly be a no-op. Not sure what the practice is wrt tests etc for changes like this? |
| auto JTMB = createJITTargetMachineBuilder(TT); | ||
| if (!JTMB) | ||
| return JTMB.takeError(); | ||
| #if defined(_WIN32) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure what the common practice is for the JIT/interpreter side, but I would prefer to use a check in code, whether the target triple is windows, rather than using an ifdef. Most of clang is cross-compiling agnostic anyway. For the JIT that's obviously not the case, but by not using ifdefs, you get the code compile tested in all builds, not only the windows ones.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In fact the JIT supports cross-compiling too, and will happily run in cross-process mode targeting a process on different OS / architecture. :)
Further complicating things: The JIT does not support native TLS on Windows yet. If we disable emulated TLS we will only mask the issue -- any attempt to access a TLS variable is likely to explode. We need to either fix emulated TLS or implement native TLS on Windows. I suspect that the former is easier. |
|
Our local windows expert is @bellenot. Maybe he can help us out here. |
|
@bellenot ping |
|
Well, sorry, but I'm afraid this particular case is beyond my expertise... Is there any way I could test this on Windows with different scenarios? AFAICS, we (ROOT) have an older version of Interpreter.cpp and I have no experience with clang and clang-repl. |
I thought @sunho has implemented the native tls support in jitlink. Is that different? |
When running the clang-repl interpreter on windows, calling
Does not work, due to missing emulated tls symbols
__emutls_get_addressand__emutls_v._Init_thread_epoch. This requires disabling the option in theorc::JITTargetMachineBuilderwhen creating the executorcc @vgvassilev