[LLDB] [Windows] Avoid using InitializeContext for allocating a CONTEXT. NFC.

[mstorsjo]

Summary

InitializeContext is useful for allocating a CONTEXT struct in an unaligned byte buffer. In this case, we already have a CONTEXT we want to initialize, and we only used this as a very roundabout way of zero initializing it.

Instead just memset the CONTEXT we have, and set the ContextFlags field manually.

This matches how it is done in NativeRegisterContextWindows_*.cpp.

This also makes LLDB run successfully in Wine (for a trivial tested case at least), as Wine haven't implemented the InitializeContext function.

[amccarth]

I'm good with the change, but have a couple small requests. I hope to hear from others, too, as this area is outside my wheelhouse.

lldb/source/Plugins/Process/Windows/Common/RegisterContextWindows.cpp

158

As far as I understand what InitializeContext does, this seems a suitable replacement for how it's used here.

But if someone were to change the flag to include CONTEXT_XSTATE, then this would no longer work, because the xstate is variable length.

I would suggest:

1. Eliminate the kWinContextFlags (defined at the top of this file) and just use CONTEXT_ALL here. The extra name doesn't seem to clarify anything, and the distance between the definition and usage can make it harder for folks to understand this code.

2. Add a comment saying this is a substitute for InitializeContext for Wine, which makes searching the code for InitializeContext useful.

[mstorsjo]

lldb/source/Plugins/Process/Windows/Common/RegisterContextWindows.cpp

158

Good point re CONTEXT_XSTATE; as far as I see the NativeRegisterContext classes share that limitation as well.

As for the change suggestions, 1. makes sense to me.

As for 2, while wine was the reason for looking at this originally, it's not a place where InitializeContext makes much sense to begin with, IMO. If you read the current original code, we just use it for getting a (variably sized) CONTEXT pointer in a large zero-initialized buffer, to use for memcpying over a fixed sized CONTEXT elsewhere. None of that feels like what InitializeContext does.

Or put another way; I wouldn't like to point out wine here, as I wouldn't suggest this change if I felt the use of InitializeContext was justified here. (In that case I'd probably fix wine instead.) If we'd get rid of the fixed size allocation of m_context, using InitializeContext for allocating a pointer would make sense though.

[amccarth]

That's fair. LGTM.

lldb/source/Plugins/Process/Windows/Common/RegisterContextWindows.cpp

 }
 
 bool RegisterContextWindows::CacheAllRegisterValues() {
   Log *log = ProcessWindowsLog::GetLogIfAny(WINDOWS_LOG_REGISTERS);
   if (!m_context_stale)
     return true;
 
   TargetThreadWindows &wthread = static_cast<TargetThreadWindows &>(m_thread);
-  uint8_t buffer[2048];
-  memset(buffer, 0, sizeof(buffer));
-  PCONTEXT tmpContext = NULL;
-  DWORD contextLength = (DWORD)sizeof(buffer);
-  if (!::InitializeContext(buffer, kWinContextFlags, &tmpContext,
-                           &contextLength)) {
-    return false;
-  }
-  memcpy(&m_context, tmpContext, sizeof(m_context));
+  memset(&m_context, 0, sizeof(m_context));
+  m_context.ContextFlags = kWinContextFlags;
   if (::SuspendThread(
           wthread.GetHostThread().GetNativeThread().GetSystemHandle()) ==
       (DWORD)-1) {
     return false;
   }
   if (!::GetThreadContext(
           wthread.GetHostThread().GetNativeThread().GetSystemHandle(),
           &m_context)) {