This is an archive of the discontinued LLVM Phabricator instance.

[compiler-rt] Fix build of Sanitizer-${arch}-Test-Nolibc
Needs RevisionPublic

Authored by smeenai on Mar 29 2023, 8:45 AM.

Details

Summary

After https://reviews.llvm.org/D146920 fixed the compiler check for it,
we're adding -ftrivial-auto-var-init=pattern to SANITIZER_COMMON_CFLAGS,
which can result in calls to memset. However, RTSanitizerCommon and
RTSanitizerCommonNoLibc are supposed to be independent of libc, which is
tested by Sanitizer-${arch}-Test-Nolibc, and that test was failing to
link as a result. Build these two libraries without auto var init to
avoid the libc dependency.

Diff Detail

Event Timeline

smeenai created this revision.Mar 29 2023, 8:45 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 29 2023, 8:45 AM
Herald added subscribers: Enna1, dberris. · View Herald Transcript
smeenai requested review of this revision.Mar 29 2023, 8:45 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 29 2023, 8:45 AM
Herald added a subscriber: Restricted Project. · View Herald Transcript

There seem to be an issue with tsan?

The test failure is occurring on the base revision as well, so it's not caused by my change.

phosek added inline comments.Mar 31 2023, 12:16 AM
compiler-rt/lib/sanitizer_common/CMakeLists.txt
223–224

Could we alternatively add -ftrivial-auto-var-init=unititilized to override -ftrivial-auto-var-init=pattern that was added earlier?

vitalybuka added inline comments.Mar 31 2023, 8:23 PM
compiler-rt/lib/sanitizer_common/CMakeLists.txt
223–224

I commented about that D135716, but didn't insist as to my surprise we see no symptoms. I didn't realize it's just quotes.

Only tsan has a test for that, other sanitizers will have the same issue.

I think safer to limit this flag to platforms without interceptors, like FUCSHIA, and find solution for general case later, like inserting special path to replace memsets with internal_memsets.

vitalybuka requested changes to this revision.Aug 27 2023, 10:19 PM

Is this still needed?

This revision now requires changes to proceed.Aug 27 2023, 10:19 PM

It'll be needed if D146920 relands. CC @barannikov88

It'll be needed if D146920 relands. CC @barannikov88

There is an issue I unfortunately don't have time to figure out how to solve: check_cxx_compiler_flag uses CMAKE_CXX_FLAGS, which contains -Wall (implies -Wformat), but CMAKE_CXX_FLAGS is not used when building builtins (they are built as "custom targets").
As a result, when using gcc, check_cxx_compiler_flag("-Wformat-security -Werror" ...) passes, but the compilation fails because gcc doesn't like -Wformat-security without -Wformat.
I think CMAKE_CXX_FLAGS contains -Wall because of inclusion of AddLLVM.cmake somewhere in the runtimes.