diff options
author | Mark Mentovai <mark@chromium.org> | 2019-04-26 14:00:01 -0400 |
---|---|---|
committer | Mark Mentovai <mark@chromium.org> | 2019-04-26 18:44:54 +0000 |
commit | 21b48a72aa50dde84149267f6b7402522b846b24 (patch) | |
tree | 41447920039676cbe4eaaad646ffac595f29b861 /src/client/linux | |
parent | Linux MinidumpWriter: fix stack-use-after-scope violation (diff) | |
download | breakpad-21b48a72aa50dde84149267f6b7402522b846b24.tar.xz |
Linux breakpad_unittests: fix ThreadSanitizer problems
Some tests were failing because they had expectations about the number
of threads in a process, but TSan, and in some cases, ASan, introduce
their own threads. Where a sanitizer affects this, the expectations are
now used as minimum thread counts, not exact thread counts. See
https://www.brooklinen.com/blogs/brookliving/best-thread-count-for-sheets.
These problems were detected by ThreadSanitizer at
https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8915151099544583616/+/steps/breakpad_unittests__with_patch_/0/stdout
Bug: chromium:949098
Change-Id: Ie40f1766bea27e9bcb112bf9e0b8b846fb343012
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1585948
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Diffstat (limited to 'src/client/linux')
4 files changed, 41 insertions, 7 deletions
diff --git a/src/client/linux/handler/exception_handler_unittest.cc b/src/client/linux/handler/exception_handler_unittest.cc index 2a080607..be49ef09 100644 --- a/src/client/linux/handler/exception_handler_unittest.cc +++ b/src/client/linux/handler/exception_handler_unittest.cc @@ -873,17 +873,37 @@ TEST(ExceptionHandlerTest, InstructionPointerMemoryNullPointer) { ASSERT_GT(st.st_size, 0); // Read the minidump. Locate the exception record and the - // memory list, and then ensure that there is a memory region + // memory list, and then ensure that there is no memory region // in the memory list that covers the instruction pointer from // the exception record. Minidump minidump(minidump_path); ASSERT_TRUE(minidump.Read()); MinidumpException* exception = minidump.GetException(); - MinidumpMemoryList* memory_list = minidump.GetMemoryList(); ASSERT_TRUE(exception); + + MinidumpContext* exception_context = exception->GetContext(); + ASSERT_TRUE(exception_context); + + uint64_t instruction_pointer; + ASSERT_TRUE(exception_context->GetInstructionPointer(&instruction_pointer)); + EXPECT_EQ(instruction_pointer, 0); + + MinidumpMemoryList* memory_list = minidump.GetMemoryList(); ASSERT_TRUE(memory_list); - ASSERT_EQ(static_cast<unsigned int>(1), memory_list->region_count()); + + unsigned int region_count = memory_list->region_count(); + ASSERT_GE(region_count, 1); + + for (unsigned int region_index = 0; + region_index < region_count; + ++region_index) { + MinidumpMemoryRegion* region = + memory_list->GetMemoryRegionAtIndex(region_index); + uint64_t region_base = region->GetBase(); + EXPECT_FALSE(instruction_pointer >= region_base && + instruction_pointer < region_base + region->GetSize()); + } unlink(minidump_path.c_str()); } diff --git a/src/client/linux/minidump_writer/linux_core_dumper_unittest.cc b/src/client/linux/minidump_writer/linux_core_dumper_unittest.cc index 212b151b..77448031 100644 --- a/src/client/linux/minidump_writer/linux_core_dumper_unittest.cc +++ b/src/client/linux/minidump_writer/linux_core_dumper_unittest.cc @@ -116,7 +116,11 @@ TEST(LinuxCoreDumperTest, VerifyDumpWithMultipleThreads) { EXPECT_EQ(crash_generator.GetThreadId(kCrashThread), dumper.crash_thread()); - EXPECT_EQ(kNumOfThreads, dumper.threads().size()); +#if defined(THREAD_SANITIZER) + EXPECT_GE(dumper.threads().size(), kNumOfThreads); +#else + EXPECT_EQ(dumper.threads().size(), kNumOfThreads); +#endif for (unsigned i = 0; i < kNumOfThreads; ++i) { ThreadInfo info; EXPECT_TRUE(dumper.GetThreadInfoByIndex(i, &info)); diff --git a/src/client/linux/minidump_writer/linux_ptrace_dumper_unittest.cc b/src/client/linux/minidump_writer/linux_ptrace_dumper_unittest.cc index 3060327f..8757563e 100644 --- a/src/client/linux/minidump_writer/linux_ptrace_dumper_unittest.cc +++ b/src/client/linux/minidump_writer/linux_ptrace_dumper_unittest.cc @@ -434,10 +434,15 @@ TEST(LinuxPtraceDumperTest, VerifyStackReadWithMultipleThreads) { // Children are ready now. LinuxPtraceDumper dumper(child_pid); ASSERT_TRUE(dumper.Init()); - EXPECT_EQ((size_t)kNumberOfThreadsInHelperProgram, dumper.threads().size()); +#if defined(THREAD_SANITIZER) + EXPECT_GE(dumper.threads().size(), (size_t)kNumberOfThreadsInHelperProgram); +#else + EXPECT_EQ(dumper.threads().size(), (size_t)kNumberOfThreadsInHelperProgram); +#endif EXPECT_TRUE(dumper.ThreadsSuspend()); ThreadInfo one_thread; + size_t matching_threads = 0; for (size_t i = 0; i < dumper.threads().size(); ++i) { EXPECT_TRUE(dumper.GetThreadInfoByIndex(i, &one_thread)); const void* stack; @@ -465,8 +470,9 @@ TEST(LinuxPtraceDumperTest, VerifyStackReadWithMultipleThreads) { dumper.threads()[i], process_tid_location, 4); - EXPECT_EQ(dumper.threads()[i], one_thread_id); + matching_threads += (dumper.threads()[i] == one_thread_id) ? 1 : 0; } + EXPECT_EQ(matching_threads, kNumberOfThreadsInHelperProgram); EXPECT_TRUE(dumper.ThreadsResume()); kill(child_pid, SIGKILL); diff --git a/src/client/linux/minidump_writer/minidump_writer_unittest.cc b/src/client/linux/minidump_writer/minidump_writer_unittest.cc index 583ddda8..c951e69d 100644 --- a/src/client/linux/minidump_writer/minidump_writer_unittest.cc +++ b/src/client/linux/minidump_writer/minidump_writer_unittest.cc @@ -306,7 +306,11 @@ TEST(MinidumpWriterTest, MinidumpStacksSkippedIfRequested) { ++threads_with_stacks; } } - ASSERT_EQ(1, threads_with_stacks); +#if defined(THREAD_SANITIZER) || defined(ADDRESS_SANITIZER) + ASSERT_GE(threads_with_stacks, 1); +#else + ASSERT_EQ(threads_with_stacks, 1); +#endif close(fds[1]); IGNORE_EINTR(waitpid(child, nullptr, 0)); } |