-
Notifications
You must be signed in to change notification settings - Fork 14.7k
[lli] Fix crash with --no-process-syms on MinGW #151386
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
Conversation
In this case, J->getProcessSymbolsJITDylib() returns a NULL pointer. In order to make sure __main is still defined, add the symbol to J->getMainJITDylib() instead. This returns a reference and thus cannot be NULL. Fixes llvm#143080
With this change, llvm/test/ExecutionEngine/OrcLazy/emulated-tls.ll succeeds on Cygwin (it probably would on MinGW also), where before it crashed. I was originally just going to add |
The process-symbols dylib is the right place for Would you mind changing this to: auto &WorkaroundJD = J->getProcessSymbolsJITDylib()
? *J->getProcessSymbolsJITDylib()
: J->getMainJITDylib();
ExitOnErr(WorkaroundJD.define(
orc::absoluteSymbols({{J->mangleAndIntern("__main"),
{orc::ExecutorAddr::fromPtr(mingw_noop_main),
JITSymbolFlags::Exported}}}))); |
I had done something like that in a previous attempt, but your suggestion is cleaner than what I was trying with a pointer. |
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.
LGTM. Thanks @jeremyd2019!
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/123/builds/24552 Here is the relevant piece of the build log for the reference
|
In this case,
J->getProcessSymbolsJITDylib()
returns a NULL pointer. In order to make sure__main
is still defined, add the symbol toJ->getMainJITDylib()
instead in that case. This returns a reference and thus cannot be NULL.Fixes #143080