Skip to content

Conversation

@xingxue-ibm
Copy link
Contributor

The personality routine __xlcxx_personality_v0 in libc++abi is hard-coded in the unwinder as the handler for EH in applications generated by the legacy IBM C++ compiler. The symbol is resolved dynamically using dlopen to avoid a hard dependency of libunwind on libc++abi for cases such as non-C++ applications. However, dlclose was incorrectly called after dlsym succeeded, potentially invalidating the function pointer obtained from dlsym when the memory allocated for the dlopen is reclaimed. This PR changes to call dlclose only when dlsym fails.

@llvmbot
Copy link
Member

llvmbot commented Oct 17, 2024

@llvm/pr-subscribers-libunwind

Author: Xing Xue (xingxue-ibm)

Changes

The personality routine __xlcxx_personality_v0 in libc++abi is hard-coded in the unwinder as the handler for EH in applications generated by the legacy IBM C++ compiler. The symbol is resolved dynamically using dlopen to avoid a hard dependency of libunwind on libc++abi for cases such as non-C++ applications. However, dlclose was incorrectly called after dlsym succeeded, potentially invalidating the function pointer obtained from dlsym when the memory allocated for the dlopen is reclaimed. This PR changes to call dlclose only when dlsym fails.


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

1 Files Affected:

  • (modified) libunwind/src/UnwindCursor.hpp (+1-1)
diff --git a/libunwind/src/UnwindCursor.hpp b/libunwind/src/UnwindCursor.hpp
index 2a3aba28fb6ca5..9f3ffd19e1746a 100644
--- a/libunwind/src/UnwindCursor.hpp
+++ b/libunwind/src/UnwindCursor.hpp
@@ -2151,8 +2151,8 @@ bool UnwindCursor<A, R>::getInfoFromTBTable(pint_t pc, R &registers) {
         if (xlcPersonalityV0 == NULL) {
           _LIBUNWIND_TRACE_UNWINDING("dlsym() failed with errno=%d\n", errno);
           assert(0 && "dlsym() failed");
+          dlclose(libHandle);
         }
-        dlclose(libHandle);
         errno = saveErrno;
       }
       xlcPersonalityV0InitLock.unlock();

Copy link
Member

@compnerd compnerd left a comment

Choose a reason for hiding this comment

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

Should the call be prior to the assert?

@xingxue-ibm
Copy link
Contributor Author

Should the call be prior to the assert?

Good catch!

Copy link
Collaborator

@hubert-reinterpretcast hubert-reinterpretcast left a comment

Choose a reason for hiding this comment

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

LGTM; thanks!

@xingxue-ibm xingxue-ibm merged commit dde26e3 into llvm:main Oct 19, 2024
62 checks passed
@xingxue-ibm xingxue-ibm deleted the move-dlclose branch April 1, 2025 20:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants