Skip to content

feat: add 26.51 support - #534

Open
wu-vincent wants to merge 34 commits into
v0.11from
feat/26.51-support
Open

wu-vincent wants to merge 34 commits into
v0.11from
feat/26.51-support

Conversation

@wu-vincent

Copy link
Copy Markdown
Member

No description provided.

wu-vincent and others added 25 commits September 15, 2026 20:57
Add an orchestration section to the bump-bds skill: the IDA databases are the
long pole and must be launched first, because almost nothing else in a bump
needs one. The protocol version, the per-class vtable diff, class sizes,
function-start validation and byte-pattern cutting are all reachable from the
shipped binaries with lief alone, so the database is never the thing being
waited on.

Also record two checks that cost real time this bump:

- A scanner of your own must be validated against the previous binary before
  its verdicts on the new one mean anything, and an identity claim belongs on
  a self-naming marker string rather than a windowed string set, which
  overruns into neighbouring functions and convicts correct offsets.
- A Script*GameplayHandler vtable change usually means a new event category,
  not event-variant drift. Compare the hooked slot's discriminant offset and
  its bounded alternative count first; an insertion below the hooked ordinal
  changes nothing. Bound a libc++ visit table by the next rip-referenced
  address, never by the first non-code qword.
Point both symbol configs at 1.26.51 and move SharedConstants to patch 51 /
network protocol 2193.

Mojang published no 1.26.50; the release line went 1.26.45 -> 1.26.51.1.

The protocol version is read out of the comparison in
ServerNetworkHandler::_validateLoginPacket. The method reproduces the known
2169 on 1.26.45 before being trusted on the new binaries, and the compare
idiom mirrors exactly across versions (two sites on Windows, one on Linux),
so 2193 is corroborated independently of the symbol tables.

The regenerated symbol tables are deliberately not part of this commit. A
pattern-only scan against 1.26.51 leaves 19 of 63 Windows entries and 9 of 63
Linux entries unresolved, and at least one entry that does resolve is wrong:
BatchedNetworkPeer::_receivePacket gained a push r14, so the stale pattern
matches two bytes past the function start and would place a hook mid-prologue.
Those land once each entry is re-cut and verified.
Each new pattern is unique in .text and resolves to an address confirmed to be
a real .pdata function start, with the function's identity established from its
body rather than its offset:

- _validateLoginPacket  0x989140  holds the protocol gate (< 2193 -> reason 35,
  != 2193 -> reason 34). The old pattern pinned the frame immediate FE FF FF FF
  literally and drifted onto a different function at 0x989cc0.
- _receivePacket        0xa8f7b0  the function gained a push r14, so the stale
  pattern matched two bytes late and would have placed a hook mid-prologue.
- load@CommandOriginLoader 0x120ddb0  NBT keys OutputReceiver, TargetOther,
  MinecartId, RotationActorId, CommandPosition.
- executeCommand        0x3da4e0  statusCode, successCount, statusMessage,
  commands.generic.invalidOrigin.
- initializeServer      0x72e7a0  single caller, NonOwnerPointer<WorkerPool>,
  ServerInstanceEventCoordinator.

Argument marshalling is unchanged in all five, so no signature changed and the
mangled names still stand; only the byte patterns had gone stale.
BDS 1.26.51 accepts network protocol 2193 only, so the workarounds that let a
1.26.44 client onto a 1.26.45 server no longer have anything to serve. This is
the drop the shims' own TODO(1.26.50) markers were waiting for.

Removed:

- acceptWireCompatibleProtocol and its ProtocolVersionHandler dispatcher, which
  rewrote a declared 2168 to the server's own version on RequestNetworkSettings.
- upgradeLoginPayload, which patched the same override into the Login payload
  before LoginPacket::_read could discard the connection request.
- upgradeSetScorePayload, which re-encoded SetScore for 1.26.44's extra fixed
  byte ahead of RemoveScore's objective name.

RequestNetworkSettingsPacket was added for the first of these and nothing else
referenced it, so it goes with them, along with the includes each shim owned.
Both touched translation units were compiled to confirm the removal is clean.

BREAKING: players must now be on 1.26.51.
Derived from a per-class RTTI vtable diff of the shipped Linux binaries, keyed
by Itanium typeinfo name. Each mapping is anchored, not counted blindly: in
every case Endstone's declared slot total already matched the measured 1.26.45
vtable exactly, which pins the mapping end to end.

- Actor 138 -> 137: canFreeze() removed at slot 85. The removed body was
  `xor eax, eax; ret`, the only signature in that neighbourhood that fits, and
  Mob shows the same deletion at the same index, so it is an Actor virtual.
  Nothing referenced canFreeze but its own declaration.
- CommandOrigin 35 -> 36: one virtual inserted at slot 14, ahead of
  hasChatPerms. Confirmed through VirtualCommandOrigin, whose forwarders call
  the inner origin at [vtable + slot*8]: old slot 14 -> 0x70 maps onto new slot
  15 -> 0x78 and old 15 -> 0x78 onto new 16 -> 0x80, so the whole forwarder run
  shifted by exactly one. This matters because hasChatPerms and hasTellPerms are
  byte-identical, so a fingerprint diff alone could have placed the boundary
  either side of them.
- Item 131 -> 132: one virtual inserted at slot 127, on the public/private
  boundary ahead of _checkUseOnPermissions. ComplexItem shows the insertion at
  the same index.

Both additions are placeholders, marked TODO(fixme) since the real names are not
recoverable from a stripped binary; only slot count and order affect the ABI.

NetEventCallback, AppPlatformListener, PlayerCapabilities::PlayerData,
GameTestLevelListener and AppPlatform also grew, but every insertion lands past
the end of what Endstone declares, so they need no change.
Record that the lief/capstone path is a stopgap for the hours an IDA database is
building, not a preferred technique, and that a hand-rolled scanner's negative
result must not be trusted. Two ways it under-reports silently, both hit this
bump: short string literals are materialized with `movups xmm0,[rip+x]` rather
than `lea`, so a lea-only scan calls them unreferenced and sends you re-anchoring
a locate recipe that was fine; and folding the ModRM byte into the disp32 capture
matches nothing at all.

Also note that a Script*GameplayHandler vtable change usually means a new event
category rather than event-variant drift, and how to bound a libc++ visit table
by the next rip-referenced address instead of the first non-code qword.
Removing the 1.26.44 compatibility shims is internal machinery, so it does not
earn its own entry. The only part a server admin can observe is which clients
can still join, which belongs on the version line itself - matching the form the
1.26.45 entry already uses. The reasoning for the removal stays in its commit.
- Container 45 -> 44: createTransactionContext removed at slot 41. Slots 0-40
  have byte-identical prologues across versions and the tail realigns exactly
  (old 42/43/44 onto new 41/42/43). An independent anchor agrees: the six
  __cxa_pure_virtual slots sit at 3, 8, 13, 21, 22, 23 in both binaries, which
  is precisely Endstone's six pure virtuals. The whole family reproduces from
  the headers with no slack - SimpleContainer 45, PlayerUIContainer 45,
  FillingContainer 54, EnderChestContainer 54, Inventory 56.

- PlayerInventory 5 -> 4: the removed slot is 4, not 3, and the header was
  already wrong before this bump. Slot 3 takes an int and reads the inventory at
  [this+0xb8] in both versions - the same function, merely restructured - while
  slot 4 dereferences +0x20 on two std::function arguments, which is
  createTransactionContext. Endstone had createTransactionContext declared at
  slot 3, so it was mapped onto a different, undeclared virtual. Replacing it
  with a placeholder both mirrors the removal and puts the surviving virtual at
  the right index.

Member layout is unchanged: the Container offsets reachable from the vtable
bodies are identical across versions and PlayerInventory is 0xe8 either side.
Nothing calls or overrides either function, so this is ABI fidelity only.
Part of the version bump; the stub carries the value the CLI version check and
the metrics reporter read.
Both mappings are anchored: Endstone's declared slot total already matched the
measured 1.26.45 vtable exactly (412 for the Level chain, 19 for Random).

- ILevel 412 -> 413: one virtual inserted at slot 347, between isEditorWorld and
  getAllowAnonymousBlockDropsInEditorWorlds. The neighbours settle it - old 346
  and new 346 are the same `cmp dword [rax+0x398], 1; sete al`, the new slot 347
  is the sibling query `cmp ..., 2` on the same LevelData field, and old 347
  realigns onto new 348. ServerLevel shows the identical change and declares no
  virtuals of its own, so the insertion is ILevel's.
