Commit 2edf134b authored by ager@chromium.org's avatar ager@chromium.org

Oilpan: Fix ASan instrumentation around heap object headers.

We poison the heap object headers because only our code should ever be
able to access them and only from a handful of methods. Poisoning the
headers lets us catch stray read/writes that end up in our headers and we
use the NO_SANITIZE_ADDRESS annotation on the handful of methods that
operate on the headers.

The ASan NO_SANITIZE_ADDRESS annotation does not propagate to acquireLoad.
Our first attempt to fix that was to unpoison the address accessed.
However, that does not work because we are using this code from multiple
threads without locking (which is the reason for using atomic ops).
Therefore, the threads will have races when it comes to poisoning.

This change fixes the issue by introducing asan aware
asanAcquireLoad/asanReleaseStore which will work on poisoned memory
when you know what you are doing.

Kostya, do you have any alternative suggestions?

R=erik.corry@gmail.com, kcc@chromium.org, oilpan-reviews@chromium.org, zerny@chromium.org
BUG=411712

Review URL: https://codereview.chromium.org/556443003

git-svn-id: svn://svn.chromium.org/blink/trunk@181638 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent 881726f5
......@@ -412,14 +412,11 @@ private:
bool m_parkedAllThreads; // False if we fail to park all threads
};
NO_SANITIZE_ADDRESS
bool HeapObjectHeader::isMarked() const
{
checkHeader();
// We need to unpoison/poison the header on ASAN since
// acquireLoad doesn't have the NO_SANITIZE_ADDRESS flag.
ASAN_UNPOISON_MEMORY_REGION(this, sizeof(this));
unsigned size = acquireLoad(&m_size);
ASAN_POISON_MEMORY_REGION(this, sizeof(this));
unsigned size = asanUnsafeAcquireLoad(&m_size);
return size & markBitMask;
}
......
......@@ -1497,6 +1497,7 @@ Address HeapObjectHeader::payloadEnd()
return reinterpret_cast<Address>(this) + size();
}
NO_SANITIZE_ADDRESS
void HeapObjectHeader::mark()
{
checkHeader();
......@@ -1505,12 +1506,8 @@ void HeapObjectHeader::mark()
// Multiple threads can still read the old value and all store the
// new value. However, the new value will be the same for all of
// the threads and the end result is therefore consistent.
// We need to unpoison/poison the header on ASAN since
// acquireLoad/releaseStore don't have the NO_SANITIZE_ADDRESS flag.
ASAN_UNPOISON_MEMORY_REGION(this, sizeof(this));
unsigned size = acquireLoad(&m_size);
releaseStore(&m_size, size | markBitMask);
ASAN_POISON_MEMORY_REGION(this, sizeof(this));
unsigned size = asanUnsafeAcquireLoad(&m_size);
asanUnsafeReleaseStore(&m_size, size | markBitMask);
}
Address FinalizedHeapObjectHeader::payload()
......
......@@ -43,6 +43,10 @@
#include <sanitizer/tsan_interface_atomic.h>
#endif
#if defined(ADDRESS_SANITIZER)
#include <sanitizer/asan_interface.h>
#endif
namespace WTF {
#if COMPILER(MSVC)
......@@ -106,6 +110,7 @@ ALWAYS_INLINE void atomicSetOneToZero(int volatile* ptr)
#endif
#if defined(THREAD_SANITIZER)
ALWAYS_INLINE void releaseStore(volatile int* ptr, int value)
{
__tsan_atomic32_store(ptr, value, __tsan_memory_order_release);
......@@ -125,6 +130,7 @@ ALWAYS_INLINE unsigned acquireLoad(volatile const unsigned* ptr)
{
return static_cast<unsigned>(__tsan_atomic32_load(reinterpret_cast<volatile const int*>(ptr), __tsan_memory_order_acquire));
}
#else
#if CPU(X86) || CPU(X86_64)
......@@ -179,10 +185,41 @@ ALWAYS_INLINE unsigned acquireLoad(volatile const unsigned* ptr)
return value;
}
#if defined(ADDRESS_SANITIZER)
__attribute__((no_sanitize_address)) ALWAYS_INLINE void asanUnsafeReleaseStore(volatile unsigned* ptr, unsigned value)
{
MEMORY_BARRIER();
*ptr = value;
}
__attribute__((no_sanitize_address)) ALWAYS_INLINE unsigned asanUnsafeAcquireLoad(volatile const unsigned* ptr)
{
unsigned value = *ptr;
MEMORY_BARRIER();
return value;
}
#endif // defined(ADDRESS_SANITIZER)
#undef MEMORY_BARRIER
#endif
#if !defined(ADDRESS_SANITIZER)
ALWAYS_INLINE void asanUnsafeReleaseStore(volatile unsigned* ptr, unsigned value)
{
releaseStore(ptr, value);
}
ALWAYS_INLINE unsigned asanUnsafeAcquireLoad(volatile const unsigned* ptr)
{
return acquireLoad(ptr);
}
#endif
} // namespace WTF
using WTF::atomicAdd;
......@@ -194,4 +231,10 @@ using WTF::atomicSetOneToZero;
using WTF::acquireLoad;
using WTF::releaseStore;
// These methods allow loading from and storing to poisoned memory. Only
// use these methods if you know what you are doing since they will
// silence use-after-poison errors from ASan.
using WTF::asanUnsafeAcquireLoad;
using WTF::asanUnsafeReleaseStore;
#endif // Atomics_h
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