[GEN][ZH] Fix uninitialized memory read in DockUpdate::loadDockPositions

[Mauller]

This fix was originally discovered by @helmutbuhler and split out of the original PR it was contained in.

This fix initiallises an array that is used immediately but not all of the bins are updated with data.
This can result in random contents being included instead of being zeroed.

• Relates [ZH] Minor memory corruption and leak fixes #716

[feliwir]

Maybe add helmutbuhler as Co-Author when he participated? See https://docs.github.com/en/pull-requests/committing-changes-to-your-project/creating-and-editing-commits/creating-a-commit-with-multiple-authors

[Mauller]

Updated with requested changes, just needs rebasing once main is confirmed okay after the hashmap reversion and testing for Compat issues now.

[Mauller]

Rebased with main and pushed so ready for testing.

[sorcerer86pt]

add a replay mismatch in vc6 version, but worse than that. launching any replay then pressing exit game, gives a immediate crash to desktop:

[helmutbuhler]

add a replay mismatch in vc6 version, but worse than that. launching any replay then pressing exit game, gives a immediate crash to desktop:

Can you give some more details? I just tested the build from CI and it works fine for me.

[sorcerer86pt]

Simply I used VC6 one, started the golden replay then exited the game. It immediately crashed. I have a Ryzen cpu A quarta, 23/04/2025, 22:38, helmutbuhler ***@***.***> escreveu:

…

*helmutbuhler* left a comment (TheSuperHackers/GeneralsGameCode#731) <#731 (comment)> add a replay mismatch in vc6 version, but worse than that. launching any replay then pressing exit game, gives a immediate crash to desktop: Can you give some more details? I just tested the build from CI and it works fine for me. — Reply to this email directly, view it on GitHub <#731 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AC2N4ZYPFFEIZ6VLBOMZ6U323AB4VAVCNFSM6AAAAAB3P7F752VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDQMRVGU2TMMBSGU> . You are receiving this because you commented.Message ID: ***@***.***>

[helmutbuhler]

Does the binary from CI crash for you as well?
You'll find it here: https://github.com/TheSuperHackers/GeneralsGameCode/actions/runs/14611589339/artifacts/2991550194

I have a Ryzen too, but it doesn't crash for me.

[sorcerer86pt]

It was the binary from the ci A quinta, 24/04/2025, 08:57, helmutbuhler ***@***.***> escreveu:

…

*helmutbuhler* left a comment (TheSuperHackers/GeneralsGameCode#731) <#731 (comment)> Does the binary from CI crash for you as well? You'll find it here: https://github.com/TheSuperHackers/GeneralsGameCode/actions/runs/14611589339/artifacts/2991550194 I have a Ryzen too, but it doesn't crash for me. — Reply to this email directly, view it on GitHub <#731 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AC2N4Z535VYRZN35I7ETVET23CKOPAVCNFSM6AAAAAB3P7F752VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDQMRWG4YDOMJQGE> . You are receiving this because you commented.Message ID: ***@***.***>

[helmutbuhler]

Can you please download the binary from my link and try again?

[sorcerer86pt]

Ok A quinta, 24/04/2025, 11:30, helmutbuhler ***@***.***> escreveu:

…

*helmutbuhler* left a comment (TheSuperHackers/GeneralsGameCode#731) <#731 (comment)> Can you please download the binary from my link and try again? — Reply to this email directly, view it on GitHub <#731 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AC2N4Z3NJVBEQPO6GN24ZDT23C4NBAVCNFSM6AAAAAB3P7F752VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDQMRXGEZDONRQHE> . You are receiving this because you commented.Message ID: ***@***.***>

[sorcerer86pt]

it did crash again
used ff

[sorcerer86pt]

https://streamable.com/qrxjro

[sorcerer86pt]

how to reproduce, load a big replay and ff it. wait some time (20~30s should suffice). then exit game

[roossienb]

how to reproduce, load a big replay and ff it. wait some time (20~30s should suffice). then exit game

I get this error in the main branch as well, without this PR.

[helmutbuhler]

how to reproduce, load a big replay and ff it. wait some time (20~30s should suffice). then exit game

Thanks! I was just about to post as @roossienb that this bug already exists in main. It seems to be what's discussed here: #645

[sorcerer86pt]

Yeah. Sorry about this. No mismatch then in regards to supply docks on vc6 A sexta, 25/04/2025, 07:46, helmutbuhler ***@***.***> escreveu:

…

*helmutbuhler* left a comment (TheSuperHackers/GeneralsGameCode#731) <#731 (comment)> how to reproduce, load a big replay and ff it. wait some time (20~30s should suffice). then exit game Thanks! I was just about to post as @roossienb <https://github.com/roossienb> that this bug already exists in main. It seems to be what's discussed here: #645 <#645> — Reply to this email directly, view it on GitHub <#731 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AC2N4Z673YPMP2T3MV3PAF323HK4JAVCNFSM6AAAAAB3P7F752VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDQMRZGUYTOMZQGY> . You are receiving this because you commented.Message ID: ***@***.***>

[Mauller]

Just rebased this on Main with the recent fixes that should resolve the crash on exit problems.

Might be worth a quick retest with replays but it should be good to go.

[Mauller]

Tweaked the comment and updated.

Fewer than the number of values within the array can be copied to it, but the whole array is considered at a later point so the remaining bins needs to be zeroed.

[helmutbuhler]

Tweaked the comment and updated.

Fewer than the number of values within the array can be copied to it, but the whole array is considered at a later point so the remaining bins needs to be zeroed.

Just watched your discussion about this. The point of my comment was to indicate that there are two variables involved, m_numberApproachPositionBones and m_numberApproachPositions, and that they are not used as originally intended. It's easy to confuse this when reading this code (as you two did yourself at first).

You can rephrase that if you want, but don't just delete it.

I also noted in the original PR about this:

Note about the second fix: You can check that it reads that uninitialized memory by setting all bits to one:
memset(approachBones, -1, sizeof(approachBones));
It will then mismatch on the golden replay 1. It seems we are lucky that that memory is all 0 on retail version.

I stumbled upon this while simulating the golden replay without client updates. In that case the stack memory is not all 0 and it mismatches.

Other than that, thank you for handling this!

[xezon]

I optimized the description for this change.

[xezon]

Revisited code comment once more after revieweing helmut's last review comment.

Review was addressed

[xezon]

Looks good to me.