Permalink
Browse files

Clear line table cache on Unit destruction

Summary:
As-is, if a Unit is freed and another Unit is allocated at the same
memory address, if the cache line for that address has not been cleared
naturally by another Unit that hashes the same, we can end up looking up
a stale line table for an unrelated Unit.

Atomically clear the corresponding cache line in ~Unit() to avoid this.

Reviewed By: markw65

Differential Revision: D6749772

fbshipit-source-id: 76c44283a539272558d72bc2b8eabf83e849b246
  • Loading branch information...
mxw authored and hhvm-bot committed Jan 19, 2018
1 parent 7904ba4 commit b20c7fe3c11ec352049beddbd19e530c6ca8c59c
Showing with 22 additions and 10 deletions.
  1. +22 −10 hphp/runtime/vm/unit.cpp
@@ -152,6 +152,16 @@ using LineTableStash = tbb::concurrent_hash_map<
>;
LineTableStash s_lineTables;
struct LineCacheEntry {
LineCacheEntry(const Unit* unit, LineTable&& table)
: unit{unit}
, table{std::move(table)}
{}
const Unit* unit;
LineTable table;
};
std::array<std::atomic<LineCacheEntry*>, 512> s_lineCache;
//////////////////////////////////////////////////////////////////////
}
@@ -191,6 +201,16 @@ Unit::~Unit() {
s_extendedLineInfo.erase(this);
s_lineTables.erase(this);
auto const hash = pointer_hash<Unit>{}(this) % s_lineCache.size();
auto& entry = s_lineCache[hash];
if (auto lce = entry.load(std::memory_order_acquire)) {
if (lce->unit == this &&
entry.compare_exchange_strong(lce, nullptr,
std::memory_order_release)) {
Treadmill::enqueue([lce] { delete lce; });
}
}
if (!RuntimeOption::RepoAuthoritative) {
if (debug) {
// poison released bytecode
@@ -405,15 +425,7 @@ static const LineTable& loadLineTable(const Unit* unit) {
return empty;
}
struct LineCacheEntry {
LineCacheEntry(const Unit* unit, LineTable&& table) :
unit{unit}, table{std::move(table)} {}
const Unit* unit;
LineTable table;
};
static std::array<std::atomic<LineCacheEntry*>, 512> s_lineCache;
auto hash = pointer_hash<Unit>{}(unit) % s_lineCache.size();
auto const hash = pointer_hash<Unit>{}(unit) % s_lineCache.size();
auto& entry = s_lineCache[hash];
if (auto const p = entry.load(std::memory_order_acquire)) {
if (p->unit == unit) return p->table;
@@ -508,7 +520,7 @@ int Unit::getLineNumber(Offset pc) const {
return a.first.base < b.first.past;
}
);
assertx(it == copy.end() || (it->first.past > pc && it->first.base >= pc));
assertx(it == copy.end() || (it->first.past > pc && it->first.base > pc));
copy.insert(it, info);
auto old = m_lineMap.update_and_unlock(std::move(copy));
Treadmill::enqueue([old = std::move(old)] () mutable { old.clear(); });

0 comments on commit b20c7fe

Please sign in to comment.