- IRandom 19 -> 18: fork() removed at slot 10. Corroborated from the emitted
  code rather than the slot count alone - chanceOneIn tail-calls chance(int,int)
  through [vtable+0x60] in 1.26.45 and [vtable+0x58] in 1.26.51, so the shift is
  visible at a call site. Nothing declared or called fork(); forkPositional
  stays and still needs <memory>.

Core::Random is an empty stub in forward.h, so its own 19 -> 18 needs no change.
All three resolve uniquely to a verified .pdata function start:

- resolve@BlockDescriptor::ResolveHelper 0x2cf6020, holding both deferred
  resolution strings. This entry also carried a stale rip_relative, which is why
  it failed silently rather than loudly - a prologue pattern with rip_relative
  set makes the dumper RIP-decode prologue bytes into a wrong address. Dropped.
- registerAlias@CommandRegistry 0x1247a50, whose only string is its own
  "Register Command <{}> Alias <{}>" log format.
- fromTag@ItemInstance 0x1aa85e0, reached from MerchantRecipe::load's buyA tag
  lookup. Confirmed by decompiling: sret return, loads the tag into a temporary
  ItemInstance, normalizes the 0x7FFF any-data wildcard to 0, copy-assigns into
  the return buffer and destroys the temporary.
…kHandler

1.26.51 gives ServerNetworkHandler a sixth base, declared last, which Endstone
was missing. This is a real break rather than a cosmetic one: the base is an
empty polymorphic interface, so its vptr sits at +80 ahead of every member, and
every declared member offset shifts by 8. server_network_handler.cpp reads
max_num_players_ and app_ directly and core/server.cpp reads network_, so all
three were reading 8 bytes low - a wrong max-player count and a garbage
RemoteConnector, with no compile error to show for it.

The identity comes from RTTI rather than inference: ServerNetworkHandler's
__vmi_class_type_info base array gains `26IPlayerConnectionConnector` at offset
80, the class is new in 1.26.51, and ServerNetworkHandler is its only deriver in
the binary. The destructor corroborates it - 1.26.51 stores a sixth vptr at
[this+0x50] and the same function's member accesses move 0x80->0x88,
0x90->0x98, 0x2c0->0x2c8, 0x3b0->0x3b8.

It contributes 4 slots: the two destructor slots (this-adjusting `add rdi,-0x50`
thunks, which confirm the -80 adjustment) and two member-address getters that
have no callers anywhere, so their names and return types stay placeholders.

sizeof went 1808 -> 2096. The other 280 new bytes land in the undeclared tail
below the last member Endstone models, so no other member needed moving. There
is no size assert on this class to re-derive.
BlockActor gained a third unique_ptr component at +40, so sizeof goes 40 -> 48
and every derived member shifts by 8. The component is
ContainerBlockActorComponent, a name taken from RTTI rather than invented: its
typeinfo is new in 1.26.51 and absent in 1.26.45, and it is the only new
*BlockActorComponent.

The evidence is the inlined BlockActor sub-destructor inside
VanillaBlockActor's D1. The two existing components keep +24 and +32; a third
teardown appears at +40, destroyed first and so declared last, with the same
fingerprint as its neighbours (`mov rdi,[this+N]; test; mov rax,[rdi]; call
[rax+8]`) - a unique_ptr to a polymorphic type, not a unique_ptr -> shared_ptr
widening, which would show an atomic refcount and __on_zero instead. RTTI agrees:
the three IVanilla*BlockActorComponent bases sit at +40/+48/+56 in 1.26.45 and
+48/+56/+64 in 1.26.51.

This one reaches Endstone. The campfire hook reads cooking_item_/cooking_time_
and the piston hook reads state_/new_state_, all of which would have been 8 bytes
low with no compile error. Sized-delete confirms every derived class is +8 and
nothing else: VanillaBlockActor 208 -> 216, PistonBlockActor 312 -> 320,
CampfireBlockActor 744 -> 752, BeehiveBlockActor 240 -> 248, FurnaceBlockActor
1264 -> 1272.

BEDROCK_STATIC_ASSERT_SIZE(BlockActor, 40, 40) had been passing and reading as
verified; it is now 48/48 and the compiler checks it on both ABIs. Two offset
comments on vanilla_block_actor.h and piston_block_actor.h were already wrong
before this bump and are corrected - the declarations themselves were right and
sum to the measured sizes.
…bles

- LevelEventListener: four virtuals inserted immediately before onEvent. The
  base's default implementations are emitted in a monotone 0x10-strided run, and
  onEvent - the distinctive variant visitor - moves from slot 11 to 15 while
  ScriptLevelGlobalEventListener's overrides at 3,4,5,6,9 stay put and its 13,14
  move to 18,19. The clincher is ScriptWorldClockGlobalEventListener, new in
  1.26.51, which overrides exactly the four new slots. Endstone declares onEvent
  at slot 11, matching 1.26.45 exactly, so the insertion lands inside the
  declared run. A fifth virtual was appended past onEvent, in the undeclared
  tail, and needs nothing.

- IResourcePackRepository: three virtuals inserted at slots 48, 50 and 51,
  around the getInvalidPacks overloads. Bounded on both sides by hard anchors -
  forEachPack at 46 is byte-identical, and deletePack shifts 49 -> 52 with its
  own vcalls following it. The IContentSourceRepository sub-vtable settles it
  independently: its this-adjusting thunks for deletePackFiles and
  postDeletePack point at primary slots 50/51 in 1.26.45 and 53/54 in 1.26.51.

BaseCircuitComponent and RemoteConnector were reported as changed by the vtable
sweep and are NOT: both readings were the run-walker over-running into an
adjacent RTTI-less pointer table. Every class deriving from BaseCircuitComponent
measures 29 slots in both versions, and a derived vtable cannot be shorter than
its base, so the 55 was never real. Neither header is touched.
The Windows table now scans clean: 63/63 resolved, no errors and no ambiguous
patterns. Every offset was confirmed by reading the function, not by trusting a
byte pattern that happened to match.

Newly located this pass, each corroborated a second way:

- fromTag@ItemInstance 0x1aa85e0 - normalizes the 0x7FFF any-data wildcard
- _loadMapData 0x858550 - getSerializationKey, LevelStorage::loadData, then
  operator new(176) for the MapItemSavedData
- createSources@RepositoryFactory 0xc7e870 - reached through the factory vtable
  slot 1 the ResourcePackRepository ctor calls after operator new(8)
- tryGetStateFromLegacyData 0x1a21750 - CommandItem::createInstance is a caller,
  and its prologue takes the u16 via movzx r14d, dx
- ServerScoreboard::ServerScoreboard 0x1ffab0 - DisplayObjectives, LastUniqueID,
  NonOwnerPointer<GameplayUserManager>, single caller
- lookupByName@ItemRegistry 0x162c220, updateWeather@WeatherManager 0x119f140,
  createBlockDescriptor 0x426c7b0, disconnectClientWithMessage 0x972f40,
  tick@PistonBlockActor 0x4872840, startSleepInBed 0x2a24a0,
  updateServerAnnouncement 0x984130, completeUsingItem 0x2a86b0

forEachBlockType needed a 271-byte pattern: BlockTypeRegistry emits two
byte-identical copies differing only in rel32s, so nothing shorter separates
them. The live one has 13 callers, the dead copy 1.

The four entries that resolved but reported ambiguous - registerOverloadInternal,
symbolToString, start@DedicatedServer and knockback@Mob - were each taking the
correct address already (0.87/0.88/0.75/0.98 body similarity against 1.26.45,
versus 0.13-0.25 for every rival), but first-match is only luck of link order, so
all four were grown to unique.

Verified: 60 entries land on a real .pdata function start, 2 are data globals
which have none by definition, and randomTick@LeavesBlock is the documented
tail-call thunk that has no .pdata record in either version.
The Windows warning about packed vftables applies verbatim to Linux
.data.rel.ro. When the object after a vtable is an RTTI-less function-pointer
table - an entt-meta or cereal type-erased manager - a run-length walk sails
through it and reports a class as far longer than it is. The terminator is the
next vtable's own header, a zero offset_to_top followed by a typeinfo pointer.

