Re: Crash with old Windows on new CPU

On Sat, Feb 13, 2016 at 4:45 PM, Christian Ullrich <chris(at)chrullrich(dot)net>
wrote:

> On February 13, 2016 4:10:34 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> > Christian Ullrich <chris(at)chrullrich(dot)net> writes:
> >> * Robert Haas wrote:
> >>> Thanks for the report and patch. Regrettably I haven't the Windows
> >>> knowledge to have any idea whether it's right or wrong, but hopefully
> >>> someone who knows Windows will jump in here.
> >
> >> In commitfest now.
> >
> > FWIW, I'm a tad suspicious of the notion that it's our job to make this
> > case work. How practical is it really to run a Windows release on
> > unsupported-by-Microsoft hardware --- aren't dozens of other programs
> > going to have the same issue?
>
> Why would the hardware be unsupported? The problem occurs on new CPUs, not
> old ones, and even the OS (2008) is still in extended support until next
> year, IIRC.
>
> > I'm also suspicious of the "#if _MSC_VER == 1800" tests, that is,
> > the code compiles on *exactly one* MSVC version.
>
> The bug exists in only that compiler version's CRT, also, that is not the
> complete version number. There may be different builds somewhere, but they
> all start with 18.0.
>

IIRC, the CRT itself does explicit checks against _MSC_VER == 1800. As in,
they don't actually bump that number in different build numbers.

How does this work wrt mingw, though? Do we have the same problem there?
AIUI this code can never run on mingw, correct?

> Lastly, I'd like to see some discussion of what side effects
> > "_set_FMA3_enable(0);" has ... I rather doubt that it's really
> > a magic-elixir-against-crashes-with-no-downsides.
>
> It tells the math library (in the CRT, no separate libm on Windows) not to
> use the AVX2-based implementations of log() and possibly other functions.
> AIUI, FMA means "fused multiply-add" and is apparently something that
> increases performance and accuracy in transcendental functions.
>
> I can check the CRT source later today and figure out exactly what it does.
>
> Also, if you look at the link I sent, you will find that a member of the
> Visual C++ Libraries team at MS is the source for the workaround. They
> probably know what they are doing, present circumstances excepted.
>

I notice the code checks IsWindows7SP1OrGreater() but the comment refers to
W7SP1 *or* 2008R2 SP1. I assume this is correct, or should there actually
be a separate check for server-windows?

> > That would
> > give us some context to estimate the risks of this code executing
> > when it's not really needed.
>
> Hence all the conditions. The problem is *certain* to occur under these
> specific conditions (x64 code on Windows before 7SP1 on a CPU with AVX2
> when built with VS2013), and under no others, and these conditions flip the
> switch exactly then.
>

Well, it doesn't flip it based on the specifics "running on a CPU with
AVX2". But presumably turning of AVX2 if the CPU doesn't support it is a
no-op.

> Without that, I'd be worried that
> > this cure is worse than the disease because it breaks cases that
> > weren't broken before.
>
> Isn't that what the buildfarm is (among other things) for?
>

The buildfarm doesn't really have a big spread of Windows animals,
unfortunately.

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/