I encouraged game developers in the past to use static analysis tools to help iron out more bugs. It's great to see Unreal Engine taking that step. A big name like this will hopefully inspire others to take the same step.
Why is "(GEngine == nullptr || !GEngine->UseSound())" safer than "(GEngine || !GEngine->UseSound())"? Is this some kind of precedence rule, where the former short-circuits but the latter doesn't?
The two conditions are not equialent - the first is true if GEngine is null or does not use sound, and the second is true if GEngine is not null and is undefined otherwise due to dereferencing a null pointer. I assume the original author intended to write "(!GEngine || !GEngine->UseSound())".
In code bases as big as Unreal Engine 4, you can easily remove as much as 10k lines of code and nobody will notice. It's not uncommon to have that much (and more!) dead code even. A single missing memcpy doesn't mean much, unless it's part of a hot code path.
All these rarely visited code path bugs matter in terms of security of course.
Code like this is painful to unit test, so it often plainly isn't unit tested.
So then comes the question why was it not detected in production (there's a good chance that there's games out there that have shipped with that line in there). Memcmp doesn't have any side effects, so instead of something updating, something just doesn't update. The function is called 'Optimize', perhaps it just calculates a more optimal dataset, and then does nothing with it, leaving the unoptimized data in there. No one would notice and some path would just be slightly less optimal.