The cheap cross-check is the derived classes: a derived vtable can never be
shorter than its primary base's, so if every deriver measures 29 in both
versions, the base's 55 is the artifact. 1.26.51 produced two false positives
this way, BaseCircuitComponent reading 55 -> 29 and RemoteConnector's -56 group
reading 22 -> 20; both classes were entirely unchanged.
Two member-layout breaks found by running the server against 1.26.51 and
following the crash. Neither is visible to a vtable diff, and neither would have
been caught by a size assert - Minecraft's sizeof is unchanged at 560.

- Minecraft: one pointer member was added ahead of save_game_path_, so every
  later member moved 8 bytes. getLevel() read game_session_ at +0xC0 instead of
  +0xC8, found a neighbouring object, and returned null; onServerThreadStarted
  then dereferenced the null Level and took an access violation on address 0.
  Pinned from the live object: the std::string SSO capacity field (0x0F) puts
  save_game_path_ at +0x70, and max_player_idle_time_ reads 0x708 - 1800
  seconds, the idle timeout - at +0xB0, which fixes everything between them.

- MapDataManager: 1.26.51 appends members, taking it from 200 to 408 bytes on
  Windows and 160 to 280 on Linux. Endstone reads none of them, but
  ServerMapDataManager derives from MapDataManager and
  IServerMapDataManagerConnector, so the second base - and the virtual
  getOnCreateMapSavedDataConnector call through it - lands at sizeof of the
  first. EndstoneServer::setLevel crashed on that call.

  The 208/120 split across platforms identifies the kinds without needing their
  names: an unordered_map is 64 bytes on MSVC and 40 on libc++, a std::function
  64 and 32, so 16 bytes of PODs plus one map and two functions is the only
  combination that gives both numbers. The offset of map_data_ is unchanged at
  112, confirmed against the live object, so nothing Endstone reads moved.

The size assert is new and now guards both platforms. With these, the server
reaches the scheduler and ticks; the remaining crash is in the devtools item
dump and is tracked separately.
BDS changed WeakPtr's representation: the stored pointer is now the object
itself, not a SharedCounter. Endstone still read `pc_->ptr`, which returns the
object's vtable pointer, so every WeakPtr dereference produced a pointer into
read-only .rdata.

The first casualty was devtools' item dump, which is not optional - conanfile
enables DevTools for every Windows build. ItemStackBase::init took the block
branch and called BlockType::tryGetStateFromLegacyData on the vtable address;
AcquireSRWLockShared then tried to write a lock into .rdata. The minidump names
it exactly: ExceptionInformation [1, 0x7ff75a148340], a write, and that address
is the object's vtable RVA plus 0x260, the lock's offset within BlockType.

Three independent readings agree the stored pointer is the object:
- Dereferencing it gives a vtable whose slots point into BlockType's own method
  cluster, and one field further along is a string reading "tile.red_concrete" -
  a block name, so the target is a BlockType and not a 24-byte control block.
- bedrock-headers confirms SharedCounter was {ptr, share_count, weak_count} with
  ptr first as recently as 1.26.40, so Endstone's old model was correct then and
  the representation changed since.
- Fixing only Item::block_type_ as a raw pointer moved the failure to a Control
  Flow Guard fast-fail (FAST_FAIL_GUARD_ICALL_CHECK_FAILURE), which is what a
  still-broken WeakPtr<Item> produces on its next virtual call. The change is in
  the template, not in one member.

There is no counter to maintain any more, so the copy and reset paths no longer
touch a refcount; that is also safer than writing through a wrong offset.
The Linux table now scans clean: 63/63, no errors and no ambiguous patterns.
Verified independently - 60 offsets land on an .eh_frame_hdr FDE initial_location
and the remaining 3 are data globals in .bss, which have no FDE by definition.

Thirteen entries were fixed: nine that no longer matched, and four ambiguous ones
whose taken address was already correct but which needed lengthening so a future
build cannot silently pick a different first match.

Every address is corroborated by a signal independent of instruction similarity -
a string anchor, a call-graph edge, a caller-count fingerprint, an RTTI vtable
slot, or the argument shape in the prologue.

MapDataManager::getMapSavedData needed real work: 1.26.51 split it, factoring the
load-and-insert tail into a separate helper, so no whole-body match could find it.
It was recovered by matching the old function's tail against the new helper and
its head against the helper's caller. That entry had no locate recipe and now
carries one recording the split.

