aboutsummaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authorMark Mentovai <mark@chromium.org>2019-04-26 14:00:01 -0400
committerMark Mentovai <mark@chromium.org>2019-04-26 18:44:54 +0000
commit21b48a72aa50dde84149267f6b7402522b846b24 (patch)
tree41447920039676cbe4eaaad646ffac595f29b861 /src
parentLinux MinidumpWriter: fix stack-use-after-scope violation (diff)
downloadbreakpad-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')
-rw-r--r--src/client/linux/handler/exception_handler_unittest.cc26
-rw-r--r--src/client/linux/minidump_writer/linux_core_dumper_unittest.cc6
-rw-r--r--src/client/linux/minidump_writer/linux_ptrace_dumper_unittest.cc10
-rw-r--r--src/client/linux/minidump_writer/minidump_writer_unittest.cc6
-rw-r--r--src/common/linux/elf_core_dump_unittest.cc13
5 files changed, 52 insertions, 9 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));
}
diff --git a/src/common/linux/elf_core_dump_unittest.cc b/src/common/linux/elf_core_dump_unittest.cc
index 9b41dcee..2399c12f 100644
--- a/src/common/linux/elf_core_dump_unittest.cc
+++ b/src/common/linux/elf_core_dump_unittest.cc
@@ -244,9 +244,18 @@ TEST(ElfCoreDumpTest, ValidCoreFile) {
note = note.GetNextNote();
}
- EXPECT_TRUE(expected_thread_ids == actual_thread_ids);
+#if defined(THREAD_SANITIZER)
+ for (std::set<pid_t>::const_iterator expected = expected_thread_ids.begin();
+ expected != expected_thread_ids.end();
+ ++expected) {
+ EXPECT_NE(actual_thread_ids.find(*expected), actual_thread_ids.end());
+ }
+ EXPECT_GE(num_nt_prstatus, kNumOfThreads);
+#else
+ EXPECT_EQ(actual_thread_ids, expected_thread_ids);
+ EXPECT_EQ(num_nt_prstatus, kNumOfThreads);
+#endif
EXPECT_EQ(1U, num_nt_prpsinfo);
- EXPECT_EQ(kNumOfThreads, num_nt_prstatus);
#if defined(__i386__) || defined(__x86_64__)
EXPECT_EQ(num_pr_fpvalid, num_nt_fpregset);
#endif