the tale of the upgraded compiler and the corrupted memory

[slab]

(I originally posted this over on cohost, but cohost will be shutting down so I'm reposting it here.)

While writing this comment I had a story from circa 2015 that didn't really fit in the comment so I figured I should instead branch it out into A Post because I don't really do enough of those here.

So, MSVC has a /kernel flag that it gained at some point (I want to say MSVC 12.0 or so, but Microsoft doesn't have the documentation online for anything earlier than MSVC 2015 anymore). This flag tells the compiler that you're building something to run in the Windows kernel. For C++, this also turns off exceptions and RTTI, so you're getting a subset of C++, but even a subset of C++ can be pretty useful.

There is also another thing that that flag does which is a bit more subtle.

So, you see, if you take a Windows kernel driver codebase from a pre-/kernel MSVC (where we had just been passing /EH- /GR- to get rid of exceptions/RTTI directly) to a version after they added /kernel, then you're probably not going to have /kernel set in your build options because it didn't exist before, and that turns out to create a problem... and that problem is that occasionally you'll get really weird memory corruptions.

The timing was difficult to reproduce (the userspace process has to be scheduled out at just the right time) and also the scope of the data corruption was not entirely obvious; sometimes you'd get processes crashing in weird ways, sometimes you'd get "when we look at this GUID it's wrong but when we look at it in the debugger it's correct". At first we thought it might just be bad memory in our test suites, but our test suites run on different physical hardware setups; it would be weird for multiple different setups to experience hardware problems.

The test suites run with different versions of Windows on the same hardware; at the time I think they reimaged between XP/32, Vista/32, Vista/64, 7/32, 7/64. After enough runs, we had confidence that the weird failures happened only on 32-bit versions of Windows, and they didn't seem to show up on 64-bit ones.

The test suites also ran on approximately every daily build (we were not doing CI at commit granularity at the time, only at top-level installer level), so we had an timeframe. Additionally, the MSVC 10.0-to-14.0 (I think this was the jump?) compiler upgrade project had started in lower-level infrastructure libraries, so we were able to narrow it down to a small set of binaries that had been built with the new compiler.

I ended up disassembling both "before" and "after" and the only changes that took place that weren't otherwise accounted for were that the compiler had "helpfully" optimized
operator=() on a sufficiently-large POD struct into moves to/from xmmN registers. That's fine, right, every x86 processor for the ten years prior has had SSE2, that shouldn't be a problem, right? Well. 32-bit Windows does not automatically preserve SSE register state on kernel transitions. (It does on 64-bit Windows, because SSE2 is required by specification on amd64 and so they use SSE registers as part of the ABI.)

...

So the other subtle thing that /kernel does is that it forces /arch:IA32, because at about the same time that they added that, they _also_ made the default /arch setting for 32-bit Windows builds be /arch:SSE2. This was not entirely unreasonable (because, again, SSE2 support was ubiquitous by that point), but it did mean that if you didn't specify the option before, then now you were suddenly getting SSE-optimized memcpy() (and things that were essentially memcpy, like operator=() on large POD structs). Since we were now (unintentionally) using the xmm registers without using KeSaveExtendedProcessorState) to save them, we were clobbering over them. And if a userspace thread happened to have gotten scheduled out in the middle of its own SSE-optimized memcpy, then part of that copy would be clobbered. Oops!

We all were very glad to have caught that before it shipped to users.

And if I remember correctly, the company showed its gratitude to finding the fix with a measly gift card and an only-slightly-above inflation raise that year.

lolsob.