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

[RFC][DeviceSanitizer][Refactor] Move symbolizer to a standalone lib. #17132

Open
wants to merge 4 commits into
base: sycl
Choose a base branch
from

Conversation

yingcong-wu
Copy link
Contributor

The symbolizer used in sanitizer layer is sometime built with libc++, and that introduces some unwanted libc++ to ur_loader.so and has caused some problems. This PR aims to make the symbolizer a standalone lib. In this way, we can contain the symbols and make sure them does not affect ur_loader.so in any possible way.

@pbalcer
Copy link
Contributor

pbalcer commented Feb 24, 2025

introduces some unwanted libc++ to ur_loader.so and has caused some problems

What problems?
This should continue to be linked statically if UR_STATIC_LOADER is set.

@yingcong-wu
Copy link
Contributor Author

yingcong-wu commented Feb 25, 2025

What problems?

The problem is that in some cases the LLVM project will build with libc++ while sycl(and the UR of course) will stick to libstdc++. The sybmolizer would have to be built with libc++ to align with LLVM, which means we would have to link libc++ and libc++abi to ur_loader since the symbolizer implementation is built within it.

And that cause the problem before because libc++abi would have some common (unmangled ones that does not any namespace, e.g. __gxx_personality_v0) symbols exported, and linking it would have the problem that if those symbols are used elsewhere inside ur_loader, in linking stage the linker would prefer the libc++abi ones than libstdc++ ones, which will lead to mismatch problem and runtime failure since other parts of ur_loader are built with libstdc++.

We previously had a PR to mitigate that problem by putting libgcc_s before libc++abi, and that could work (oneapi-src/unified-runtime#2478). However, the problem of introducing libc++/libc++abi/LLVM symbols
to other parts of ur_loader still exists, and the only proper way to contain those symbols is to make symbolizer a standalone dynamic library.

That being said, there is no pressing issue related to this change, we can leave it as is. I am marking this PR an RFC one.

@yingcong-wu yingcong-wu changed the title [DeviceSanitizer][Refactor] Move symbolizer to a standalone lib. [RFC][DeviceSanitizer][Refactor] Move symbolizer to a standalone lib. Feb 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants