win x86: Grab bag of restructuring to get tests working on x86-on-x86

snapshot/win/cpu_context_win.cc

 // Copyright 2015 The Crashpad Authors. All rights reserved.

[Mark Mentovai, 2015/09/16 02:57:19]

In the change description, BUG=chromium:531663 so that it makes more sense when
it shows up in Crashpad’s history.

[scottmg, 2015/09/16 18:05:06]

Done.

 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
 // You may obtain a copy of the License at
 //
 //     http://www.apache.org/licenses/LICENSE-2.0
 //
 // Unless required by applicable law or agreed to in writing, software
 // distributed under the License is distributed on an "AS IS" BASIS,
 // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
@@ skipping 50 matching lines @@
   out->dr7 = context.Dr7;
 
   static_assert(sizeof(out->fxsave) == sizeof(context.FltSave),
                 "types must be equivalent");
   memcpy(&out->fxsave, &context.FltSave.ControlWord, sizeof(out->fxsave));
 }
 
 #else  // ARCH_CPU_64_BITS
 
 void InitializeX86Context(const CONTEXT& context, CPUContextX86* out) {
-  CHECK(false) << "TODO(scottmg) InitializeX86Context()";
+  out->eax = context.Eax;

[Mark Mentovai, 2015/09/16 02:57:19]

It occurs to me that we should consult context.ContextFlags, both here and in
the X64 function, to make sure we’re only setting things to valid data. If bit
we need in ContextFlags is clear, we should probably write zeroes to |out| for
the fields that it controls.

We may actually want to add validity fields to CPUContextX86 and
CPUContextX86_64 too, otherwise, a minidump reader would think that all of those
zeroes are actual register data.

[scottmg, 2015/09/16 18:05:06]

Done.
Maybe a follow up for this to make sure we have something makes sense for both
OS X and Windows.

+  out->ebx = context.Ebx;
+  out->ecx = context.Ecx;
+  out->edx = context.Edx;
+  out->edi = context.Edi;
+  out->esi = context.Esi;
+  out->ebp = context.Ebp;
+  out->esp = context.Esp;
+  out->eip = context.Eip;
+  out->eflags = context.EFlags;
+  out->cs = static_cast<uint16_t>(context.SegCs);
+  out->ds = static_cast<uint16_t>(context.SegDs);
+  out->es = static_cast<uint16_t>(context.SegEs);
+  out->fs = static_cast<uint16_t>(context.SegFs);
+  out->gs = static_cast<uint16_t>(context.SegGs);
+  out->ss = static_cast<uint16_t>(context.SegSs);
+
+  out->dr0 = context.Dr0;
+  out->dr1 = context.Dr1;
+  out->dr2 = context.Dr2;
+  out->dr3 = context.Dr3;
+  // DR4 and DR5 are obsolete synonyms for DR6 and DR7, see
+  // http://en.wikipedia.org/wiki/X86_debug_register.

[Mark Mentovai, 2015/09/16 02:57:19]

For here and the x86 funciton: use https in links when it works, as it does
here. I would have referenced the Intel SDM for this as it’s more authoritative
than Wikipedia, but we have that reference in minidump_context.h already so I
guess this is all right.

[scottmg, 2015/09/16 18:05:06]

Done.

+  out->dr4 = context.Dr6;
+  out->dr5 = context.Dr7;
+  out->dr6 = context.Dr6;
+  out->dr7 = context.Dr7;
+
+  static_assert(sizeof(out->fxsave) == sizeof(context.ExtendedRegisters),
+                "types must be equivalent");
+  memcpy(&out->fxsave, &context.ExtendedRegisters, sizeof(out->fxsave));

[Mark Mentovai, 2015/09/16 02:57:19]

ExtendedRegisters won’t be valid on a CPU that doesn’t support fxsave (meaning
those that don’t support SSE), but there will still be valid x87 state in
FloatSave. In that case, ContextFlags won’t have CONTEXT_EXTENDED_REGISTERS but
will have CONTEXT_FLOATING_POINT. When this happens, we should fill out as much
of out->fxsave as we can given the x87 data, and zero out the rest. Most of this
is straightforward, but we’ll need to add an FsaveToFxsaveTagWord() function.
That part is a little involved and it’s OK to do in its own change if you’d
like. Fortunately, I already wrote the inverse function, which is much more
complex. Going from the full fsave to the abridged fxsave tag word is easier
because you don’t need to interpret the x87 st registers at all, but you will
need to map between physical and logical slots based on fsw.

[scottmg, 2015/09/16 18:05:06]

Ah yes, good point. I CHECKd in that case for now, and I'll take a look
separately.

[scottmg, 2015/09/22 18:14:56]

I just digested this a little better... fxsave became available with SSE (1),
right? I don't think there's much point in supporting pre-SSE hardware given
that VS2013 assumes SSE2, and Chrome has required it for quite a while now:
https://code.google.com/p/chromium/issues/detail?id=349320.

[Mark Mentovai, 2015/09/22 18:23:15]

Yup, that’s right.
Oh, I remember that bug now. I forgot that Chrome required SSE on Windows. I was
remembering some old code in Chrome that had to test for SSE or SSE2 on Windows
and Linux, but was allowed to assume it was available on Mac.

 }
 
 #endif  // ARCH_CPU_64_BITS
 
 }  // namespace crashpad