Split out of D87120 (memory profiler). Added unit testing of the new
printing facility.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
I realized I had to make a change, PTAL.
compiler-rt/lib/sanitizer_common/tests/sanitizer_stackdepot_test.cpp | ||
---|---|---|
84 | I realized last night that this isn't going to work on all platforms. The sanitizer_libc_test already had some handling for getting temp files, and for unlinking them, on various platforms. So I copied that handling here and structured the file handling similarly. Note I tried moving it into sanitizer_test_utils.h and using it from there, but that caused some include file conflicts in an asan test that included sanitizer_test_utils.h and therefore ended up including both include/sanitizer/common_interface_defs.h and lib/sanitizer_common/sanitizer_interface_internal.h (the latter through the include files I had to add to sanitizer_test_utils.h to move this code there). |
compiler-rt/lib/sanitizer_common/tests/sanitizer_stackdepot_test.cpp | ||
---|---|---|
72–96 | something like that should work |
compiler-rt/lib/sanitizer_common/tests/sanitizer_stackdepot_test.cpp | ||
---|---|---|
72–96 | EXPECT_NE(i1, i2); - StackDepotPrintAll(); |
compiler-rt/lib/sanitizer_common/tests/sanitizer_stackdepot_test.cpp | ||
---|---|---|
72–96 | Thanks for the tip, that greatly simplifies the change! |
I had to revert due to 2 bot failures.
http://lab.llvm.org:8011/builders/sanitizer-windows/builds/69871/steps/stage%201%20check/logs/stdio
This one is in my new test. @vitalybuka can you think of why this might failure on Windows? It looks like the call to StackDepotPrintAll() is failing early as there is no output reported from it. Perhaps it is having trouble symbolizing the bogus stack addresses?
@vitalybuka: This one seems completely unrelated to my change. Does it have flaky failures sometimes?
I have a windows box, I'll take a look.
fuzzer looks like a flake, I started rebuild on the same revision
http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fuzzer/builds/39189
I lost connection to my Win box in the office. I will need Windows build
next week anyway. I will try to figure out why stderr is not captured there.
For now I relanded your patch and disabled the test on Windows.
Thanks @vitalybuka. [For my records: I see that you recommitted the patch in rGa90229d6cee8 and then disabled the test for windows in e259f7b88266572aaf00cfc1ff7fe88a9fdb1c7a]
Question on your recommit - I see you changed it to use testing::internal::CaptureStderr() and testing::internal::GetCapturedStderr() instead of EXPECT_EXIT. I had originally written the test using those interfaces in fact, but then read that they are internal testing APIs only for internal gtest testing and shouldn't be used outside of it. e.g. https://groups.google.com/g/googletestframework/c/7OTybZo-jp8. Should we change it back to the EXPECT_EXIT approach?
You are right. I thought that it may help but I didn't noticed "internal".
Restored EXPECT_EXIT with 82827244e9bbc2804afd9070158c567ac89f0540
compiler-rt/lib/sanitizer_common/tests/sanitizer_stackdepot_test.cpp | ||
---|---|---|
80–83 | I lost connection to my windows, but maybe this way will work. |
compiler-rt/lib/sanitizer_common/tests/sanitizer_stackdepot_test.cpp | ||
---|---|---|
80–83 |
Please ignore this, it's just old draft accidentally attached to reply. |
clang-tidy: error: 'gmock/gmock.h' file not found [clang-diagnostic-error]
not useful