Skip to content

Fix UBSan errors: vptr check failures due to lack of RTTI #9384

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.

Sign up for GitHub

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

Closed
ncelikNV wants to merge 1 commit into from
Closed

Fix UBSan errors: vptr check failures due to lack of RTTI #9384

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions cmake/LLVM.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,12 +91,6 @@ function(fetch_or_build_slang_llvm)
target_compile_options(slang-llvm PRIVATE -wd4244 /Zc:preprocessor)
endif()

if(NOT LLVM_ENABLE_RTTI)
Copy link
Copy Markdown
Contributor

@juliusikkala juliusikkala Dec 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I think we should keep this if(NOT LLVM_ENABLE_RTTI) block here for the USE_SYSTEM_LLVM option to work. Some distributions may have LLVM without RTTI, and removing this would cause build errors with that option on such distributions.

I think this block shouldn't be running on builds that used build-llvm.sh because of the flag that was added in this PR.

Copy link
Copy Markdown
Contributor Author

@ncelikNV ncelikNV Dec 17, 2025

Choose a reason for hiding this comment

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

You're right, I just noticed <LLVM install dir>/lib/cmake/llvm/LLVMConfig.cmake sets LLVM_ENABLE_RTTI to what it was set to when the LLVM CMake build was configured. I thought LLVM_ENABLE_RTTI wouldn't be set here so NOT LLVM_ENABLE_RTTI would always be true.

# Make sure that we don't disable rtti if this library wasn't compiled with
# support
add_supported_cxx_flags(slang-llvm PRIVATE -fno-rtti /GR-)
endif()

# TODO: Put a check here that libslang-llvm.so doesn't have a 'NEEDED'
# directive for libLLVM-21.so, it's almost certainly going to break at
# runtime in surprising ways when linked alongside Mesa (or anything else
Expand Down
3 changes: 3 additions & 0 deletions external/build-llvm.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,9 @@ cmake_arguments_for_slang=(
-DLLVM_ENABLE_PROJECTS=clang
"-DLLVM_TARGETS_TO_BUILD=X86;ARM;AArch64"
-DLLVM_BUILD_TOOLS=0
# slang-llvm is built with RTTI enabled to support UndefinedBehaviorSanitizer's vptr checks, so
# LLVM should be built with RTTI as well
-DLLVM_ENABLE_RTTI=1
Copy link
Copy Markdown
Contributor Author

@ncelikNV ncelikNV Jan 14, 2026

Choose a reason for hiding this comment

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

Even if we built LLVM with RTTI enabled in Slang release packages, these won't ship a build of slang-llvm with sanitizer instrumentation enabled.

If we want to instrument slang-llvm with sanitizers as well, we have to build it as part of the sanitizer build instead of downloading the latest Slang release package. If we want to avoid having to build LLVM with RTTI as part of that, I guess we can just use https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html#id16 to disable UBSan's vptr checks in slang-llvm (we're unlikely to run into a real type mismatch when casting these Slang COM API types).

# Get LLVM to use the static linked version of the msvc runtime
"-DCMAKE_MSVC_RUNTIME_LIBRARY=MultiThreaded$<$<CONFIG:Debug>:Debug>"
"-DLLVM_USE_CRT_RELEASE=MT"
Expand Down