PM_TraceModel may return uninitialized trace

[a1batross]

Related issue: FWGS/xash3d-fwgs#885

In short, on some maps compiled with ZHLT it's possible that TraceModel may return uninitialized trace values, which then used in ladder code: https://github.com/ValveSoftware/halflife/blob/master/pm_shared/pm_shared.c#L2092

Following the execution, these uninitialized values may get interpreted as NaN, poisoning other float variables down to player's velocity: https://github.com/ValveSoftware/halflife/blob/master/pm_shared/pm_shared.c#L2173

Further execution gets to another trace function, this time PM_PlayerTrace: https://github.com/ValveSoftware/halflife/blob/master/pm_shared/pm_shared.c#L830 which then causes an infinite loop in engine's PM_HullPointContents.

The fix is simple: PM_TraceModel should initialize trace_t early, like this:

static inline void PM_InitTrace( trace_t *trace, const vec3_t end )
{
	memset( trace, 0, sizeof( *trace ));
	VectorCopy( end, trace->endpos );
	trace->allsolid = true;
	trace->fraction = 1.0f;
}

static float GAME_EXPORT pfnTraceModel( physent_t *pe, float *start, float *end, trace_t *trace )
{
	// variable declarations...
	PM_InitTrace( trace, end );
	...
}

This way result data will be interpreted as "didn't hit anything, stuck in solid", thus fraction = 1.0, allsolid = true and endpos as end point. On game code side, this can be fixed similarly, just initializing trace structure before any call to TraceModel.

Relevant fix in xash3d-fwgs source: FWGS/xash3d-fwgs@c076f4f and FWGS/xash3d-fwgs@85895c5

Thanks to @FreeSlave for finding a way to reproduce this bug and Unkle Mike for correct trace struct initialization.

[tschumann]

Is there a way to fix this from game code?

…

On Monday, 27 June 2022, a1batross ***@***.***> wrote: Related issue: FWGS/xash3d-fwgs#885 <FWGS/xash3d-fwgs#885> In short, on some maps compiled with ZHLT it's possible that TraceModel may return uninitialized trace values, which then used in ladder code: https://github.com/ValveSoftware/halflife/blob/ master/pm_shared/pm_shared.c#L2092 Following the execution, these uninitialized values may get interpreted as NaN, poisoning other float variables down to player's velocity: https://github.com/ValveSoftware/halflife/blob/ master/pm_shared/pm_shared.c#L2173 Further execution gets to another trace function, this time PM_PlayerTrace: https://github.com/ValveSoftware/halflife/blob/ master/pm_shared/pm_shared.c#L830 which then causes an infinite loop in engine's PM_HullPointContents. The fix is simple: PM_TraceModel should initialize trace_t early, like this: static inline void PM_InitTrace( trace_t *trace, const vec3_t end ) { memset( trace, 0, sizeof( *trace )); VectorCopy( end, trace->endpos ); trace->allsolid = true; trace->fraction = 1.0f; } static float GAME_EXPORT pfnTraceModel( physent_t *pe, float *start, float *end, trace_t *trace ) { // variable declarations... PM_InitTrace( trace, end ); ... } This way result data will be interpreted as "didn't hit anything, stuck in solid", thus fraction = 1.0, allsolid = true and endpos as end point. On game code side, this can be fixed similarly, just initializing trace structure before any call to TraceModel. Relevant fix in xash3d-fwgs source: ***@***.*** <FWGS/xash3d-fwgs@c076f4f> and ***@***.*** <FWGS/xash3d-fwgs@85895c5> Thanks to @FreeSlave <https://github.com/FreeSlave> for finding a way to reproduce this bug and Unkle Mike for correct trace struct initialization. — Reply to this email directly, view it on GitHub <#3283>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA34IYTZGLDYDILWJS7TUFLVRCLFVANCNFSM5Z4JKS4A> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[a1batross]

@tschumann yes!

You may copy the PM_InitTrace function from the example above and call it before PM_TraceModel, assuming that it may not return an initialized trace.

[tschumann]

Okay thanks - is it only a problem for maps compiled with ZHLT?

…

On Wed, 29 Jun 2022 at 02:11, a1batross ***@***.***> wrote: @tschumann <https://github.com/tschumann> yes! You may copy the PM_InitTrace function from the example above and call it before PM_TraceModel, assuming that it may not return an initialized trace. — Reply to this email directly, view it on GitHub <#3283 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA34IYROL5SHQDTKELK47HTVRMP2FANCNFSM5Z4JKS4A> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[a1batross]

@tschumann I don't remember now but at least this bug shouldn't happen with original compilers.