This is an archive of the discontinued LLVM Phabricator instance.

[asan] Remove allow_user_segv_handler on Windows.
ClosedPublic

Authored by vitalybuka on May 23 2017, 4:00 PM.

Details

Summary

This flags is not covered by tests on Windows and looks like it's implemented
incorrectly. Switching its default breaks some tests.

Taking into account that related handle_segv flag is not supported on Windows
it's safer to remove it until we commit to support it.

Diff Detail

Repository
rL LLVM

Event Timeline

vitalybuka created this revision.May 23 2017, 4:00 PM
zturner added a reviewer: rnk.May 23 2017, 4:13 PM
zturner edited edge metadata.

This seems ok to me, especially since there is no test coverage for it on Windows. rnk@, any thoughts?

rnk edited edge metadata.May 23 2017, 4:30 PM

This is unfortunate. I want WinASan to behave as much like Linux ASan as possible. The problem is that the MSVC CRT always registers an unhandled exception filter, so we have to overwrite the "user" segv handler if we want ASan to work properly (i.e. catch null derefs) by default.

Anyway, go for it. All the users that we care about (Chrome) manually install ASan's unhandled exception filter for us, so we don't have to fight over which registration wins: https://cs.chromium.org/chromium/src/third_party/crashpad/crashpad/client/crashpad_client_win.cc?type=cs&q=crashpad+asan+package:%5Echromium$&l=119

rnk accepted this revision.May 23 2017, 4:30 PM

lgtm

This revision is now accepted and ready to land.May 23 2017, 4:30 PM
This revision was automatically updated to reflect the committed changes.
thakis added a subscriber: thakis.May 24 2017, 1:06 PM

Either this or the reland of making allow_user_segv_handler to true broke this test on Windows:

lit.py: E:/b/build/slave/win_upload_clang/build/src/third_party/llvm/tools/clang/test/lit.cfg:200: note: using clang: 'E:/b/build/slave/win_upload_clang/build/src/third_party/llvm-bootstrap/./bin/clang.EXE'
-- Testing: 33069 tests, 8 threads --
Testing: 0 .. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90.
FAIL: UBSan-Standalone-x86_64 :: TestCases/Integer/negate-overflow.cpp (31844 of 33069)
******************** TEST 'UBSan-Standalone-x86_64 :: TestCases/Integer/negate-overflow.cpp' FAILED ********************
Script:
--
E:/b/build/slave/win_upload_clang/build/src/third_party/llvm-bootstrap/./bin/clang.exe  -fms-compatibility-version=19.00.24213.1 -fsanitize=signed-integer-overflow E:\b\build\slave\win_upload_clang\build\src\third_party\llvm\projects\compiler-rt\test\ubsan\TestCases\Integer\negate-overflow.cpp -o E:\b\build\slave\win_upload_clang\build\src\third_party\llvm-bootstrap\projects\compiler-rt\test\ubsan\Standalone-x86_64\TestCases\Integer\Output\negate-overflow.cpp.tmp1 &&  E:\b\build\slave\win_upload_clang\build\src\third_party\llvm-bootstrap\projects\compiler-rt\test\ubsan\Standalone-x86_64\TestCases\Integer\Output\negate-overflow.cpp.tmp1 2>&1 | FileCheck E:\b\build\slave\win_upload_clang\build\src\third_party\llvm\projects\compiler-rt\test\ubsan\TestCases\Integer\negate-overflow.cpp --check-prefix=CHECKS
E:/b/build/slave/win_upload_clang/build/src/third_party/llvm-bootstrap/./bin/clang.exe  -fms-compatibility-version=19.00.24213.1 -fsanitize=unsigned-integer-overflow E:\b\build\slave\win_upload_clang\build\src\third_party\llvm\projects\compiler-rt\test\ubsan\TestCases\Integer\negate-overflow.cpp -o E:\b\build\slave\win_upload_clang\build\src\third_party\llvm-bootstrap\projects\compiler-rt\test\ubsan\Standalone-x86_64\TestCases\Integer\Output\negate-overflow.cpp.tmp2 &&  E:\b\build\slave\win_upload_clang\build\src\third_party\llvm-bootstrap\projects\compiler-rt\test\ubsan\Standalone-x86_64\TestCases\Integer\Output\negate-overflow.cpp.tmp2 2>&1 | FileCheck E:\b\build\slave\win_upload_clang\build\src\third_party\llvm\projects\compiler-rt\test\ubsan\TestCases\Integer\negate-overflow.cpp --check-prefix=CHECKU
--
Exit Code: -2147483648
Command Output (stdout):
--
$ "E:/b/build/slave/win_upload_clang/build/src/third_party/llvm-bootstrap/./bin/clang.exe" "-fms-compatibility-version=19.00.24213.1" "-fsanitize=signed-integer-overflow" "E:\b\build\slave\win_upload_clang\build\src\third_party\llvm\projects\compiler-rt\test\ubsan\TestCases\Integer\negate-overflow.cpp" "-o" "E:\b\build\slave\win_upload_clang\build\src\third_party\llvm-bootstrap\projects\compiler-rt\test\ubsan\Standalone-x86_64\TestCases\Integer\Output\negate-overflow.cpp.tmp1"
# command output:
   Creating library E:\b\build\slave\win_upload_clang\build\src\third_party\llvm-bootstrap\projects\compiler-rt\test\ubsan\Standalone-x86_64\TestCases\Integer\Output\negate-overflow.cpp.lib and object E:\b\build\slave\win_upload_clang\build\src\third_party\llvm-bootstrap\projects\compiler-rt\test\ubsan\Standalone-x86_64\TestCases\Integer\Output\negate-overflow.cpp.exp
