From 7104ab1f76d9fe6823016dcb5561e179174e39a2 Mon Sep 17 00:00:00 2001 From: Tom J Nowell Date: Tue, 28 Jul 2026 13:55:19 +0100 Subject: [PATCH] Sim/COB: avoid UB in float-to-short angle casts (fixes arm64/x86 desync) (#3075) * Sim/COB: avoid UB in float-to-short angle casts (fixes arm64/x86 desync) Casting a floating-point heading/pitch expression directly to `short` is undefined behaviour when the truncated value does not fit in `short`, and this is genuinely reached: `heading * RAD2TAANG` equals +-32768 at heading = +-pi, one past short's 32767 maximum. Because the result is UB, arm64 and x86 produced different values at the angle extremes, which was observed as an arm64/x86 multiplayer desync. x86 already lowers `short(float)` as float->int->short (cvttss2si into a 32-bit register, then narrow), so making the `int()` step explicit leaves the already-correct x86 result unchanged while pinning arm64 (whose fcvtzs saturated differently) to the same value. The int->short narrowing is the intended 16-bit TA-angle wraparound. Sites fixed in CobInstance.cpp: WindChanged, StartBuilding, AimWeapon. Sibling casts were checked and left untouched: UnitScript.cpp:1058 casts asin(...)*RAD2TAANG (range +-16384, always fits short, no UB) and lines 1095/1099 already route through int; CobThread.cpp has no such casts. Pure correctness fix to synced simulation code; it does not alter results on the platform that was already correct. Cross-arch sync validation is recommended. Origin: discussed in PR #2991's review (credit BambaDamba). AI assistance: implemented with Claude Code (Anthropic) from a written plan; reasoning and build verification reviewed by a human. * address review: extract RadAngleToCobShort helper with explanatory comment sprunk asked for a comment and a named helper around the float->short angle cast. Wrap the conversion in RadAngleToCobShort() and document why it exists: COB angles are circular 16-bit TA units (full turn == COBSCALE), so values past a half turn intentionally wrap modulo 2^16 via the int->short narrowing. The float->int step stays because the wrap is the desired behaviour and is deterministic across arm64/x86; clamping the range (as suggested) would break angles past a half turn rather than wrapping them. No behavioural change vs the previous short(int(...)) form. --- rts/Sim/Units/Scripts/CobInstance.cpp | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/rts/Sim/Units/Scripts/CobInstance.cpp b/rts/Sim/Units/Scripts/CobInstance.cpp index f4028a80797..e073959df9b 100644 --- a/rts/Sim/Units/Scripts/CobInstance.cpp +++ b/rts/Sim/Units/Scripts/CobInstance.cpp @@ -42,6 +42,20 @@ /******************************************************************************/ /******************************************************************************/ +// COB scripts encode angles as TA units where a full turn is COBSCALE (65536), +// so any angle past a half turn exceeds the range of a signed short and is meant +// to wrap around (it is a circular 16-bit angle). Truncating the scaled value to +// int first is well defined for the bounded angles the sim feeds in, and the +// following int->short narrowing performs that modular wrap deterministically. +// Converting straight from float to short would be undefined behaviour once the +// value leaves short's range, and produced different results on arm64 vs x86, +// desyncing multiplayer. +static inline short RadAngleToCobShort(float radAngle) +{ + return static_cast(static_cast(radAngle * RAD2TAANG)); +} + + CR_BIND_DERIVED(CCobInstance, CUnitScript, ) CR_REG_METADATA(CCobInstance, ( @@ -237,7 +251,7 @@ void CCobInstance::WindChanged(float heading, float speed) { ZoneScoped; Call(COBFN_SetSpeed, int(speed * 3000.0f)); - Call(COBFN_SetDirection, short(heading * RAD2TAANG)); + Call(COBFN_SetDirection, RadAngleToCobShort(heading)); } @@ -387,8 +401,8 @@ void CCobInstance::StartBuilding(float heading, float pitch) std::array callinArgs; callinArgs[0] = 2; - callinArgs[1] = short(heading * RAD2TAANG); - callinArgs[2] = short( pitch * RAD2TAANG); + callinArgs[1] = RadAngleToCobShort(heading); + callinArgs[2] = RadAngleToCobShort(pitch); Call(COBFN_StartBuilding, callinArgs); } @@ -439,8 +453,8 @@ void CCobInstance::AimWeapon(int weaponNum, float heading, float pitch) std::array callinArgs; callinArgs[0] = 2; - callinArgs[1] = short(heading * RAD2TAANG); - callinArgs[2] = short( pitch * RAD2TAANG); + callinArgs[1] = RadAngleToCobShort(heading); + callinArgs[2] = RadAngleToCobShort(pitch); Call(COBFN_AimPrimary + COBFN_Weapon_Funcs * weaponNum, callinArgs, CBAimWeapon, weaponNum, nullptr); }