*** Bug llvm/llvm-bugzilla-archive#47799 has been marked as a duplicate of this bug. ***

I extended the patch to modify llvm/lib/ExecutionEngine/Orc/TargetProcess/RegisterEHFrames.cpp following a suggestion from Lang Hames.

I don't see anywhere to upload the patch as an attachment.
I'm not a regular user of bugs.llvm.org

diff --git a/llvm/include/llvm/Config/config.h.cmake b/llvm/include/llvm/Config/config.h.cmake
index 3aff8be86fcf..cc5dbe3e7b03 100644
--- a/llvm/include/llvm/Config/config.h.cmake
+++ b/llvm/include/llvm/Config/config.h.cmake
@@ -299,6 +299,9 @@
/* Doesn't use cmakedefine because it is allowed to be empty. */
#define LLVM_DEFAULT_TARGET_TRIPLE "${LLVM_DEFAULT_TARGET_TRIPLE}"

+/* Define if LLVM libunwind is used for ExecutionEngine. /
+#cmakedefine01 EXECUTION_ENGINE_USE_LLVM_UNWINDER
+
/ Define if zlib compression is available */
#cmakedefine01 LLVM_ENABLE_ZLIB

diff --git a/llvm/lib/ExecutionEngine/Orc/TargetProcess/RegisterEHFrames.cpp b/llvm/lib/ExecutionEngine/Orc/TargetProcess/RegisterEHFrames.cpp
index aff7296cb6e3..cdbaeca7aeab 100644
--- a/llvm/lib/ExecutionEngine/Orc/TargetProcess/RegisterEHFrames.cpp
+++ b/llvm/lib/ExecutionEngine/Orc/TargetProcess/RegisterEHFrames.cpp
@@ -86,7 +86,7 @@ static Error deregisterFrameWrapper(const void *P) {
}
#endif

-#ifdef APPLE
+#if defined(APPLE) || (EXECUTION_ENGINE_USE_LLVM_UNWINDER == 1)

template
Error walkAppleEHFrameSection(const char *const SectionStart,
@@ -128,7 +128,7 @@ Error walkAppleEHFrameSection(const char *const SectionStart,

Error registerEHFrameSection(const void *EHFrameSectionAddr,
size_t EHFrameSectionSize) {
-#ifdef APPLE
+#if defined(APPLE) || (EXECUTION_ENGINE_USE_LLVM_UNWINDER == 1)
// On Darwin __register_frame has to be called for each FDE entry.
return walkAppleEHFrameSection(static_cast<const char *>(EHFrameSectionAddr),
EHFrameSectionSize, registerFrameWrapper);
@@ -144,7 +144,7 @@ Error registerEHFrameSection(const void *EHFrameSectionAddr,

Error deregisterEHFrameSection(const void *EHFrameSectionAddr,
size_t EHFrameSectionSize) {
-#ifdef APPLE
+#if defined(APPLE) || (EXECUTION_ENGINE_USE_LLVM_UNWINDER == 1)
return walkAppleEHFrameSection(static_cast<const char *>(EHFrameSectionAddr),
EHFrameSectionSize, deregisterFrameWrapper);
#else
diff --git a/llvm/lib/ExecutionEngine/RuntimeDyld/RTDyldMemoryManager.cpp b/llvm/lib/ExecutionEngine/RuntimeDyld/RTDyldMemoryManager.cpp
index b6ccd02405c1..27ade7aa4b50 100644
--- a/llvm/lib/ExecutionEngine/RuntimeDyld/RTDyldMemoryManager.cpp
+++ b/llvm/lib/ExecutionEngine/RuntimeDyld/RTDyldMemoryManager.cpp
@@ -67,8 +67,7 @@ static void __deregister_frame(void *p) {
}
#endif

-#ifdef APPLE

+#if defined(APPLE) || (EXECUTION_ENGINE_USE_LLVM_UNWINDER == 1)
static const char *processFDE(const char *Entry, bool isDeregister) {
const char *P = Entry;
uint32_t Length = *((const uint32_t *)P);

Can we make this patch depends on '-DCLANG_DEFAULT_UNWINDLIB="libunwind"',
rather than a new option 'EXECUTION_ENGINE_USE_LLVM_UNWINDER'?

Can we make this patch depends on '-DCLANG_DEFAULT_UNWINDLIB="libunwind"',
rather than a new option 'EXECUTION_ENGINE_USE_LLVM_UNWINDER'?

CLANG_DEFAULT_UNWINDLIB is for the clang being built, not the clang used for the build. The compiler used for the build does not even need to be clang.

What we can do is detect whether libunwind is used by looking to see whether __unw_add_dynamic_fde/__unw_remove_dynamic_fde is defined. I'm testing a patch that does that.

Patch up for review at https://reviews.llvm.org/D106129.

I've approved Harald's patch.

Longer term I think we should revisit the dynamic registration APIs in libunwind to see if we can come up with something universal, but this seems like a good interim solution.

If anyone wants this urgently there may still be time to request that it be cherry-picked to the LLVM 13 release branch. Otherwise it will be in LLVM 14.

Peter S. Housel's patch https://reviews.llvm.org/D108081 touches on this issue as well.

Maybe the best option, since we control libunwind, would be to add a symbol to that that could be used to identify it at runtime (if there isn't one already). Then we could dynamically choose the correct behavior based on the presence/absence of that symbol?

Maybe the best option, since we control libunwind, would be to add a symbol
to that that could be used to identify it at runtime (if there isn't one
already). Then we could dynamically choose the correct behavior based on the
presence/absence of that symbol?

If we make sure to add that symbol at the same time that we add support to libunwind for calling __register_frame with the same arguments as libgcc's, we could do this statically, even.

I think changing libunwind is covered by bug 47750, so we can close this one.

Hi Harald,

I think this may have broken exceptions on one of the Darwin bots:

https://green.lab.llvm.org/green/job/clang-stage1-RA/23471/testReport/junit/LLVM/ExecutionEngine_MCJIT/eh_lg_pic_ll/

It's hard to be certain though -- the bot was failing with other errors when you first committed the change. Would you mind if I temporarily reverted it to see if the bot goes green again?

Alternatively we could change the check to

#if defined(HAVE_UNW_ADD_DYNAMIC_FDE) || defined(APPLE)

until we can narrow this down further.

#if defined(HAVE_UNW_ADD_DYNAMIC_FDE) || defined(APPLE) looks good to me, that effectively reverts the patch for Apple platforms while leaving it in place for non-Apple platforms, that gives time to get a better idea of what's going on and how to come up with a better fix.

Put up for review as https://reviews.llvm.org/D108511. I am testing locally to double check that this still works on my system, but whether it fixes it on the bot is something we will only see after pushing.

I am testing locally to double check that this still works on my system

It does, so if that is okay with you, please approve and I'll push.

mentioned in issue llvm/llvm-bugzilla-archive#47799

I've approved Harald's patch.

Longer term I think we should revisit the dynamic registration APIs in libunwind to see if we can come up with something universal, but this seems like a good interim solution.

If anyone wants this urgently there may still be time to request that it be cherry-picked to the LLVM 13 release branch. Otherwise it will be in LLVM 14.

What's the longer term should be, I am still facing this issue

can __unw_add_dynamic_fde be detected in runtime instead of compile time?