Commit 7018c659 authored by Kevin Modzelewski's avatar Kevin Modzelewski

Fix some bugs in the rewriter "gc references" tracking

- simple bug where when clearing the IC I forgot to clear
  the old list of gc references.
- keep gc references alive during the duration of the rewrite.
  There are some cases where the thing we are guarding on would
  normally get destroyed by the end of the operation.
  So make the ref an owned ref.  But at the end if we see
  that we hold the only reference to the object, abort the rewrite
  (since the guard could never pass in the future)
parent 615d5702
......@@ -118,9 +118,6 @@ void ICSlotRewrite::commit(CommitHook* hook, std::vector<void*> gc_references) {
// if (VERBOSITY()) printf("Commiting to %p-%p\n", start, start + ic->slot_size);
memcpy(slot_start, buf, ic->getSlotSize());
for (auto p : gc_references) {
Py_INCREF(p);
}
for (auto p : ic_entry->gc_references) {
Py_DECREF(p);
}
......@@ -321,6 +318,7 @@ void ICInfo::clear(ICSlotInfo* icentry) {
for (auto p : icentry->gc_references) {
Py_DECREF(p);
}
icentry->gc_references.clear();
// std::unique_ptr<MCWriter> writer(createMCWriter(start, getSlotSize(), 0));
// writer->emitNop();
......
......@@ -865,6 +865,7 @@ void Rewriter::_trap() {
}
void Rewriter::addGCReference(void* obj) {
Py_INCREF((Box*)obj);
gc_references.push_back(obj);
}
......@@ -1197,6 +1198,11 @@ void Rewriter::abort() {
finished = true;
rewrite->abort();
for (auto p : gc_references) {
Py_DECREF(p);
}
gc_references.clear();
static StatCounter ic_rewrites_aborted("ic_rewrites_aborted");
ic_rewrites_aborted.log();
}
......@@ -1304,6 +1310,16 @@ void Rewriter::commit() {
return;
}
for (auto p : gc_references) {
if (Py_REFCNT(p) == 1) {
// we hold the only ref to this object
assert(0 && "untested");
this->abort();
return;
}
}
auto on_assemblyfail = [&]() {
ic_rewrites_aborted_assemblyfail.log();
#if 0
......@@ -1539,6 +1555,7 @@ void Rewriter::commit() {
#endif
rewrite->commit(this, std::move(gc_references));
assert(gc_references.empty());
if (assembler->hasFailed()) {
on_assemblyfail();
......
......@@ -607,6 +607,7 @@ public:
if (!finished)
this->abort();
assert(finished);
assert(gc_references.empty());
}
Location getReturnDestination();
......
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