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) {