Three recipes had premises that were already stale and are corrected:
startSleepInBed's anchor string does not exist in 1.26.45 either, so it is now
located by RTTI vtable slot; updateServerAnnouncement is not called by
_onPlayerLeft in either version, it merely follows it in .text; and
createSources is vtable slot 2, not 1.

Five entries moved from call-site to direct prologue patterns, each with its
rip_relative and rip_offset lines removed - leaving them would make the dumper
RIP-decode prologue bytes into a silently wrong address.
Reverts the WeakPtr change in 9a2c8cc, which was wrong, and fixes the actual
member instead.

9a2c8cc concluded from one member that WeakPtr's representation had changed
release-wide. It had not. Iterating ItemRegistry's name-to-item map settles it:
the map is unordered_map<HashedString, WeakPtr<Item>>, its keys read back
correctly - "minecraft:red_concrete_slab" - and the pointer each WeakPtr<Item>
stores is an array of 32-byte records shaped {ptr, share_count=1, weak_count=8},
which are SharedCounters. WeakPtr<Item> still holds a control block, so the
existing model was right and changing the template broke every other use of it.

What actually changed is Item::mBlockType alone, which now stores the BlockType
directly. The evidence for that member is unchanged and still stands: the stored
pointer's first qword is a vtable whose slots land in BlockType's own method
cluster, and a field further in is a string reading "tile.red_concrete".

Confirmed by how the two versions fail. With the template changed, the map's
values became garbage and the devtools dump died reading an item's name. With
only this member changed, that dump gets past the block branch entirely.
getPacksByResourceLocation and friends walk the repository's pack records
and, in 1.26.45, read the manifest as [pack+8] -> Pack* -> manifest. In
1.26.51 the same code reads [[pack+0]+8] -> Pack* -> manifest: the object
handed out is now 8 bytes holding a pointer to what used to be the class
body, which still has the old layout.

Endstone's getManifest() dereferenced the old offset, so every pack in
resource_packs with PackOrigin::User faulted at startup.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
InternalItemDescriptor, ItemTagDescriptor, MolangDescriptor,
DeferredDescriptor and ComplexAliasDescriptor all went from 18 to 19
vtable slots, with the new entry landing between toString and getItem -
callers that used to reach getItem through [vptr+0x28] now use
[vptr+0x30].

Endstone's ItemDescriptor::serialize(Json::Value&) dispatched through
the old slot and hit a CFG fast-fail as soon as the devtools dump
serialized its first recipe ingredient. The new slot defaults to
getFullName(), which is what BDS's own base implementation does.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ctors and fast-fail bisect

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Actor lost canFreeze and Player gained a bool getter in the same release,
so Player and ServerPlayer measure 246 and 248 slots in both versions and
never showed up in a vtable-length diff. Everything from the new slot on
was one short: sendNetworkPacket dispatched to
sendComplexInventoryTransaction and faulted inside BDS as soon as a player
joined and the join message was broadcast.

The new slot sits between isTeacher and onSuspension and reads the byte at
this+0xCBE, right next to isTeacher's this+0xCBD.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
PlayerCapabilities::PlayerData went from 5 to 6 vtable slots; the new one
is appended after isOperator and forwards to the virtual Player itself
gained in the same release.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@wu-vincent
wu-vincent changed the base branch from main to v0.11 September 16, 2026 00:31
The five appended members were inferred from the 208/120 per-platform size
delta on the assumption that libc++ lays std::function out in 32 bytes. It
does not: aligned_storage<3*sizeof(void*)> picks a 16-byte alignment, so the
buffer is 32 and std::function is 48. The declared members therefore summed to
320 on Linux against an asserted 280, and the branch did not compile - CI was
red on this line.

Endstone reads none of these members; only the size matters, because
ServerMapDataManager's second base sits after them. Replace them with a byte
array sized per platform, which is also the honest model given the kinds are
not known.

The 280 in the assert was the offset of that base, not sizeof. 1.26.51's
__vmi_class_type_info for ServerMapDataManager records
IServerMapDataManagerConnector at 280, and get_new_unique_id_ forces a 16-byte
alignment, so the object is 288 with the base placed in its tail padding.
Confirmed with -fdump-record-layouts: the base now lands at 280.
BDS orders the lock no_thread_id_, owner_ref_count_, owner_thread_; Endstone
declared the empty thread_hasher_ first, which pushed owner_ref_count_ to +4
and no_thread_id_ to +8. Both layouts are 24 bytes, so the size assert could
never catch it.

