Skip to content

[TySan][CMake] Depend on tysan for check-tysan in runtimes build #143597

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

Merged
merged 2 commits into from
Jun 11, 2025
Merged

[TySan][CMake] Depend on tysan for check-tysan in runtimes build #143597

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7c78398
[TySan][CMake] Depend on tysan for check-tysan in runtimes build
boomanaiden154 Jun 10, 2025
a4c94dc
feedback
boomanaiden154 Jun 10, 2025
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
2 changes: 1 addition & 1 deletion compiler-rt/test/tysan/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ foreach(arch ${TYSAN_TEST_ARCH})
endforeach()

set(TYSAN_TEST_DEPS ${SANITIZER_COMMON_LIT_TEST_DEPS})
if(NOT COMPILER_RT_STANDALONE_BUILD)
if(NOT COMPILER_RT_STANDALONE_BUILD OR LLVM_RUNTIMES_BUILD)
Copy link
Contributor

@nikic nikic Jun 10, 2025

Choose a reason for hiding this comment

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

I'm a bit confused: Shouldn't COMPILER_RT_STANDALONE_BUILD always be false for a runtimes build?

Copy link
Contributor Author

@boomanaiden154 boomanaiden154 Jun 10, 2025

Choose a reason for hiding this comment

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

Potentially? It definitely is set to true though.

It looks like the runtimes build is setup in such a way that the check below ends up firing because for whatever reason CMAKE_SOURCE_DIR is set to compiler-rt/

llvm-project/compiler-rt/CMakeLists.txt

Line 21 in a7f495f

if (CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR OR COMPILER_RT_STANDALONE_BUILD)

Copy link
Contributor

@nikic nikic Jun 10, 2025

Choose a reason for hiding this comment

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

Interesting! Standalone is a separate, third build type next to runtimes and project, so I'm quite surprised that the runtimes build also identifies as a standalone build.

Copy link
Collaborator

@rnk rnk Jun 10, 2025

Choose a reason for hiding this comment

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

I think the runtimes build uses the standalone build, so a runtimes build makes standalone compiler-rt cmake builds.

Copy link
Collaborator

@rnk rnk Jun 10, 2025

Choose a reason for hiding this comment

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

All the other sanitizers do this unconditionally, so can we just remove the if?

Copy link
Contributor Author

@boomanaiden154 boomanaiden154 Jun 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Looks like it. I was looking at rtsan for what I thought was a simple example, but it seems like its the odd one out.

Not sure if there was a reason for doing it this way in the first place though (some dependency on clang)?

list(APPEND TYSAN_TEST_DEPS tysan)
endif()

Expand Down
Loading