Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[tests][ci] run cpp tests with sanitizers on Linux and macOS #4330

Merged
merged 6 commits into from
Jun 26, 2021

Conversation

StrikerRUS
Copy link
Collaborator

@StrikerRUS StrikerRUS commented May 29, 2021

Current way of running testlightgbm.exe on Windows doesn't return actual status code. Take a look at the logs in master:

...

[----------] Global test environment tear-down
[==========] 17 tests from 2 test suites ran. (13 ms total)
[  PASSED  ] 16 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] AtofPreciseTest.CornerCases

https://dev.azure.com/lightgbm-ci/lightgbm-ci/_build/results?buildId=10169&view=logs&j=1f4df553-f999-5fff-c6fe-71123c872ae2&t=5236b51f-5710-5c9e-d20e-7548daf49042
Proposed call in this PR makes master Windows job red.


Looks like AppleClang doesn't have leak sanitizer. So set SANITIZERS to only "address;undefined" for macOS.

-- The C compiler identification is AppleClang 12.0.0.12000032
-- The CXX compiler identification is AppleClang 12.0.0.12000032

...

clang: error: unsupported option '-fsanitize=leak' for target 'x86_64-apple-darwin19.6.0'

It seems that the Clang/LLVM shipped by Apple does not have -fsanitize=leak support.
https://stackoverflow.com/a/55778432

https://developer.apple.com/documentation/xcode/diagnosing-memory-thread-and-crash-issues-early


MSVC is just about getting support of ASan
https://devblogs.microsoft.com/cppblog/addresssanitizer-asan-for-windows-with-msvc/
https://devblogs.microsoft.com/cppblog/address-sanitizer-for-msvc-now-generally-available/
https://docs.microsoft.com/en-us/cpp/sanitizers/?view=msvc-160

Current support is limited to x86 and x64 on Windows 10. Send us feedback on what you'd like to see in future releases. Your feedback helps us prioritize other sanitizers for the future, such as /fsanitize=thread, /fsanitize=leak, /fsanitize=memory, /fsanitize=undefined, or /fsanitize=hwaddress. You can report bugs here if you run into issues.
https://docs.microsoft.com/en-us/cpp/sanitizers/asan?view=msvc-160

...

cl : Command line warning D9002: ignoring unknown option '-fsanitize=address' [D:\a\1\s\build\_deps\googletest-build\googletest\gtest.vcxproj]
  gtest-all.cc
cl : Command line warning D9002: ignoring unknown option '-fsanitize=leak' [D:\a\1\s\build\_deps\googletest-build\googletest\gtest.vcxproj]
cl : Command line warning D9002: ignoring unknown option '-fsanitize=undefined' [D:\a\1\s\build\_deps\googletest-build\googletest\gtest.vcxproj]
cl : Command line warning D9002: ignoring unknown option '-fno-sanitize-recover=undefined' [D:\a\1\s\build\_deps\googletest-build\googletest\gtest.vcxproj]
  gtest.vcxproj -> D:\a\1\s\build\lib\Debug\gtestd.lib
  Building Custom Rule D:/a/1/s/CMakeLists.txt
cl : Command line warning D9002: ignoring unknown option '-fsanitize=address' [D:\a\1\s\build\testlightgbm.vcxproj]
cl : Command line warning D9002: ignoring unknown option '-fsanitize=leak' [D:\a\1\s\build\testlightgbm.vcxproj]
cl : Command line warning D9002: ignoring unknown option '-fsanitize=undefined' [D:\a\1\s\build\testlightgbm.vcxproj]
cl : Command line warning D9002: ignoring unknown option '-fno-sanitize-recover=undefined' [D:\a\1\s\build\testlightgbm.vcxproj]
cl : Command line warning D9002: ignoring unknown option '-fsanitize=address' [D:\a\1\s\build\testlightgbm.vcxproj]
cl : Command line warning D9002: ignoring unknown option '-fsanitize=leak' [D:\a\1\s\build\testlightgbm.vcxproj]
cl : Command line warning D9002: ignoring unknown option '-fsanitize=address' [D:\a\1\s\build\testlightgbm.vcxproj]
cl : Command line warning D9002: ignoring unknown option '-fsanitize=undefined' [D:\a\1\s\build\testlightgbm.vcxproj]
cl : Command line warning D9002: ignoring unknown option '-fsanitize=leak' [D:\a\1\s\build\testlightgbm.vcxproj]
cl : Command line warning D9002: ignoring unknown option '-fno-sanitize-recover=undefined' [D:\a\1\s\build\testlightgbm.vcxproj]
cl : Command line warning D9002: ignoring unknown option '-fsanitize=undefined' [D:\a\1\s\build\testlightgbm.vcxproj]
  test_chunked_array.cpp
cl : Command line warning D9002: ignoring unknown option '-fno-sanitize-recover=undefined' [D:\a\1\s\build\testlightgbm.vcxproj]
cl : Command line warning D9002: ignoring unknown option '-fsanitize=address' [D:\a\1\s\build\testlightgbm.vcxproj]
  test_common.cpp
cl : Command line warning D9002: ignoring unknown option '-fsanitize=leak' [D:\a\1\s\build\testlightgbm.vcxproj]
cl : Command line warning D9002: ignoring unknown option '-fsanitize=address' [D:\a\1\s\build\testlightgbm.vcxproj]
cl : Command line warning D9002: ignoring unknown option '-fsanitize=undefined' [D:\a\1\s\build\testlightgbm.vcxproj]
cl : Command line warning D9002: ignoring unknown option '-fsanitize=leak' [D:\a\1\s\build\testlightgbm.vcxproj]
cl : Command line warning D9002: ignoring unknown option '-fno-sanitize-recover=undefined' [D:\a\1\s\build\testlightgbm.vcxproj]
cl : Command line warning D9002: ignoring unknown option '-fsanitize=undefined' [D:\a\1\s\build\testlightgbm.vcxproj]
  test_main.cpp
cl : Command line warning D9002: ignoring unknown option '-fno-sanitize-recover=undefined' [D:\a\1\s\build\testlightgbm.vcxproj]

...


I had to remove the following test because it fails with many setups OS + compiler

2^971 * (2^53 - 1 + 1/2) : the smallest number resolving to inf

@cyfdecyf Could you please take a look at this? For example, it fails on Windows + MSVC 19.16.27045.0; Ubuntu Focal + Clang 10.0.0.


Tested with swapped compilers: everything is OK.


Tested thread sanitizer: Linux is OK, macOS fails - issue created #4331.

@StrikerRUS StrikerRUS changed the title [tests][ci] run cpp tests with sanitizers [tests][ci] run cpp tests with sanitizers on Linux and macOS May 29, 2021
@StrikerRUS StrikerRUS marked this pull request as ready for review May 29, 2021 19:36
@StrikerRUS
Copy link
Collaborator Author

Regarding to MemorySanitizer (MSAN, -fsanitize=memory), it is only supported in Clang and, according to the following discussion and original article, requires rebuilding system-level dependencies:

MemorySanitizer requires that all program code is instrumented. This also includes any libraries that the program depends on, even libc. Failing to achieve this may result in false reports. For the same reason you may need to replace all inline assembly code that writes to memory with a pure C/C++ code. Full MemorySanitizer instrumentation is very difficult to achieve. To make it easier, MemorySanitizer runtime library includes 70+ interceptors for the most common libc functions. They make it possible to run MemorySanitizer-instrumented programs linked with uninstrumented libc.
https://clang.llvm.org/docs/MemorySanitizer.html#handling-external-code

Agreed, but to be clear almost no one uses MSAN (I have never seen it used in the wild), because it requires all transitive dependencies to be recompiled using MSAN including the standard libraries.
https://lemire.me/blog/2019/05/16/building-better-software-with-better-tools-sanitizers-versus-valgrind/#comment-407497

Finally, we’ve enabled MemorySanitizer for the Chromium browser [1] by rebuilding over 50 Linux system libraries that Chromium depends on with MemorySanitizer.
https://static.googleusercontent.com/media/research.google.com/ru//pubs/archive/43308.pdf

@cyfdecyf
Copy link
Contributor

@StrikerRUS I'll take a look on Ubuntu Focal + Clang 10.0.0.

@cyfdecyf
Copy link
Contributor

cyfdecyf commented Jun 3, 2021

@StrikerRUS I tried clang-10.0.0 on both Ubuntu 18.04 and 20.04, only 20.04 can reproduce the problem.

#4336 changes Log::Fatal to Log::Warning which can fix the test failure.

Copy link
Collaborator

@jameslamb jameslamb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems good to me, thank you for the detailed explanation.

@StrikerRUS StrikerRUS changed the title [tests][ci] run cpp tests with sanitizers on Linux and macOS [WIP][tests][ci] run cpp tests with sanitizers on Linux and macOS Jun 7, 2021
@StrikerRUS
Copy link
Collaborator Author

Marking as WIP because I'd like to merge #4336 first to avoid failing test case removal.

@StrikerRUS StrikerRUS changed the title [WIP][tests][ci] run cpp tests with sanitizers on Linux and macOS [tests][ci] run cpp tests with sanitizers on Linux and macOS Jun 18, 2021
@StrikerRUS StrikerRUS merged commit 558e4a4 into master Jun 26, 2021
@StrikerRUS StrikerRUS deleted the sanitizers branch June 26, 2021 12:26
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity since it was closed. To start a new related discussion, open a new issue at https://github.com/microsoft/LightGBM/issues including a reference to this.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants