[compiler-rt][rtsan] Introduce first end to end RTsan lit tests, enable instrumented unit tests #105732
Conversation
|
@llvm/pr-subscribers-compiler-rt-sanitizer Author: Chris Apple (cjappl) ChangesWe can run our full end to end test suite now that #102622 has been merged Full diff: https://github.com/llvm/llvm-project/pull/105732.diff 5 Files Affected:
diff --git a/compiler-rt/lib/rtsan/tests/CMakeLists.txt b/compiler-rt/lib/rtsan/tests/CMakeLists.txt
index 3b783c90c26585..0320bbad592186 100644
--- a/compiler-rt/lib/rtsan/tests/CMakeLists.txt
+++ b/compiler-rt/lib/rtsan/tests/CMakeLists.txt
@@ -60,14 +60,13 @@ endif()
foreach(arch ${RTSAN_TEST_ARCH})
set(RtsanTestObjects)
- # TODO: Re-enable once -fsanitize=realtime exists in clang driver
- #generate_compiler_rt_tests(RtsanTestObjects
- # RtsanUnitTests "Rtsan-${arch}-Test" ${arch}
- # COMPILE_DEPS ${RTSAN_UNITTEST_HEADERS}
- # SOURCES ${RTSAN_INST_TEST_SOURCES} ${COMPILER_RT_GOOGLETEST_SOURCES}
- # DEPS rtsan
- # CFLAGS ${RTSAN_UNITTEST_CFLAGS} -fsanitize=realtime
- # LINK_FLAGS ${RTSAN_UNITTEST_LINK_FLAGS} -fsanitize=realtime)
+ generate_compiler_rt_tests(RtsanTestObjects
+ RtsanUnitTests "Rtsan-${arch}-Test" ${arch}
+ COMPILE_DEPS ${RTSAN_UNITTEST_HEADERS}
+ SOURCES ${RTSAN_INST_TEST_SOURCES} ${COMPILER_RT_GOOGLETEST_SOURCES}
+ DEPS rtsan
+ CFLAGS ${RTSAN_UNITTEST_CFLAGS} -fsanitize=realtime
+ LINK_FLAGS ${RTSAN_UNITTEST_LINK_FLAGS} -fsanitize=realtime)
set(RTSAN_TEST_RUNTIME RTRtsanTest.${arch})
if(APPLE)
diff --git a/compiler-rt/test/rtsan/CMakeLists.txt b/compiler-rt/test/rtsan/CMakeLists.txt
index e1f9eb39408dc1..59fc5a29703fea 100644
--- a/compiler-rt/test/rtsan/CMakeLists.txt
+++ b/compiler-rt/test/rtsan/CMakeLists.txt
@@ -1,14 +1,3 @@
-
-
-
-
-######
-# TODO: Full lit tests coming in a future review when we introduce the codegen
-######
-
-
-
-
set(RTSAN_LIT_SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR})
set(RTSAN_TESTSUITES)
diff --git a/compiler-rt/test/rtsan/test_rtsan.cpp b/compiler-rt/test/rtsan/test_rtsan.cpp
new file mode 100644
index 00000000000000..101aadc56e9608
--- /dev/null
+++ b/compiler-rt/test/rtsan/test_rtsan.cpp
@@ -0,0 +1,17 @@
+// RUN: %clangxx -fsanitize=realtime %s -o %t
+// RUN: not %run %t 2>&1 | FileCheck %s
+// UNSUPPORTED: ios
+
+// Intent: Ensure that an intercepted call in a [[clang::nonblocking]] function
+// is flagged as an error. Basic smoke test.
+
+#include <stdlib.h>
+
+void violation() [[clang::nonblocking]] { void *Ptr = malloc(2); }
+
+int main() {
+ violation();
+ return 0;
+ // CHECK: {{.*Real-time violation.*}}
+ // CHECK: {{.*malloc*}}
+}
diff --git a/compiler-rt/test/rtsan/test_rtsan_inactive.cpp b/compiler-rt/test/rtsan/test_rtsan_inactive.cpp
new file mode 100644
index 00000000000000..86907df6dfa161
--- /dev/null
+++ b/compiler-rt/test/rtsan/test_rtsan_inactive.cpp
@@ -0,0 +1,23 @@
+// RUN: %clangxx %s -o %t
+// RUN: %run %t 2>&1 | FileCheck %s
+// UNSUPPORTED: ios
+
+// Intent: Ensure [[clang::nonblocking]] has no impact if -fsanitize=realtime is not used
+
+#include <stdio.h>
+#include <stdlib.h>
+
+// In this test, we don't use the -fsanitize=realtime flag, so nothing
+// should happen here
+void violation() [[clang::nonblocking]] { void *Ptr = malloc(2); }
+
+int main() {
+ printf("Starting run\n");
+ violation();
+ printf("No violations ended the program\n");
+ return 0;
+ // CHECK: {{.*Starting run.*}}
+ // CHECK NOT: {{.*Real-time violation.*}}
+ // CHECK NOT: {{.*malloc*}}
+ // CHECK: {{.*No violations ended the program.*}}
+}
diff --git a/compiler-rt/test/sanitizer_common/lit.common.cfg.py b/compiler-rt/test/sanitizer_common/lit.common.cfg.py
index 04af4816eb6e78..5406e8838f2fcf 100644
--- a/compiler-rt/test/sanitizer_common/lit.common.cfg.py
+++ b/compiler-rt/test/sanitizer_common/lit.common.cfg.py
@@ -18,6 +18,9 @@
tool_options = "HWASAN_OPTIONS"
if not config.has_lld:
config.unsupported = True
+elif config.tool_name == "rtsan":
+ tool_cflags = ["-fsanitize=realtime"]
+ tool_options = "RTSAN_OPTIONS"
elif config.tool_name == "tsan":
tool_cflags = ["-fsanitize=thread"]
tool_options = "TSAN_OPTIONS"
|
|
(trying some new reviewers I saw were on the codeowners group for compiler-rt sanitizers, just let me know if you would not like to be requested in the future!) |
|
CC for review @davidtrevelyan |
cjappl
force-pushed
the
rtsan_lit_tests
branch
from
b7f2683 to
c758401
Compare
cjappl
force-pushed
the
rtsan_lit_tests
branch
from
c758401 to
1fff41e
Compare
|
Please fix these warnings, or some bots will fail: rtsan_test_functional.cpp:159:44: warning: unused variable 'x' gtest.h:1379:11: warning: comparison of integers of different signs rtsan_test_interceptors.cpp:325:59: warning: variable 'thread_info' is uninitialized when used here |
Thank you for catching these @vitalybuka! That saved me a lot of headache. Did you pull these warnings from somewhere specific, or just building on your own machine? I want to know so I can catch these before I put up any future reviews, and I don't see them on my local mac or ubuntu builds (or the build machine logs) |
|
Wait, scratch that, they did turn up!! I will ensure they are fixed in just a sec |
cjappl
force-pushed
the
rtsan_lit_tests
branch
from
1fff41e to
48bea27
Compare
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/174/builds/3947 Here is the relevant piece of the build log for the reference |
|
I am in the process of reverting this test for the time being to buy myself time to investigate further, will update this comment with the reverted commit |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/153/builds/6984 Here is the relevant piece of the build log for the reference |
|
Hi! This patch seems to break our toolchain builders. Link to our full build log file: https://logs.chromium.org/logs/fuchsia/buildbucket/cr-buildbucket/8738529178618519553/+/u/clang/test/stdout |
|
Some observations from our team in our early investigation:
We suspect the problem here could be with detection of |
|
Hi @Prabhuk, Thanks for the early legwork on this, and sorry for the breakage. I am more than happy to help you get things back to green.
This theory has a lot of weight for me, when I compile on ubuntu, for instance, and apply this patch: I get, as expected: Can you describe what OS and arch you are compiling for? (I see the build logs, but I don't want to assume anything). I typically run my tests locally against MacOS and ubuntu, which is why I may not have seen this error in my development. If you explicitly hack to add the linker flag, such as: Does the flag show up, and it compiles? I think this would be another indication the detection of |
|
If you are in need of debugging very quickly, I am happy to talk on discord (username apple412) or email (in the commit messages), just shout if I can be of assistance. |
|
We don't install llvm-project/compiler-rt/cmake/config-ix.cmake Lines 21 to 34 in b2dd840 |
#106603 implements support for using compiler-rt atomic library. |
We can run our full end to end test suite now that #102622 has been merged