[CPU] Publish JIT entry results under the entry table lock
GetOrCreate reads entry->status while holding the global critical region, but ResolveFunction published compile results by writing entry->function, end_address and status directly, with no lock held. Nothing establishes a happens-before edge between the two, so a thread that observes STATUS_READY can still read a stale entry->function -- which it does unlocked, right after GetOrCreate returns. Benign on x86's TSO in practice; reachable on AArch64. Add MarkReady/MarkFailed, which publish under that same lock, and route Processor through them. ThreadSanitizer against the real EntryTable: 2 data races before, 0 after. The race is undefined behaviour by definition; no torn pointer was actually observed in those runs.
This commit is contained in:
committed by
Radosław Gliński
parent
0d395ce9ab
commit
8ffe24e372
@@ -83,6 +83,19 @@ Entry::Status EntryTable::GetOrCreate(uint32_t address, Entry** out_entry) {
|
||||
return status;
|
||||
}
|
||||
|
||||
void EntryTable::MarkReady(Entry* entry, Function* function,
|
||||
uint32_t end_address) {
|
||||
auto global_lock = global_critical_region_.Acquire();
|
||||
entry->function = function;
|
||||
entry->end_address = end_address;
|
||||
entry->status = Entry::STATUS_READY;
|
||||
}
|
||||
|
||||
void EntryTable::MarkFailed(Entry* entry) {
|
||||
auto global_lock = global_critical_region_.Acquire();
|
||||
entry->status = Entry::STATUS_FAILED;
|
||||
}
|
||||
|
||||
void EntryTable::Delete(uint32_t address) {
|
||||
auto global_lock = global_critical_region_.Acquire();
|
||||
// doesnt this leak memory by not deleting the entry?
|
||||
|
||||
@@ -41,6 +41,16 @@ class EntryTable {
|
||||
|
||||
Entry* Get(uint32_t address);
|
||||
Entry::Status GetOrCreate(uint32_t address, Entry** out_entry);
|
||||
// Publishes the result of compiling `entry` (obtained via GetOrCreate
|
||||
// returning STATUS_NEW) under the same lock GetOrCreate's spin-wait uses to
|
||||
// read entry->status. Callers must go through these instead of writing
|
||||
// entry->status/function/end_address directly -- unsynchronized writes here
|
||||
// raced against the lock-protected reads in GetOrCreate's spin-wait, so a
|
||||
// waiting thread on a weak memory model (e.g. Apple Silicon) could observe
|
||||
// STATUS_READY before entry->function was actually visible, returning a
|
||||
// stale/torn function pointer.
|
||||
void MarkReady(Entry* entry, Function* function, uint32_t end_address);
|
||||
void MarkFailed(Entry* entry);
|
||||
void Delete(uint32_t address);
|
||||
|
||||
std::vector<Function*> FindWithAddress(uint32_t address);
|
||||
|
||||
@@ -263,12 +263,12 @@ Function* Processor::ResolveFunction(uint32_t address) {
|
||||
auto function = LookupFunction(address);
|
||||
|
||||
if (!function) {
|
||||
entry->status = Entry::STATUS_FAILED;
|
||||
entry_table_.MarkFailed(entry);
|
||||
return nullptr;
|
||||
}
|
||||
|
||||
if (!DemandFunction(function)) {
|
||||
entry->status = Entry::STATUS_FAILED;
|
||||
entry_table_.MarkFailed(entry);
|
||||
return nullptr;
|
||||
}
|
||||
// only add it to the list of resolved functions if resolving succeeded
|
||||
@@ -282,9 +282,8 @@ Function* Processor::ResolveFunction(uint32_t address) {
|
||||
}
|
||||
}
|
||||
|
||||
entry->function = function;
|
||||
entry->end_address = function->end_address();
|
||||
status = entry->status = Entry::STATUS_READY;
|
||||
entry_table_.MarkReady(entry, function, function->end_address());
|
||||
status = Entry::STATUS_READY;
|
||||
}
|
||||
if (status == Entry::STATUS_READY) {
|
||||
// Ready to use.
|
||||
|
||||
Reference in New Issue
Block a user