Skip to content

fix(labels): repair and accelerate Delphi recovery - #424

Merged
danielplohmann merged 6 commits into
danielplohmann:masterfrom
r0ny123:fix/delphi-recovery-audit
Oct 29, 2026
Merged

danielplohmann merged 6 commits into
danielplohmann:masterfrom
r0ny123:fix/delphi-recovery-audit

Conversation

@r0ny123

@r0ny123 r0ny123 commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

The DelphiReSym port miscomputed class RTTI offsets and misread some published/extended method tables, leaving usable names unrecovered. This fixes byte-based cursors, record bounds, Unicode names and executable targets, while preserving valid names when optional signature data is damaged. It also shares provider scans and recovers validated 32-bit Delphi finally continuations.

The finally matcher rejects implicit flag-stack and push/pop-all operations, as well as partial SP operations, before inferring the saved continuation. Nine Capstone-decoded regressions demonstrate the rejected cases. Finally recovery intentionally changes CFGs and fingerprints.

Validation: against master (bfe39ec), complete reports are bit-identical on 18 bundled non-Delphi fixtures (cutwail, asprox, komplex, bashlite, mirai x86/x64, the msvc/rust/go/cxx PE and ELF fixtures, and both PE+PDB pairs with and without the PDB). The full suite passes (2,691 passed, 2 skipped), as do ruff check, ruff format --check and ty.

The earlier metadata and Ghidra 12.2 audit, its dated measurements, real metadata fixtures, and reproduction commands are in profiling/delphi_audit/. Those external accuracy/performance measurements were not rerun for this stack-guard follow-up. The 64-bit HeidiSQL Ghidra comparison disables Create Address Tables after stock analysis exhausted a 6 GB heap; the audit records that limitation.

@r0ny123 r0ny123 changed the title labels: repair and accelerate Delphi recovery fix(labels): repair and accelerate Delphi recovery Oct 26, 2026
Reject flag-stack and push/pop-all instructions, and writes involving SP,
before inferring a saved Delphi finally continuation. Add nine cases
decoded by Capstone and correct the audit output link.

Validation: 171 focused tests; eight control report hashes unchanged.
@r0ny123

r0ny123 commented Oct 29, 2026

Copy link
Copy Markdown
Contributor Author

Added the stack-effect guard and nine Capstone-decoded regressions for flag-stack, push/pop-all and partial-SP operations. The continuation matcher now rejects these cases before adding a CFG edge.

Validation on 1ffc13f: 171 focused tests passed, and complete report hashes are unchanged on eight bundled control fixtures. CI, fuzzing, the fixture benchmark, security and changelog workflows all passed.

@danielplohmann danielplohmann left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this is a substantial piece of work. I checked it against master: the slow tier passes, and on 16 non-Delphi fixtures (cutwail, asprox, komplex, mirai, the msvc/rust/go/cxx PE and ELF fixtures, PE+PDB with and without the PDB) the report JSON is byte-identical to master. The ReSym loops are all bounded now, and the finally continuation is gated tightly enough that I have no concerns about it. A few things before merging:

1. The PDB pass change is not Delphi-specific, and the CHANGELOG describes it differently.
On master, _runLabelProviderUpdate(pdb_info) re-ran every provider on the empty PDB BinaryInfo, which cleared PeSymbolProvider whenever a PDB was supplied. With pdb_only=True that stops: on msvc_cxx_pdb_pe with its PDB, master ends up with 0 PE symbols and this PR keeps 3. Because USE_SYMBOLS_AS_CANDIDATES is on by default, PE exports and the OEP are candidates again on any PE+PDB run, so the function set can grow wherever the PDB does not already name an export. The fix itself is right, but the entry should say so: "PE export and entry-point labels (and their candidates) are no longer dropped when a PDB is supplied", not "preserve Delphi labels". Against master the PDB already won, because the PE labels had been wiped.

2. Delphi detection got looser, which the entry does not mention.
LanguageAnalyzer._computeDelphiScore now raises the score to 0.6 whenever ReSym finds any VMT, which clears checkDelphi()'s 0.5 and turns on the Delphi candidates and the finally recovery. Together with DelphiPythiaProvider dropping _validate_metadata_addresses, and _read_pascal_string decoding cp1252, so that 0x80-0x9F now count as printable, a non-Delphi image is somewhat more likely to be classed as Delphi. None of the bundled non-Delphi fixtures contain TObject, so the tests do not exercise this. Could you either justify the 0.6 floor with a measurement over non-Delphi samples, or require more than one VMT? Either way, please give it a line in the CHANGELOG.

3. Smaller CHANGELOG gaps

  • MAX_DELPHI_METADATA_ENTRIES is a new config option and belongs under Added.
  • The (intel) bullet has no cost statement. "Bit-identical on the non-Delphi fixtures" would be accurate.
  • The "report hashes match" claim in the description compares against the PR's own earlier head, not master. Please state it against master.

4. Attribution for tests/delphi_corpus
The Inno Setup licence is covered by INNO-LICENSE.txt. The excerpts themselves (TObject's VMT, the System RTTI, the _HandleFinally sequence), though, look like Delphi RTL code compiled into Inno rather than Jordan Russell's own, and Embarcadero is not credited. It is a tiny, mostly structural amount, and tests/ is pruned from the sdist, but please add a line to the corpus README and to NOTICE, as NOTICE already does for the vendored rust_demangler.

Also heads-up: this conflicts with #416 in CHANGELOG.md, so whichever lands second needs a rebase.

@r0ny123

r0ny123 commented Oct 29, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for checking it against master. Pushed 80cd527, which merges master (the CHANGELOG conflict with #414 is resolved there) and covers all four points:

  1. The PDB change has its own Fixed entry, worded as you suggested: PE export and entry-point labels (and their candidates) are no longer dropped when a PDB is supplied. On msvc_cxx_pdb_pe with its PDB that keeps 3 PE symbols instead of 0. The function sets of msvc_cxx_pdb_pe and rust_pe_msvc_i686 with their PDBs do not change.
  2. I went with requiring more than one VMT. The ReSym floor now needs at least two validated VMTs (DelphiReSymProvider.getVmtCount()), and a new test checks that a single-VMT image does not pass checkDelphi(). A real image has one VMT per class, so only single-VMT images lose the floor. The looser detection (the floor, Pythia without _validate_metadata_addresses, cp1252 Pascal strings) has a Changed entry.
  3. MAX_DELPHI_METADATA_ENTRIES is under Added. For the (intel) cost I reran the comparison against master (bfe39ec) on 18 bundled non-Delphi fixtures, including both PE+PDB pairs with and without the PDB: complete reports are bit-identical, and the entry says so. The description now states the comparison against master instead of against the earlier head.
  4. The Embarcadero RTL attribution is in the corpus README and in NOTICE.

I merged master instead of rebasing, so whichever of this and #416 lands second needs one more merge for the CHANGELOG.

@danielplohmann
danielplohmann merged commit 4be15dc into danielplohmann:master Oct 29, 2026
30 checks passed
@r0ny123
r0ny123 deleted the fix/delphi-recovery-audit branch October 29, 2026 20:08
@danielplohmann danielplohmann added this to the v5.0.2 milestone Oct 30, 2026
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.

2 participants