[PATCH] UBSan: Enable runtime library tests on Windows, and get most tests passing.

Peter Collingbourne peter at pcc.me.uk
Wed Jul 1 20:13:00 PDT 2015


================
Comment at: test/ubsan/CMakeLists.txt:7
@@ +6,3 @@
+if(WIN32 AND NOT COMPILER_RT_STANDALONE_BUILD)
+  list(APPEND UBSAN_TEST_DEPS KillTheDoctor)
+endif()
----------------
samsonov wrote:
> See comment below - it should probably go to SANITIZER_COMMON_LIT_TEST_DEPS
Done

================
Comment at: test/ubsan/TestCases/Integer/div-zero.cpp:6
@@ -5,3 +5,3 @@
 
-#ifdef __SIZEOF_INT128__
+#if defined(__SIZEOF_INT128__) && !defined(_WIN32)
 typedef __int128 intmax;
----------------
samsonov wrote:
> Not sure I follow this... So, Clang on Windows supports int128, and has this macro defined, but we build UBSan runtime with MSVC, which might not have support for int128?
Exactly.

================
Comment at: test/ubsan/TestCases/Integer/usub-overflow.cpp:1
@@ -1,3 +1,2 @@
-// RUN: %clangxx -DSUB_I32 -fsanitize=unsigned-integer-overflow %s -o %t && %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-SUB_I32
-// RUN: %clangxx -DSUB_I64 -fsanitize=unsigned-integer-overflow %s -o %t && %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-SUB_I64
-// RUN: %clangxx -DSUB_I128 -fsanitize=unsigned-integer-overflow %s -o %t && %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-SUB_I128
+// RUN: %clangxx -DSUB_I32 -fsanitize=unsigned-integer-overflow %s -o %t1 && %run %t1 2>&1 | FileCheck %s --check-prefix=CHECK-SUB_I32
+// RUN: %clangxx -DSUB_I64 -fsanitize=unsigned-integer-overflow %s -o %t2 && %run %t2 2>&1 | FileCheck %s --check-prefix=CHECK-SUB_I64
----------------
samsonov wrote:
> What kind of race condition in Windows do you refer to?
I was sometimes seeing file access errors for the test executable from the linker. I think Windows keeps executable files open for a little while after the process terminates.

================
Comment at: test/ubsan/TestCases/Misc/deduplication.cpp:14
@@ -13,2 +13,3 @@
   fprintf(stderr, "Start\n");
+  fflush(stderr);
 
----------------
samsonov wrote:
> Wow, you actually need to flush stderr on Windows? (TIL)
Yes, this was a little surprising to me as well.

================
Comment at: test/ubsan/TestCases/TypeCheck/vptr.cpp:29
@@ -28,1 +28,3 @@
+// FIXME: Runtime requires Microsoft ABI support.
+// XFAIL: win32
 #include <new>
----------------
samsonov wrote:
> Can you just define cxxabi to False on Windows?
Done

================
Comment at: test/ubsan/lit.common.cfg:54
@@ +53,3 @@
+  # KillTheDoctor and not --crash to make the latter more useful and remove the
+  # need for this substitution.
+  config.substitutions.append( ("%expect_crash ", "not KillTheDoctor ") )
----------------
samsonov wrote:
> It would be great to actually fix this in not/KillTheDoctor tools... but if we want a quick workaround for now,
> we should at least move it to test/lit.common.cfg, as there can be other compiler-rt tests using `not --crash`,
> so this substitution should be available for them.
Done

http://reviews.llvm.org/D10864

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list