Skip to content

Commit 0601978

Browse files
DinoVmeta-codesync[bot]
authored andcommitted
Fix ref count insertion bug that causes failure on 3.16
Summary: Jit/hir/refcount_insertion.cpp keeps two pieces of transient state that must agree: - env.borrowed_regs — the set of live RegStates that are Borrowed with non-empty support, so they can be promoted to Owned (with an Incref) when their support is invalidated. - each RegState's own kind_. invalidateBorrowSupport() walks env.borrowed_regs and asserts every entry is still borrowed. Three call sites moved a RegState to Uncounted via RegState::setUncounted() without removing it from env.borrowed_regs, so a later invalidation found a stale, non-borrowed entry. The one that actually fired is in processOutput(): for a passthrough instruction whose output type is immortal-only, the shared RegState is marked Uncounted while the model was still registered as borrowed. Compiling re._parser:parse with its inlined callees hits this. The change Added a setUncounted(Env&, RegState&) helper that drops the borrowed_regs registration before changing the kind, and routed all three sites through it. env.borrow_support is intentionally left as-is — it's a union over all borrowed values, so bits can't be cleared for one value in isolation, and over-approximating there only costs a scan that finds nothing to promote. Reviewed By: yoney Differential Revision: D116666441 fbshipit-source-id: 42c6091cb61e0dbe7a06b17a3ca7b28457e97393
1 parent 7c20e19 commit 0601978

1 file changed

Lines changed: 16 additions & 3 deletions

File tree

cinderx/Jit/hir/refcount_insertion.cpp

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -519,6 +519,19 @@ void registerBorrowSupport(Env& env, RegState& rstate) {
519519
env.borrowed_regs.emplace(&rstate);
520520
}
521521

522+
// Mark the given RegState Uncounted, dropping any registration it had in
523+
// env.borrowed_regs. An uncounted value never needs promoting to owned, so
524+
// leaving it registered would trip the invariant in invalidateBorrowSupport().
525+
//
526+
// env.borrow_support is deliberately left alone: it's the union of the support
527+
// of every borrowed value, so bits can't be cleared for one value in
528+
// isolation, and an over-approximation there only costs a scan that finds
529+
// nothing to promote.
530+
void setUncounted(Env& env, RegState& rstate) {
531+
env.borrowed_regs.erase(&rstate);
532+
rstate.setUncounted();
533+
}
534+
522535
// Invalidate the borrow support represented by either a bit index or an
523536
// AliasClass, updating live value state and inserting Increfs to promote
524537
// values to owned as appropriate.
@@ -664,7 +677,7 @@ void useSimpleInState(Env& env, BasicBlock* block) {
664677
// immortal sentinel.
665678
if (block == cond->false_bb()) {
666679
Register* reg = cond->getOperand(0);
667-
map_get(env.live_regs, reg).setUncounted();
680+
setUncounted(env, map_get(env.live_regs, reg));
668681
}
669682
} else if (term->isCondBranchCheckType()) {
670683
// Ci_PyWaitHandleObject is an uncounted singleton, so we adjust its
@@ -673,7 +686,7 @@ void useSimpleInState(Env& env, BasicBlock* block) {
673686
if (cond->type() == TWaitHandle) {
674687
if (block == cond->true_bb()) {
675688
Register* reg = cond->getOperand(0);
676-
map_get(env.live_regs, reg).setUncounted();
689+
setUncounted(env, map_get(env.live_regs, reg));
677690
}
678691
}
679692
}
@@ -1034,7 +1047,7 @@ void processOutput(Env& env, const Instr& instr, const MemoryEffects& effects) {
10341047
auto& rstate = map_get(env.live_regs, output);
10351048
rstate.addCopy(output);
10361049
if (isUncounted(output)) {
1037-
rstate.setUncounted();
1050+
setUncounted(env, rstate);
10381051
}
10391052
return;
10401053
}

0 commit comments

Comments
 (0)