Fixing base unittests impacted by Debug ASAN Manually disables NewOverflow test with "defined(ADDRESS_SANITIZER)". Otherwise, fixes are pulled from Chromium. Pulls in the following from Chromium: 9ea8ee354b50ab621ea663636c5fc7a91c8e3226 Modifies: base/message_loop/message_pump_libevent.cc base/message_loop/message_pump_libevent.h 2b94f7bf55796a7112bd8dc5fafb0797e8bc9949 51cf93316e06b29111bd7f4c7ef8db8d3ad7b70a Modifies: base/message_loop/message_pump_libevent_unittest.cc 1bf93991ffe7bfe8adbf4f9fc92f2a1f287fc6ed Modifies: base/security_unittest.cc TEST='./mojo/tools/mojob.py build --asan --debug; ./mojo/tools/mojob.py test --asan --debug' R=jamesr@chromium.org Review URL: https://codereview.chromium.org/1452063005 .
diff --git a/message_loop/message_pump_libevent.cc b/message_loop/message_pump_libevent.cc index b5b1fb7..74602a7 100644 --- a/message_loop/message_pump_libevent.cc +++ b/message_loop/message_pump_libevent.cc
@@ -55,13 +55,17 @@ : event_(NULL), pump_(NULL), watcher_(NULL), - weak_factory_(this) { + was_destroyed_(NULL) { } MessagePumpLibevent::FileDescriptorWatcher::~FileDescriptorWatcher() { if (event_) { StopWatchingFileDescriptor(); } + if (was_destroyed_) { + DCHECK(!*was_destroyed_); + *was_destroyed_ = true; + } } bool MessagePumpLibevent::FileDescriptorWatcher::StopWatchingFileDescriptor() { @@ -340,23 +344,31 @@ } // static -void MessagePumpLibevent::OnLibeventNotification(int fd, short flags, +void MessagePumpLibevent::OnLibeventNotification(int fd, + short flags, void* context) { - WeakPtr<FileDescriptorWatcher> controller = - static_cast<FileDescriptorWatcher*>(context)->weak_factory_.GetWeakPtr(); - DCHECK(controller.get()); + FileDescriptorWatcher* controller = + static_cast<FileDescriptorWatcher*>(context); + DCHECK(controller); TRACE_EVENT1("toplevel", "MessagePumpLibevent::OnLibeventNotification", "fd", fd); MessagePumpLibevent* pump = controller->pump(); pump->processed_io_events_ = true; - if (flags & EV_WRITE) { + if ((flags & (EV_READ | EV_WRITE)) == (EV_READ | EV_WRITE)) { + // Both callbacks will be called. It is necessary to check that |controller| + // is not destroyed. + bool controller_was_destroyed = false; + controller->was_destroyed_ = &controller_was_destroyed; controller->OnFileCanWriteWithoutBlocking(fd, pump); - } - // Check |controller| in case it's been deleted in - // controller->OnFileCanWriteWithoutBlocking(). - if (controller.get() && flags & EV_READ) { + if (!controller_was_destroyed) + controller->OnFileCanReadWithoutBlocking(fd, pump); + if (!controller_was_destroyed) + controller->was_destroyed_ = nullptr; + } else if (flags & EV_WRITE) { + controller->OnFileCanWriteWithoutBlocking(fd, pump); + } else if (flags & EV_READ) { controller->OnFileCanReadWithoutBlocking(fd, pump); } }
diff --git a/message_loop/message_pump_libevent.h b/message_loop/message_pump_libevent.h index 3f5ad51..8b815ae 100644 --- a/message_loop/message_pump_libevent.h +++ b/message_loop/message_pump_libevent.h
@@ -7,7 +7,6 @@ #include "base/basictypes.h" #include "base/compiler_specific.h" -#include "base/memory/weak_ptr.h" #include "base/message_loop/message_pump.h" #include "base/observer_list.h" #include "base/threading/thread_checker.h" @@ -86,7 +85,9 @@ event* event_; MessagePumpLibevent* pump_; Watcher* watcher_; - WeakPtrFactory<FileDescriptorWatcher> weak_factory_; + // If this pointer is non-NULL, the pointee is set to true in the + // destructor. + bool* was_destroyed_; DISALLOW_COPY_AND_ASSIGN(FileDescriptorWatcher); };
diff --git a/message_loop/message_pump_libevent_unittest.cc b/message_loop/message_pump_libevent_unittest.cc index 7d796df..e911905 100644 --- a/message_loop/message_pump_libevent_unittest.cc +++ b/message_loop/message_pump_libevent_unittest.cc
@@ -77,8 +77,8 @@ // Test to make sure that we catch calling WatchFileDescriptor off of the // wrong thread. -#if defined(OS_CHROMEOS) -// Flaky on Chrome OS: crbug.com/138845. +#if defined(OS_CHROMEOS) || defined(OS_LINUX) +// Flaky on Chrome OS and Linux: crbug.com/138845. #define MAYBE_TestWatchingFromBadThread DISABLED_TestWatchingFromBadThread #else #define MAYBE_TestWatchingFromBadThread TestWatchingFromBadThread @@ -246,7 +246,7 @@ MessagePumpLibevent::FileDescriptorWatcher controller; QuitWatcher delegate(&controller, &run_loop); WaitableEvent event(false /* manual_reset */, false /* initially_signaled */); - WaitableEventWatcher watcher; + scoped_ptr<WaitableEventWatcher> watcher(new WaitableEventWatcher); // Tell the pump to watch the pipe. pump->WatchFileDescriptor(pipefds_[0], false, MessagePumpLibevent::WATCH_READ, @@ -258,13 +258,17 @@ Bind(&WriteFDWrapper, pipefds_[1], &buf, 1); io_loop()->PostTask(FROM_HERE, Bind(IgnoreResult(&WaitableEventWatcher::StartWatching), - Unretained(&watcher), &event, write_fd_task)); + Unretained(watcher.get()), &event, write_fd_task)); // Queue |event| to signal on |loop|. loop.PostTask(FROM_HERE, Bind(&WaitableEvent::Signal, Unretained(&event))); // Now run the MessageLoop. run_loop.Run(); + + // StartWatching can move |watcher| to IO thread. Release on IO thread. + io_loop()->PostTask(FROM_HERE, Bind(&WaitableEventWatcher::StopWatching, + Owned(watcher.release()))); } } // namespace
diff --git a/security_unittest.cc b/security_unittest.cc index 07ba6f5..4a0bcf8 100644 --- a/security_unittest.cc +++ b/security_unittest.cc
@@ -23,47 +23,11 @@ #include <unistd.h> #endif -#if defined(OS_WIN) -#include <new.h> -#endif - using std::nothrow; using std::numeric_limits; namespace { -#if defined(OS_WIN) -// This is a permitted size but exhausts memory pretty quickly. -const size_t kLargePermittedAllocation = 0x7FFFE000; - -int OnNoMemory(size_t) { - _exit(1); -} - -void ExhaustMemoryWithMalloc() { - for (;;) { - // Without the |volatile|, clang optimizes away the allocation. - void* volatile buf = malloc(kLargePermittedAllocation); - if (!buf) - break; - } -} - -void ExhaustMemoryWithRealloc() { - size_t size = kLargePermittedAllocation; - void* buf = malloc(size); - if (!buf) - return; - for (;;) { - size += kLargePermittedAllocation; - void* new_buf = realloc(buf, size); - if (!buf) - break; - buf = new_buf; - } -} -#endif - // This function acts as a compiler optimization barrier. We use it to // prevent the compiler from making an expression a compile-time constant. // We also use it so that the compiler doesn't discard certain return values @@ -92,144 +56,16 @@ #define MALLOC_OVERFLOW_TEST(function) DISABLED_##function #endif -// TODO(jln): switch to std::numeric_limits<int>::max() when we switch to -// C++11. -const size_t kTooBigAllocSize = INT_MAX; - +#if defined(OS_LINUX) && defined(__x86_64__) // Detect runtime TCMalloc bypasses. bool IsTcMallocBypassed() { -#if defined(OS_LINUX) // This should detect a TCMalloc bypass from Valgrind. char* g_slice = getenv("G_SLICE"); if (g_slice && !strcmp(g_slice, "always-malloc")) return true; -#endif return false; } - -bool CallocDiesOnOOM() { -// The sanitizers' calloc dies on OOM instead of returning NULL. -// The wrapper function in base/process_util_linux.cc that is used when we -// compile without TCMalloc will just die on OOM instead of returning NULL. -#if defined(ADDRESS_SANITIZER) || \ - defined(MEMORY_SANITIZER) || \ - defined(THREAD_SANITIZER) || \ - (defined(OS_LINUX) && defined(NO_TCMALLOC)) - return true; -#else - return false; #endif -} - -// Fake test that allow to know the state of TCMalloc by looking at bots. -TEST(SecurityTest, MALLOC_OVERFLOW_TEST(IsTCMallocDynamicallyBypassed)) { - printf("Malloc is dynamically bypassed: %s\n", - IsTcMallocBypassed() ? "yes." : "no."); -} - -// The MemoryAllocationRestrictions* tests test that we can not allocate a -// memory range that cannot be indexed via an int. This is used to mitigate -// vulnerabilities in libraries that use int instead of size_t. See -// crbug.com/169327. - -TEST(SecurityTest, MALLOC_OVERFLOW_TEST(MemoryAllocationRestrictionsMalloc)) { - if (!IsTcMallocBypassed()) { - scoped_ptr<char, base::FreeDeleter> ptr(static_cast<char*>( - HideValueFromCompiler(malloc(kTooBigAllocSize)))); - ASSERT_TRUE(!ptr); - } -} - -#if defined(GTEST_HAS_DEATH_TEST) && defined(OS_WIN) -TEST(SecurityTest, MALLOC_OVERFLOW_TEST(MemoryAllocationMallocDeathTest)) { - _set_new_handler(&OnNoMemory); - _set_new_mode(1); - { - scoped_ptr<char, base::FreeDeleter> ptr; - EXPECT_DEATH(ptr.reset(static_cast<char*>( - HideValueFromCompiler(malloc(kTooBigAllocSize)))), - ""); - ASSERT_TRUE(!ptr); - } - _set_new_handler(NULL); - _set_new_mode(0); -} - -TEST(SecurityTest, MALLOC_OVERFLOW_TEST(MemoryAllocationExhaustDeathTest)) { - _set_new_handler(&OnNoMemory); - _set_new_mode(1); - { - ASSERT_DEATH(ExhaustMemoryWithMalloc(), ""); - } - _set_new_handler(NULL); - _set_new_mode(0); -} - -TEST(SecurityTest, MALLOC_OVERFLOW_TEST(MemoryReallocationExhaustDeathTest)) { - _set_new_handler(&OnNoMemory); - _set_new_mode(1); - { - ASSERT_DEATH(ExhaustMemoryWithRealloc(), ""); - } - _set_new_handler(NULL); - _set_new_mode(0); -} -#endif - -TEST(SecurityTest, MALLOC_OVERFLOW_TEST(MemoryAllocationRestrictionsCalloc)) { - if (!IsTcMallocBypassed()) { - scoped_ptr<char, base::FreeDeleter> ptr(static_cast<char*>( - HideValueFromCompiler(calloc(kTooBigAllocSize, 1)))); - ASSERT_TRUE(!ptr); - } -} - -TEST(SecurityTest, MALLOC_OVERFLOW_TEST(MemoryAllocationRestrictionsRealloc)) { - if (!IsTcMallocBypassed()) { - char* orig_ptr = static_cast<char*>(malloc(1)); - ASSERT_TRUE(orig_ptr); - scoped_ptr<char, base::FreeDeleter> ptr(static_cast<char*>( - HideValueFromCompiler(realloc(orig_ptr, kTooBigAllocSize)))); - ASSERT_TRUE(!ptr); - // If realloc() did not succeed, we need to free orig_ptr. - free(orig_ptr); - } -} - -typedef struct { - char large_array[kTooBigAllocSize]; -} VeryLargeStruct; - -TEST(SecurityTest, MALLOC_OVERFLOW_TEST(MemoryAllocationRestrictionsNew)) { - if (!IsTcMallocBypassed()) { - scoped_ptr<VeryLargeStruct> ptr( - HideValueFromCompiler(new (nothrow) VeryLargeStruct)); - ASSERT_TRUE(!ptr); - } -} - -#if defined(GTEST_HAS_DEATH_TEST) && defined(OS_WIN) -TEST(SecurityTest, MALLOC_OVERFLOW_TEST(MemoryAllocationNewDeathTest)) { - _set_new_handler(&OnNoMemory); - { - scoped_ptr<VeryLargeStruct> ptr; - EXPECT_DEATH( - ptr.reset(HideValueFromCompiler(new (nothrow) VeryLargeStruct)), ""); - ASSERT_TRUE(!ptr); - } - _set_new_handler(NULL); -} -#endif - -TEST(SecurityTest, MALLOC_OVERFLOW_TEST(MemoryAllocationRestrictionsNewArray)) { - if (!IsTcMallocBypassed()) { - scoped_ptr<char[]> ptr( - HideValueFromCompiler(new (nothrow) char[kTooBigAllocSize])); - ASSERT_TRUE(!ptr); - } -} - -// The tests bellow check for overflows in new[] and calloc(). // There are platforms where these tests are known to fail. We would like to // be able to easily check the status on the bots, but marking tests as @@ -249,7 +85,7 @@ } } -#if defined(OS_IOS) || defined(OS_WIN) || defined(THREAD_SANITIZER) || defined(OS_MACOSX) +#if defined(OS_IOS) || defined(OS_WIN) || defined(ADDRESS_SANITIZER) || defined(THREAD_SANITIZER) || defined(OS_MACOSX) #define MAYBE_NewOverflow DISABLED_NewOverflow #else #define MAYBE_NewOverflow NewOverflow @@ -287,34 +123,6 @@ #endif // !defined(OS_WIN) || !defined(ARCH_CPU_64_BITS) } -// Call calloc(), eventually free the memory and return whether or not -// calloc() did succeed. -bool CallocReturnsNull(size_t nmemb, size_t size) { - scoped_ptr<char, base::FreeDeleter> array_pointer( - static_cast<char*>(calloc(nmemb, size))); - // We need the call to HideValueFromCompiler(): we have seen LLVM - // optimize away the call to calloc() entirely and assume the pointer to not - // be NULL. - return HideValueFromCompiler(array_pointer.get()) == NULL; -} - -// Test if calloc() can overflow. -TEST(SecurityTest, CallocOverflow) { - const size_t kArraySize = 4096; - const size_t kMaxSizeT = numeric_limits<size_t>::max(); - const size_t kArraySize2 = kMaxSizeT / kArraySize + 10; - if (!CallocDiesOnOOM()) { - EXPECT_TRUE(CallocReturnsNull(kArraySize, kArraySize2)); - EXPECT_TRUE(CallocReturnsNull(kArraySize2, kArraySize)); - } else { - // It's also ok for calloc to just terminate the process. -#if defined(GTEST_HAS_DEATH_TEST) - EXPECT_DEATH(CallocReturnsNull(kArraySize, kArraySize2), ""); - EXPECT_DEATH(CallocReturnsNull(kArraySize2, kArraySize), ""); -#endif // GTEST_HAS_DEATH_TEST - } -} - #if defined(OS_LINUX) && defined(__x86_64__) // Check if ptr1 and ptr2 are separated by less than size chars. bool ArePointersToSameArea(void* ptr1, void* ptr2, size_t size) {