VS2019 optimising away pDevice in ma_device_init

[alkama]

When compiling in Release mode with VS2019, it seems we got the ma_device* pDevice passed in parameter to ma_device_init() optimised out.

The code then crashes at
https://github.com/dr-soft/miniaudio/blob/master/miniaudio.h#L23182
Giving some read access violation on the ma_zero_object(pDevice);
Everything seems fine and correctly setup, up to that line.

Trying to find a solution on our client code, where the pDevice we pass (as ref) is a global, we did attempt to use heap allocated version, or r/w some random members from our code, but it didn't solve the problem.

We found a workaround by switching to SecureZeroMemory instead of ZeroMemory at line
https://github.com/dr-soft/miniaudio/blob/master/miniaudio.h#L3483
(so in miniaudio / mini_al)

I dont know if you can reproduce it. I hope it's not something we overlooked on our side.
All I know is that it runs fine when built with all our other targets, with previous VS compilers, and even with VS2019 (in Debug mode).

[mackron]

This seems very strange. Are you able to compile and run the simple_playback example?

I don't have an installation of 2019 readily available, but I can't see how this can possibly be happening...

Edit: I looked at the documentation for ZeroMemory and I think you're right. What an absolutely stupid thing to optimize away. How could they be so stupid? I will update this as soon as possible.

[mackron]

So I've been pondering your comment and I'm still a bit unsure. I understand from the documentation that ZeroMemory and memset can be optimized out (which I still think is incredibly stupid - nothing like safe programming). What I'm questioning is how you'd get a read violation in that API. Are you absolutely sure you're passing in the correct pointer? The compiler can't possibly be optimizing away the entire ma_device object because it's used directly in the code below that line. My concern is that SecureZeroMemory() may be working by coincidence rather than by design and that it could be masking a more important bug. Before blindly replacing ZeroMemory with SecureZeroMemory, I'd like to know if simple_playback works.

[alkama]

I tried compiling the examples with the same kind of settings, but it seems to not optimise away the device there.
Guess we will just fix the version we embed in out project with the SecureZeroMemory, since it solves the problem (that only appears on this compiler) on our side :D

Feel free to close the issue, at least if someone else faces the problem, they know what they can do!

Oh, and apparently, memset being optimised out (dead store optimisation) seems to be a know problem in the crypto world. And one that is hard to fix. It might happen with any compiler when optimisation is enabled. With our project it seems to happen only with VS2019, but it's just luck :D Here is a very interesting recorded talk on the topic https://media.ccc.de/v/35c3-9788-memsad

[mackron]

By the way, in case you're interested, you can actually override MAL_ZERO_MEMORY with your own implementation instead of modifying mini_al.h directly. You would do something like this just above the implementation part of mini_al.h:

#if defined(_WIN32)
#define MAL_ZERO_MEMORY(p, sz) SecuredZeroMemory((p), (sz))
#endif
#define MINI_AL_IMPLEMENTATION
#inlcude <mini_al.h>

This saves you from having a modified version of mini_al lying around which is a bit messy.

In any case, I understand the idea of dead store optimizations, but it's just that it doesn't make sense that it would crash. The problem is more about sensitive data not being scrubbed. This is just not adding up to me - it feels like a more serious underlying bug somewhere else that's triggering it. I assume this crash is happening with Bonzomatic? I had a look at the code in FFT.cpp, but I can't see anything obvious on that end. Nor can I see any issues in miniaudio... I'm not sure what's going on...

Have you looked at a disassembly of the compiled code by the way?

[alkama]

Nice! Hadn't checked to see if MAL_ZERO_MEMORY could not be defined without being overwritten when including :D
It's indeed in Bonzomatic and in the FFT.cpp file. Cant see no problem there either, it's a pretty simple class. Static analysis finds nothing fishy, and it works fine with quite a few compilers and platforms.

According to where it crashes and after debugging to see the content of everything above that "zeroing of pDevice", it looks looks like everything else is working as it should and contains the right data. My conclusion was that maybe the whole pDevice was discarded by the compiler and never created for it appeared unused, leading to that "read access violation". I then supposed that forcing the compiler to realise pDevice is actually used would help :D And using the SecureZeroMemory apparently did the job of both guarantee it's zeroed, and have it realise it is not a dead variable.

Havent checked the disassembly yet (was scared by the pain of reading the result of a Release build), but I will have a look tonight.

[mackron]

I'm going to go ahead and close this one. I'm not convinced this is an issue with miniaudio directly, especially considering the examples seem to be working just fine.