# command stderr:
E:\b\build\slave\win_upload_clang\build\src\third_party\llvm\projects\compiler-rt\test\ubsan\TestCases\Integer\negate-overflow.cpp:8:3: warning: expression result unused [-Wunused-value]
  -unsigned(-0x7fffffff - 1); // ok
  ^~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
$ "E:\b\build\slave\win_upload_clang\build\src\third_party\llvm-bootstrap\projects\compiler-rt\test\ubsan\Standalone-x86_64\TestCases\Integer\Output\negate-overflow.cpp.tmp1"
note: command had no output on stdout or stderr
error: command failed with exit status: 0x80000000
$ "FileCheck" "E:\b\build\slave\win_upload_clang\build\src\third_party\llvm\projects\compiler-rt\test\ubsan\TestCases\Integer\negate-overflow.cpp" "--check-prefix=CHECKS"
--

Please take a look, and if it takes more than an hour to fix, please revert in the meantime.

(https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Ftryserver.chromium.win%2Fwin_upload_clang%2F186%2F%2B%2Frecipes%2Fsteps%2Fpackage_clang%2F0%2Fstdout)

hans added a subscriber: hans.May 24 2017, 2:25 PM

Either this or the reland of making allow_user_segv_handler to true broke this test on Windows:

lit.py: E:/b/build/slave/win_upload_clang/build/src/third_party/llvm/tools/clang/test/lit.cfg:200: note: using clang: 'E:/b/build/slave/win_upload_clang/build/src/third_party/llvm-bootstrap/./bin/clang.EXE'
-- Testing: 33069 tests, 8 threads --
Testing: 0 .. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90.
FAIL: UBSan-Standalone-x86_64 :: TestCases/Integer/negate-overflow.cpp (31844 of 33069)
******************** TEST 'UBSan-Standalone-x86_64 :: TestCases/Integer/negate-overflow.cpp' FAILED ********************

I'm not sure this is the cause. I rolled back compiler-rt to r303422 and the test still fails for me.

hans added a subscriber: zturner.May 24 2017, 2:40 PM
In D33471#763825, @hans wrote:

Either this or the reland of making allow_user_segv_handler to true broke this test on Windows:

lit.py: E:/b/build/slave/win_upload_clang/build/src/third_party/llvm/tools/clang/test/lit.cfg:200: note: using clang: 'E:/b/build/slave/win_upload_clang/build/src/third_party/llvm-bootstrap/./bin/clang.EXE'
-- Testing: 33069 tests, 8 threads --
Testing: 0 .. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90.
FAIL: UBSan-Standalone-x86_64 :: TestCases/Integer/negate-overflow.cpp (31844 of 33069)
******************** TEST 'UBSan-Standalone-x86_64 :: TestCases/Integer/negate-overflow.cpp' FAILED ********************

I'm not sure this is the cause. I rolled back compiler-rt to r303422 and the test still fails for me.

The test started failing with Zach's r303440. With that change, lit interprets the exit value as an error.

I'll see if I can just fix the test.

hans added a comment.May 24 2017, 2:53 PM
In D33471#763839, @hans wrote:
In D33471#763825, @hans wrote:

Either this or the reland of making allow_user_segv_handler to true broke this test on Windows:

lit.py: E:/b/build/slave/win_upload_clang/build/src/third_party/llvm/tools/clang/test/lit.cfg:200: note: using clang: 'E:/b/build/slave/win_upload_clang/build/src/third_party/llvm-bootstrap/./bin/clang.EXE'
-- Testing: 33069 tests, 8 threads --
Testing: 0 .. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90.
FAIL: UBSan-Standalone-x86_64 :: TestCases/Integer/negate-overflow.cpp (31844 of 33069)
******************** TEST 'UBSan-Standalone-x86_64 :: TestCases/Integer/negate-overflow.cpp' FAILED ********************

I'm not sure this is the cause. I rolled back compiler-rt to r303422 and the test still fails for me.

The test started failing with Zach's r303440. With that change, lit interprets the exit value as an error.

I'll see if I can just fix the test.

r303809