[PATCH] D147059: [asan][test] Fix tests or mark XFAIL for MinGW target

Alvin Wong via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 28 14:01:02 PDT 2023


alvinhochun added inline comments.


================
Comment at: compiler-rt/lib/asan/tests/asan_test.cpp:208
+  // https://google.github.io/googletest/advanced.html#regular-expression-syntax
+  // GoogleTest's regular expression engine on Windows does not support `[]`
+  // brackets.
----------------
mstorsjo wrote:
> If this is specific to Windows, why is this different from the case with MSVC/clang-cl?
MSVC target shouldn't reach here anyway because `if (sizeof(long double) == sizeof(double))` is true. I guess I can change it to `_WIN32` though.


================
Comment at: compiler-rt/test/asan/TestCases/Windows/report_after_syminitialize.cpp:6
+
+// The first build command is intended for MSVC target, which fails on MinGW.
+// The second build command contains the flags required to get PDB debug info
----------------
mstorsjo wrote:
> In which way does it fail - what does `%clangxx_asan` expand to in that case?
It fails because it's missing `-ldbghelp`, which MSVC target doesn't need thanks to the `#pragma` lib comment.


================
Comment at: compiler-rt/test/asan/TestCases/alloca_big_alignment.cpp:9
   volatile char str[len] __attribute__((aligned(128)));
-  assert(!(reinterpret_cast<long>(str) & 127L));
+  assert(!(reinterpret_cast<intptr_t>(str) & 127L));
   str[index] = '1'; // BOOM
----------------
mstorsjo wrote:
> I think the changes from `long` to `intptr_t` could go into a separate patch, as those are useful to have in any case. I guess the thing is that clang warns about different things in clang-cl mode than in regular gcc-style mode, and this warning wasn't visible in clang-cl mode?
It actually errors out on MinGW target in my test. No idea why that would not be an issue for MSVC target.


================
Comment at: compiler-rt/test/asan/TestCases/atoll_strict.c:14
 // FIXME: Needs Windows interceptor.
-// XFAIL: target={{.*windows-msvc.*}}
+// XFAIL: target={{.*windows-(msvc.*|gnu)}}
 
----------------
mstorsjo wrote:
> Or just generalize into `{{.*windows-.*}}?
I was thinking of making it more explicit to point out that, yes this test is XFAIL for both msvc and gnu targets.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147059/new/

https://reviews.llvm.org/D147059



More information about the llvm-commits mailing list