microsoft / DirectXTK

The DirectX Tool Kit (aka DirectXTK) is a collection of helper classes for writing DirectX 11.x code in C++
https://walbourn.github.io/directxtk/
MIT License
2.55k stars 506 forks source link

Reproducible crash in the 3D Audio System #460

Closed HSNB closed 3 weeks ago

HSNB commented 1 month ago

If you set Audio Listener's velocity to NAN (X, Y or Z any or all). An internal crash occurs inside XAudio2_9.dll

HSNB commented 1 month ago

Similarly, setting Audio Emitter's velocity to NAN also causes a crash

walbourn commented 1 month ago

XAudio2 expects that you don't pass invalid data to it. I know that the DEBUG functionality will emit ETW warnings for these scenarios.

Per the header in x3daudio.h:

    4.  Only real values are permissible with functions using 32-bit
        float parameters -- NAN and infinite values are not accepted.
        All computation occurs in 32-bit precision mode.    
walbourn commented 1 month ago

Note I will look at adding some additional logging validation in Debug builds of DirectX Tool Kit for Audio to help debug this.

HSNB commented 1 month ago

Ah I see. I suppose to patch the problem I'll do something like

std::isnormal(vel.x) ? vel.x : 0.f

in the ::SetVelocity() methods

walbourn commented 1 month ago

Where is NaN coming from in the first place in your case?

HSNB commented 1 month ago

Honestly it was just during debugging.

A direction vector used for movement direction was zero length and got normalized (divide by zero), which then caused the player position to move to a NaN position. Subsequently the 3D Audio Listener velocity became NaN. Then the game crashed immediately in XAudio2_9.dll.

Other values, such as listener position, also became NaN, but setting position to NaN or one of the other listener params to NaN don't actually cause crashes, peculiarly only velocity does.

I figured that even though I provide invalid data into X3DAudio (via DirectXTK Audio), such a thing shouldn't cause a crash. So I reported it here.

One side note I'd like to mention is that in my game players sometimes (rarely) crash within XAudio and I don't know why, so perhaps this is for the same reason, or something similar. I haven't checked if it's the exact same crash as this one. But if it is then I have some interest in such a crash being fixed I suppose :)

Regardless, I find it somewhat odd that XAudio can crash from simply providing an invalid float value. I don't think Direct3D would crash if it gets passed invalid floats to it, does it?

HSNB commented 1 month ago

assembly callstack code

walbourn commented 1 month ago

XAudio2 was originally written for Xbox 360 (PowerPC) where "floating-point specials" were extremely slow, and as such the design assumes you never provide specials and will make sure that is always the case. For games, specials are avoided for many reasons.

I think the only thing I can really offer here is some extra asserts in the helper methods that warn about specials. I'm not sure that forcing them to be 0 is necessarily a desired behavior... Using std::isnormal would also force denormals to 0, which may be good for performance but not expected behavior either--for Xbox 360, the CPU was typically set to force denormals to zero anyhow for SIMD operations.