lock() seeds its compare_exchange with no_thread_id_, so it read four bytes of
the reference count instead, the exchange never matched an unowned lock, and it
spun forever.

Latent until now: 1.26.51's createBlockDescriptor returns a deferred
descriptor, so tryGetBlockNoLogging takes the locking path for the first time
and every Server::createBlockData wedged the server thread at 100% CPU.

Read off BDS's own inlined lock, which loads no_thread_id_ from the lock's
first eight bytes, stores the reference count as a dword eight bytes in, and
targets owner_thread_ sixteen bytes in with its cmpxchg. [[no_unique_address]]
keeps the stateless hasher out of the layout.
operator== took the address of the NonOwnerPointer member rather than of the
objective it points at, so two handles onto the same objective never compared
equal. It read correctly when the member was a reference and was missed when
that changed.

Every lookup hands back a fresh handle, so `objective in scoreboard.objectives`
was always false.
MutableLevelGameplayEvent<CoordinatorResult> gained a second alternative, which
moves its discriminant from +0x18 to +0x28, and ScriptModuleStartupEvent gained
a sixth registry reference. Both read off the 1.26.51 Linux binary: the visit
table behind ScriptLevelGameplayHandler's handleEvent2 now holds two thunks
whose ValueOrRef payloads measure 16 and 32 bytes, and the startup event's
payload measures 48 where it measured 40.

Endstone reads neither, so both are placeholders sized to match; the sizes are
what the ABI depends on. Confirmed with -fdump-record-layouts: the variant's
index now sits at 40, which is the offset BDS loads it from.
clang-cl rejects [[no_unique_address]] as an unknown attribute and ignores it,
which puts the empty hasher back in the layout: the lock grows to 32 bytes and
no_thread_id_ returns to +8, reinstating the spin the previous commit fixed, on
Windows only and with no diagnostic beyond a warning.

Use the [[msvc::no_unique_address]] spelling there, matching brstd's existing
NO_UNIQUE_ADDRESS macro. Checked both targets against a freestanding repro:
24 bytes with no_thread_id_ at 0, owner_ref_count_ at 8 and owner_thread_ at 16.
The visitor called handleEvent on whatever alternative it was given, which
compiled only while MutableLevelGameplayEvent<CoordinatorResult> held exactly
one. 1.26.51 gives it a second, so guard on the type the way the actor, block
and scripting handlers already do.
__iter__ is part of the published API but could never run. It appended each
entry by value, which throws for every non-copyable registry type, and it
returned a list rather than an iterator, so `for block_type in registry` would
have raised TypeError even once the copy was fixed.

Cast each entry as a reference, matching __getitem__, and return an iterator
over them. This is the behaviour develop already has; the rewrite that
introduced it there has not reached this line.

Iterating the block registry now yields all 1477 block types and the item
registry all 2076 item types.
…its factor data

BDS puts FactorCalculationData at MobEffectInstance+48. Endstone left eight
bytes out ahead of it, which libc++ hid: the factor data holds a std::function
and so aligns to 16 there, landing on 48 with or without them. MSVC aligns it
to 8, so the omission put it at 40 and made every enclosing type eight bytes
short.

ActorAddEffectEvent is the largest alternative of
MutableActorGameplayEvent<CoordinatorResult>, so its size sets the variant's
storage and therefore where the discriminant sits. Endstone read _Which at 168
while BDS wrote it at 176, which threw bad_variant_access out of
ScriptActorGameplayHandler::handleEvent4 the first time a covered event
dispatched - that is, once a player joined, not during the network handshake.

Read out of the 1.26.51 Windows binary: the dispatch loads the discriminant
from +176 and hands _Which+1 to an extractor whose eight cases test their
ValueOrRef flags at 48, 168, 48, 16, 24, 16, 16 and 48. Only the second
disagreed with Endstone, at 160. The model now measures 144 for
MobEffectInstance, 168 for ActorAddEffectEvent and 176 for the discriminant,
matching all four numbers. Linux is untouched: all 24 alternatives of the
sibling variant still match, and the index stays at 176.
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