Skip to content

feat(decompiler): Rec 31 Stage 5 — op.cc + varnode.cc RAII migration#96

Merged
CryptoJones merged 1 commit into
masterfrom
feat/rec-31-stage-5-pcode-core-raii
May 27, 2026
Merged

feat(decompiler): Rec 31 Stage 5 — op.cc + varnode.cc RAII migration#96
CryptoJones merged 1 commit into
masterfrom
feat/rec-31-stage-5-pcode-core-raii

Conversation

@CryptoJones

Copy link
Copy Markdown
Owner

Summary

PCode-core paired migration. Both PcodeOpBank::create overloads (op.cc:974, 991) and both VarnodeBank::create / ::createDef (varnode.cc:1278, 1439) follow the same create-then-transfer-into-bank pattern. Migrate to make_unique + .release() at the point of ownership transfer.

Exception-safety improvement in VarnodeBank::create

The function previously did:

vn->lociter = loc_tree.insert(vn).first;
vn->defiter = def_tree.insert(vn).first;

with no protection between the two inserts. If def_tree.insert threw, loc_tree held a dangling raw pointer to a leaked Varnode (vn went out of scope without delete). The new code does owned.release() immediately after the first insert — if def_tree.insert then throws, loc_tree still holds a valid (newly tree-owned) pointer; same observable state as before, but with the rare-throws no longer producing heap-corruption.

xref() ownership transfer

VarnodeBank::createDef passes vn to xref() via owned.release(). xref() either stores vn in the trees (line 1334) or delete vn; on duplicate detection (line 1326), so ownership transfers correctly either way.

Audit gate

op.cc + varnode.cc join cppRaiiAudit's PROTECTED_FILES (count: 58 → 60). op.hh + varnode.hh were already in from PR #94.

Test plan

  • C++ unit tests continue to pass.
  • gradle cppRaiiAudit passes; op.cc + varnode.cc now appear in the protected set.
  • The decompiler smoke test in the release pipeline exercises both PCodeOp and Varnode creation extensively.

Proudly Made in Nebraska. Go Big Red! 🌽 https://xkcd.com/2347/

PCode-core paired migration. Both create() overloads in
`PcodeOpBank` and both `VarnodeBank::create` / `::createDef`
follow the same create-then-transfer-into-bank pattern:

  T *thing = new T(...);
  bank.insert(thing);  // ownership transfer
  ... use thing as alias ...
  return thing;

Migrate to:

  auto owned = make_unique<T>(...);
  T *thing = owned.get();
  bank.insert(thing);
  owned.release();  // ownership transferred
  ... use thing as alias ...
  return thing;

Exception-safety improvement in `VarnodeBank::create`: previously
the function did

  vn->lociter = loc_tree.insert(vn).first;
  vn->defiter = def_tree.insert(vn).first;

with no protection — if def_tree.insert threw, loc_tree held a
dangling raw pointer to a leaked Varnode (because vn was leaked
on stack unwind). The new code does `owned.release()` immediately
after `loc_tree.insert` so the cleanup-on-unwind doesn't fire over
a tree-held pointer; if def_tree.insert subsequently throws, the
loc_tree still holds a valid (newly tree-owned) pointer — same
state as before, but with the rare-throws no longer producing
heap-corruption.

`VarnodeBank::createDef` and `PcodeOpBank::create(...)` overloads
follow the same pattern. `xref(owned.release())` is fine: xref
either stores in the trees or `delete vn;` on duplicate detection
(line 1326), so ownership transfers correctly either way.

`op.cc` + `varnode.cc` join `cppRaiiAudit`'s `PROTECTED_FILES`
(count: 58 → 60). `op.hh` + `varnode.hh` were already in from #94.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@CryptoJones
CryptoJones merged commit 76a6ec2 into master May 27, 2026
13 checks passed
@CryptoJones
CryptoJones deleted the feat/rec-31-stage-5-pcode-core-raii branch May 27, 2026 12:25
CryptoJones added a commit that referenced this pull request May 28, 2026
Six raw-`new` sites migrated in the sleigh OpTpl / VarnodeTpl /
HandleTpl decode paths:

  - Two member-assignment sites (OpTpl::decode line 719
    `output = new VarnodeTpl()`, ConstructTpl::decode line 913
    `result = new HandleTpl()`) use `make_unique<T>().release()` —
    the member types stay raw-pointer because the destructors
    manually delete (preserved bit-for-bit).
  - Two `vec.push_back(new T())` loop sites (line 723 VarnodeTpl,
    line 917 OpTpl) use the create-then-transfer-with-named-raw-ptr
    pattern from varnode.cc PR #96: `auto owned = make_unique<T>();
    T *p = owned.get(); vec.push_back(owned.release());
    p->decode(decoder);`.
  - One create-then-addInput pair in fillinBuild (lines 782–783)
    uses `.release()`-immediate on both the OpTpl(BUILD) and the
    synthesized VarnodeTpl constant.

`make_unique` reaches semantics.cc via semantics.hh → slaformat.hh
→ marshal.hh's `using std::make_unique;`. No header changes needed.

semantics.cc joins PROTECTED_FILES. Local `make decomp_test_dbg`
green in 11s.

Protected-set count: 202 → 203.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant