Black/GJPersistent.cpp: Matching -- carve ??_R0 into the TU split by Marklepolo · Pull Request #157 · openblack/bw1-decomp

Marklepolo · 2026-06-13T15:25:18Z

The Persistent base class is defined in GJPersistent.cpp, confirmed by the FILE string baked at .data:0x00BEF87C which this unit's assert path emits. Lift its four virtuals (VirtualFunc / OnLoaded / DefineProperties / ~Persistent), all no-ops, and carve the unit's RTTI type descriptor ??_R0?AVPersistent@@ into GJPersistent's own .data split (0x00BEF860..0x00BEF8A0, covering the ??_R0, the FILE string, and trailing pad up to the next TU).

The object now DEFINES its own ??_R0 (dumpbin: SECT, not UNDEF) rather than having it resolved from another translation unit -- per review feedback on the earlier scope:weak approach.

exact_gate (relocs masked): all functions byte-exact. objdiff: the four methods at 100%.

bwrsandman · 2026-06-13T15:36:59Z

@@ -0,0 +1,28 @@
+#include "Persistent.h"
+
+// TU GJPersistent.cpp (confirmed by the __FILE__ string baked at .data:0x00BEF87C, which the


Don't need these comments. All details should be in the PR description

bwrsandman · 2026-06-13T15:44:40Z

Still failing tests

bwrsandman · 2026-06-13T16:41:42Z

+    # has no body, so the link fails on it. Redirect ??_E -> ??_G: the vtable then relocates to the
+    # real ??_G, byte-identical to retail. One entry per polymorphic Matching class (Persistent is
+    # the first). See docs / memory baw-matching-tu-link-fixes.
+    "/alternatename:??_EPersistent@@UAEPAXI@Z=??_GPersistent@@UAEPAXI@Z",


I might have to fix lld for this

Black/GJPersistent.cpp: Matching

Lifts the Persistent base class as a byte-exact Matching TU.

TU identity — the source file is GJPersistent.cpp (it defines class Persistent), confirmed two ways: the Windows FILE string baked at .data:0x00BEF87C, and the Mac symbol __sinit_GJPersistent_cpp (the per-file static initializer) in the CodeWarrior build. The four methods (VirtualFunc, OnLoaded, DefineProperties, ~Persistent) match the Mac symbols by name.

??_R0 carved into the split (per review) — the RTTI type descriptor ??_R0?AVPersistent@@ is defined by this TU via a .data split carve 0x00BEF860..0x00BEF8A0 (the even-aligned block: ??_R0 + the FILE string + pad, up to the next TU's ??_R0). The object now defines its own descriptor (DUMPBIN /SYMBOLS shows SECT, not UNDEF) rather than borrowing it via scope:weak.

Link — as the first polymorphic Matching TU, this is the first to hit our c1xx emitting the vector deleting dtor ??_E in the vtable's dtor slot where retail used the scalar ??_G (which we define byte-exact). ??_E has no body, so the full link (--map) needs /alternatename:??_EPersistent@@UAEPAXI@Z=??_GPersistent@@UAEPAXI@Z (added to config.ldflags).

Verification — exact_gate (relocs masked): all functions byte-exact; objdiff: the four methods 100%.

I force pushed changes and the above is standing in for the comments, but yes, right now it seems I cannot get any func to 100% matching with out ldflag hacks, c1xx and the original retail just won't emit the same idioms

I thought I had done experiments prior that solved this issue just by using different compiler flags

which ones? I have big trouble now with dtors that have a run-once guard, but different flags did not work

Yeah that's the tricky part

This is most likely wrong and the declaration of the virtual destructor in the header probably never was explicitly written.
The vector detector is probably implicitly added due to a global static array of a subclass of Persistent.
The real destructor might be unlabeled still.
This is the case for the Base class

Fixed in #172 so you can remove this flag

bwrsandman

Needs equivalent with 1.0 and 1.1

Marklepolo · 2026-06-13T18:13:40Z

Needs equivalent with 1.0 and 1.1

1.1 I will push

1.0 folds the no-op virtuals (VF=OL=0x4017F0, DP=0x4017C0); only ??_G is unique. How do you want folded virtuals represented in a Matching unit — scope:weak COMDATs, given you rejected scope:weak for the ??_R0?"

bwrsandman · 2026-06-13T18:19:17Z

1.0 is the only version that used /OPT:REF so maybe that's why you see folding. Maybe there's another undiscovered /OPT:

Marklepolo · 2026-06-13T18:36:29Z

1.0 is the only version that used /OPT:REF so maybe that's why you see folding. Maybe there's another undiscovered /OPT:

OPT:ICF hat's massive identical-function folding`, maybe?

Marklepolo · 2026-06-14T07:04:20Z

Lift Persistent (Matching, byte-exact). Real ~Persistent frees the refcounted name-string at this+8; added the members + a PersistentName COW-string struct so the compiler emits the real dtor + matching ??_G/vtable/RTTI. No manual split — symbol-driven placement. /alternatename ??_E→??_G (our cl emits the vector deleting dtor). Verified byte-exact: dtk shasum -c config/BW1E142/build.sha1 → all files OK.

bwrsandman · 2026-06-14T11:37:52Z

+    char* data;
+    int length;
+    int capacity;
+


The convention is to include a comment above every function in a header with the address on windows (only version 1.41) and on mac.

You can look at any other header to see examples but it is in this format exactly:

// win1.41 {08x:win_addr} mac {08x:mac_addr} {mac_function_name_demangled} // win1.41 0041a2b0 mac 1007c6d0 Animal::SetStateSpeed(void)

needs to be done for every function in every header.

bwrsandman · 2026-06-14T11:38:35Z

+
+struct PersistentName
+{
+    int field_0;


No clue what this is?

bwrsandman · 2026-06-14T11:38:48Z

+#ifndef BW1_DECOMP_GJPERSISTENT_INCLUDED_H
+#define BW1_DECOMP_GJPERSISTENT_INCLUDED_H
+
+void __cdecl operator delete(void* block);


Needs to be in its own header and I don't think block is the right name.

bwrsandman · 2026-06-14T11:40:24Z

+            char* shared = data - 1;
+            char count = *shared;
+            if (count == 0 || count == -1)
+            {
+                ::operator delete(shared);
+            }
+            else
+            {
+                *shared = count - 1;
+            }


This looks like custom memory allocator code and I doubt that this is unique to this class.

bwrsandman · 2026-06-14T11:41:04Z

+    PersistentOwner* m_owner;
+    PersistentName* m_name;


We don't use the m_ convention. This is invented.

bwrsandman · 2026-06-14T11:41:24Z

+    virtual ~Persistent();
+    virtual void DefineProperties(PropertyList* param_1);
+
+private:


No reason to use private

bwrsandman · 2026-06-14T11:43:52Z

+class FloatProvider : public Persistent
+{
+public:
+    virtual ~FloatProvider();
+    virtual void DefineProperties(PropertyList* param_1);
+};


This class declaration doesn't have any definition in this PR. Why is it there?

The real ~Persistent frees a refcounted name-string member at this+8, so give Persistent its members and a PersistentName COW-string struct. The compiler then emits the real ~Persistent (set vptr; delete m_name -> refcount the buffer at data[-1], free or decrement; zero data/length/capacity) plus the matching ??_G, vtable and RTTI. No manual split needed -- symbol-driven placement is correct. ldflag /alternatename ??_E -> ??_G (our cl emits the vector deleting dtor). Verified byte-exact: dtk shasum -c config/BW1E142/build.sha1 -> all files OK.

bwrsandman requested changes Jun 13, 2026

View reviewed changes

bwrsandman reviewed Jun 13, 2026

View reviewed changes

bwrsandman requested changes Jun 13, 2026

View reviewed changes

Marklepolo force-pushed the pr-persistent branch from 6bb7d56 to 1510557 Compare June 13, 2026 16:58

Marklepolo marked this pull request as draft June 13, 2026 19:03

Marklepolo force-pushed the pr-persistent branch 2 times, most recently from 34fc1eb to a00d89f Compare June 14, 2026 07:03

Marklepolo marked this pull request as ready for review June 14, 2026 07:03

Marklepolo requested a review from bwrsandman June 14, 2026 07:36

bwrsandman requested changes Jun 14, 2026

View reviewed changes

Marklepolo marked this pull request as draft June 14, 2026 12:13

Marklepolo force-pushed the pr-persistent branch 3 times, most recently from 4f5a128 to 2dfba21 Compare June 20, 2026 19:25

Marklepolo force-pushed the pr-persistent branch from 2dfba21 to da9f06e Compare June 21, 2026 08:24

bwrsandman changed the base branch from dtk-template to main June 27, 2026 11:49

		@@ -0,0 +1,28 @@
		#include "Persistent.h"

		// TU GJPersistent.cpp (confirmed by the __FILE__ string baked at .data:0x00BEF87C, which the

Uh oh!

Conversation

Marklepolo commented Jun 13, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bwrsandman commented Jun 13, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Marklepolo Jun 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bwrsandman left a comment

Choose a reason for hiding this comment

Uh oh!

Marklepolo commented Jun 13, 2026

Uh oh!

bwrsandman commented Jun 13, 2026

Uh oh!

Marklepolo commented Jun 13, 2026

Uh oh!

Marklepolo commented Jun 14, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Marklepolo Jun 13, 2026 •

edited

Loading