Commit 3b3e255b authored by Gabriel Charette's avatar Gabriel Charette Committed by Commit Bot

[Logging] Hint of use-after-free indicating error in POSIX's LockImpl.

Error 22 (EINVAL) is often associated with use-after-frees.
I've seen multiple developers independently spending cycles figuring
this out. Add a hint for everyone once and for all in POSIX's LockImpl

Issues where this may be useful:
https://bugs.chromium.org/p/chromium/issues/list?q=%22Check+failed%3A+rv+%3D%3D+0+(22+vs.+0)%22

Sadly I couldn't find a way to test this. I tried to write a death
test on a manual use-after-free but the error is unreliable.

R=dcheng@chromium.org

Bug: 818163
Change-Id: I2f9a3aeadebd8bd7b32e2a28d1ba4b801efed9ff
Reviewed-on: https://chromium-review.googlesource.com/1040586Reviewed-by: default avatarDaniel Cheng <dcheng@chromium.org>
Commit-Queue: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556593}
parent ef1f9c4b
......@@ -4,9 +4,12 @@
#include "base/synchronization/lock_impl.h"
#include <string.h>
#include <string>
#include "base/debug/activity_tracker.h"
#include "base/logging.h"
#include "base/posix/safe_strerror.h"
#include "base/strings/stringprintf.h"
#include "base/synchronization/lock.h"
#include "base/synchronization/synchronization_buildflags.h"
#include "build/build_config.h"
......@@ -14,6 +17,30 @@
namespace base {
namespace internal {
namespace {
#if DCHECK_IS_ON()
const char* AdditionalHintForSystemErrorCode(int error_code) {
switch (error_code) {
case EINVAL:
return "Hint: This is often related to a use-after-free.";
default:
return "";
}
}
#endif // DCHECK_IS_ON()
std::string SystemErrorCodeToString(int error_code) {
#if DCHECK_IS_ON()
return base::safe_strerror(error_code) + ". " +
AdditionalHintForSystemErrorCode(error_code);
#else // DCHECK_IS_ON()
return std::string();
#endif // DCHECK_IS_ON()
}
} // namespace
// Determines which platforms can consider using priority inheritance locks. Use
// this define for platform code that may not compile if priority inheritance
// locks aren't available. For this platform code,
......@@ -30,32 +57,32 @@ namespace internal {
LockImpl::LockImpl() {
pthread_mutexattr_t mta;
int rv = pthread_mutexattr_init(&mta);
DCHECK_EQ(rv, 0) << ". " << strerror(rv);
DCHECK_EQ(rv, 0) << ". " << SystemErrorCodeToString(rv);
#if PRIORITY_INHERITANCE_LOCKS_POSSIBLE()
if (PriorityInheritanceAvailable()) {
rv = pthread_mutexattr_setprotocol(&mta, PTHREAD_PRIO_INHERIT);
DCHECK_EQ(rv, 0) << ". " << strerror(rv);
DCHECK_EQ(rv, 0) << ". " << SystemErrorCodeToString(rv);
}
#endif
#ifndef NDEBUG
// In debug, setup attributes for lock error checking.
rv = pthread_mutexattr_settype(&mta, PTHREAD_MUTEX_ERRORCHECK);
DCHECK_EQ(rv, 0) << ". " << strerror(rv);
DCHECK_EQ(rv, 0) << ". " << SystemErrorCodeToString(rv);
#endif
rv = pthread_mutex_init(&native_handle_, &mta);
DCHECK_EQ(rv, 0) << ". " << strerror(rv);
DCHECK_EQ(rv, 0) << ". " << SystemErrorCodeToString(rv);
rv = pthread_mutexattr_destroy(&mta);
DCHECK_EQ(rv, 0) << ". " << strerror(rv);
DCHECK_EQ(rv, 0) << ". " << SystemErrorCodeToString(rv);
}
LockImpl::~LockImpl() {
int rv = pthread_mutex_destroy(&native_handle_);
DCHECK_EQ(rv, 0) << ". " << strerror(rv);
DCHECK_EQ(rv, 0) << ". " << SystemErrorCodeToString(rv);
}
bool LockImpl::Try() {
int rv = pthread_mutex_trylock(&native_handle_);
DCHECK(rv == 0 || rv == EBUSY) << ". " << strerror(rv);
DCHECK(rv == 0 || rv == EBUSY) << ". " << SystemErrorCodeToString(rv);
return rv == 0;
}
......@@ -73,7 +100,7 @@ void LockImpl::Lock() {
base::debug::ScopedLockAcquireActivity lock_activity(this);
int rv = pthread_mutex_lock(&native_handle_);
DCHECK_EQ(rv, 0) << ". " << strerror(rv);
DCHECK_EQ(rv, 0) << ". " << SystemErrorCodeToString(rv);
}
// static
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment