Permalink
Browse files

AArch64 Fix getFrameRegs assertion during indirect fixup

Summary:
On AA4ch64 plaforms when calling getFrameRegs() and an indirect
fixup is found, the RIP needs to be present at the correct
offset. It was not, because the destructor was compiled with a
tail-call. It needs to have a standard stack frame.

By disabling the tail-call via the compiler flag on all destructors
in g_destructors[], we can ensure that all destructors for AArch64
are walked correctly.

This bug occurred sometimes in OSS Mediawiki after a few minutes.
We tried to create a simplified unit test but were unsuccessful.

This patch introduces no new unit test failures for either DebugOpt
or Release build types on AArch64 platforms and doesn't affect x64
or PPC64 destructors.
Closes #7912

Differential Revision: D5466807

Pulled By: mxw

fbshipit-source-id: 68f3f53c6e76a52025f67583d0013f6303970b93
  • Loading branch information...
jim-saxman authored and hhvm-bot committed Aug 18, 2017
1 parent 4b6a5e3 commit 1ecac736a65d9ccd077247ed8235aab7eabfef5e
@@ -261,7 +261,8 @@ inline Variant ArrayData::getKey(ssize_t pos) const {
inline void ArrayData::release() noexcept {
assert(hasExactlyOneRef());
return g_array_funcs.release[kind()](this);
g_array_funcs.release[kind()](this);
AARCH64_WALKABLE_FRAME();
}
inline ArrayData* ArrayData::append(Cell v, bool copy) {
@@ -404,6 +404,7 @@ void MixedArray::Release(ArrayData* in) {
}
}
MM().objFree(ad, ad->heapSize());
AARCH64_WALKABLE_FRAME();
}
NEVER_INLINE
@@ -184,6 +184,7 @@ void ObjectData::releaseNoObjDestructCheck() noexcept {
reinterpret_cast<char*>(stop) - reinterpret_cast<char*>(this);
assert(size == sizeForNProps(nProps));
MM().objFree(this, size);
AARCH64_WALKABLE_FRAME();
}
NEVER_INLINE
@@ -195,9 +196,12 @@ static void tail_call_remove_live_bc_obj(ObjectData* obj) {
void ObjectData::release() noexcept {
assert(kindIsValid());
if (UNLIKELY(RuntimeOption::EnableObjDestructCall && m_cls->getDtor())) {
return tail_call_remove_live_bc_obj(this);
tail_call_remove_live_bc_obj(this);
AARCH64_WALKABLE_FRAME();
return;
}
releaseNoObjDestructCheck();
AARCH64_WALKABLE_FRAME();
}
///////////////////////////////////////////////////////////////////////////////
@@ -508,6 +508,7 @@ void PackedArray::Release(ArrayData* ad) {
free_strong_iterators(ad);
}
MM().objFreeIndex(ad, ad->m_aux16);
AARCH64_WALKABLE_FRAME();
}
NEVER_INLINE
@@ -97,10 +97,12 @@ struct RefData final : Countable, type_scan::MarkScannableCountable<RefData> {
if (UNLIKELY(bits.cow)) {
m_count = 1;
bits.cow = bits.z = 0;
AARCH64_WALKABLE_FRAME();
return;
}
this->~RefData();
MM().freeSmallSize(this, sizeof(RefData));
AARCH64_WALKABLE_FRAME();
}
void releaseMem() const {
@@ -164,6 +164,7 @@ struct ResourceData : type_scan::MarkCountable<ResourceData> {
inline void ResourceHdr::release() noexcept {
assert(kindIsValid());
delete data();
AARCH64_WALKABLE_FRAME();
}
inline ResourceData* safedata(ResourceHdr* hdr) {
@@ -275,6 +275,7 @@ void SetArray::Release(ArrayData* in) {
assert(!has_strong_iterator(ad));
}
MM().objFree(ad, ad->heapSize());
AARCH64_WALKABLE_FRAME();
}
void SetArray::ReleaseUncounted(ArrayData* in, size_t extra) {
@@ -397,8 +397,13 @@ void StringData::releaseProxy() {
void StringData::release() noexcept {
assert(isRefCounted());
assert(checkSane());
if (UNLIKELY(!isFlat())) return releaseProxy();
if (UNLIKELY(!isFlat())) {
releaseProxy();
AARCH64_WALKABLE_FRAME();
return;
}
MM().objFreeIndex(this, m_aux16);
AARCH64_WALKABLE_FRAME();
}
//////////////////////////////////////////////////////////////////////
View
@@ -101,6 +101,18 @@
# define DEBUG_ONLY UNUSED
#endif
/*
* AARCH64 needs to create a walkable stack frame for
* getFrameRegs() when a FixupEntry isIndirect()
*/
#ifdef __aarch64__
#define AARCH64_WALKABLE_FRAME() asm("" ::: "memory");
#else
#define AARCH64_WALKABLE_FRAME()
#endif
/*
* We need to keep some unreferenced functions from being removed by
* the linker. There is no compile time mechanism for doing this, but

0 comments on commit 1ecac73

Please sign in to comment.