[ros-dev] [ros-diffs] [tkreuzer] 51748: [NTOSKRNL] Patch by Paolo Bonzini <bonzini [at] gnu [dot] org> Fix implementation of ExInterlockedAddLargeStatistic The old version wasn't really atomic. See issue #6223 for more ...

[Alex Ionescu]

Look, I don't understand why this is so hard for the project to understand:

Certain Windows functions are BROKEN. Certain Windows algorithms make NO
SENSE. Other things are BADLY OPTIMIZED. Other things make WRONG
ASSUMPTIONS.

But these set of behaviors and design failures are what billions of
customers and pieces of software DEPEND ON. Microsoft cannot change the
STACK LAYOUT of IE due to fears that BHOs which look up the file handle up
the stack will break. They have SHIMS that LIE about the stack order to make
those BHOs work, even as the stack changes.

Microsoft, on this function, writes,

"Remarks
------------------------------

This function is not atomic because it is implemented as two separate locked
instructions. An atomic 64-bit read that occurs on another thread during the
execution of this intrinsic could result in an inconsistent value being
read.
"



Can you claim, with absolute CERTAINTY that NOBODY in the WORLD depends on
this behavior? That it's two separate intrusctions, and that an inconsistent
value can be read? How do you know certain VMs don't look for this code
sequence, how do you know security/emulation software doesn't abuse/utilise
this? ETC ETC ETC. It's such a simple/short sequence that it's easy to
recognize.

As a simple example, HexRays recognizes the particular ASM sequence as
InterlockedAddLargeStatistic. Now, in ReactOS, it won't.

What's worse, is that recent Windows kernels define ExInterlocked.... to the
compiler intrinsic _Interlocked. Do you suggest to also implement the
intrinsic differently, which is in-lined? So now you will inline those 50
lines of (perfect, yes) code in every caller? These functions are called at
ISR level usually -- great idea!

Or maybe you keep the (broken) MS implementation in the intrinsic, but use
this one for the export? Great, now you introduce two sets of behaviors, and
add even more non-determinism.

Do you really believe Microsoft is stupid and didn't realize this? That they
didn't change things around just because they forgot? No, it's 20 years too
late to change anything, and if you start doing this in ReactOS, you are
certain to fail.

Please leave all personal attacks and politics aside, tell me where I'm
technically wrong. You cannot prove this better implementation won't break
things. You have the compiler intrinsic to worry about. I named at least one
piece of software that depends on it. I named critical speed/code size
issues with this function. What more is needed to prove the point?

Best regards,
Alex Ionescu


On Sat, May 14, 2011 at 4:07 PM, Timo Kreuzer <timo.kreuzer at web.de> wrote:

> Am 15.05.2011 00:06, schrieb Alex Ionescu:
>
>  Congrats on once again breaking another key part of Windows compatibility.
>>
> Sorry for my ignorance. Could you please enlighten me and explain how code
> that behaves identically, except in very, very rare cases, where it doesnt
> fail miserably like the old version, and which might be a tiny piece slower,
> will break compatibility. Do drivers expect the function to fail randomly?
> Is the function time critical? Do snakes have legs? Whats the purpose of
> life?
>
>
>
>> This function is DOCUMENTED to tear. It was DESIGNED that way on purpose.
>>
> I see, "Its not a bug, its a feature"(tm)
>
>
>
>> Utter failure, once more.
>>
> Of course, "As usual"(tm)
>
>
>> Best regards,
>> Alex Ionescu
>>
>
>
> _______________________________________________
> Ros-dev mailing list
> Ros-dev at reactos.org
> http://www.reactos.org/mailman/listinfo/ros-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.reactos.org/pipermail/ros-dev/attachments/20110514/828e36c0/attachment.htm>