[debuginfo-tests][Dexter] Fix process creation flake-out on Windows

[jmorse]

Summary

When writing the Windows dbgeng driver for Dexter, I couldn't work out why it would either launch a process and leave it free running, or if I started the process suspended, never do anything with it. The result was a hack to create and attach processes manually. This has been flaking out on Reids Windows buildbot, and clearly wasn't a good solution.

Digging into this, it turns out that the "normal" cdb / windbg behaviour of breaking whenever we attach to a process is not the default: it has to be explicitly requested from the debug engine. This patch does so (by setting DEBUG_ENGOPT_INITIAL_BREAK in the engine options), after which we can simply call "CreateProcessAndAttach2" and everything automagically works.

No test for this behaviour: everything was just broken before.

[Orlando]

debuginfo-tests/dexter/dex/debugger/dbgeng/setup.py

101

What happens if the timeout fails?

Does it make sense to have a similar snippet to the one found a couple lines down for another WaitForEvent:

if res == S_FALSE:
  client.TerminateProcesses()
  raise Exception("Debuggee did not reach main function in a timely manner")

[rnk]

Neat. :)

BTW, the buildbot is currently not running debuginfo-tests:
cmake ... -GNinja -DLLVM_ENABLE_PROJECTS=llvm;clang;clang-tools-extra;lld

I think the main blocker was the compiler wrapper batch scripts not being quite right for newer VS versions. I want to try this again locally, and if I can get it working there, move on to the bot.

[TWeaver]

@rnk

Hi Reid,

Do you have any output/stack traces/error messages for the VS error you've seen?

The windows debug info tests are built with clang and debugged using dbgeng atm, so there shouldn't be any overlap with visual studio at all.

I'd be very interested to see what the issue is.

Thanks
Tom W

[TWeaver]

LGTM

[rnk]

In D74409#1872084, @TWeaver wrote:

The windows debug info tests are built with clang and debugged using dbgeng atm, so there shouldn't be any overlap with visual studio at all.

I'd be very interested to see what the issue is.

There are my check-debuginfo logs:
https://reviews.llvm.org/P8194

I think the VS dependence is here:
https://github.com/llvm/llvm-project/blob/master/debuginfo-tests/dexter/dex/builder/scripts/windows/clang-cl_vs2015.bat#L4

call "%VS140COMNTOOLS%..\..\VC\bin\amd64\vcvars64.bat"

This would set up the environment so that clang-cl can find MSVC headers and libraries. For me, VS140COMNTOOLS is not set, and the later errors are probably a downstream consequence of that. I launched a new "developer command prompt" for VS 2019 and it also lacks that env var, so I'm not sure it can be relied on.

Hopefully that helps.

[jmorse]

Greg via email points out vswhere: https://devblogs.microsoft.com/setup/vswhere-is-now-installed-with-visual-studio-2017/ which seems to be just what the doctor ordered. I'll take a look at that shortly (and land this patch).

[jmorse]

debuginfo-tests/dexter/dex/debugger/dbgeng/setup.py

101

I guess it does, that's a better failure mode than the current situation, where calls will randomly fail with status 0x8000FFFF.

debuginfo-tests/dexter/dex/debugger/dbgeng/README.md

 The majority of the work occurs in setup.py and probe_process.py. The
 former contains routines to launch a process and attach the debugger to
 it, while the latter extracts as much information as possible from a
 stopped process, returning a list of stack frames with associated variable
 information.
 
 ## Sharp edges
 
-For reasons still unclear, using CreateProcessAndAttach never appears to
-allow the debuggee to resume, hence this implementation creates the
-debuggee process manually, attaches, and resumes.
-
 On process startup, we set a breakpoint on main and then continue running
 to it. This has the potential to never complete -- although of course,
 there's no guarantee that the debuggee will ever do anything anyway.
 
 There doesn't appear to be a way to instruct DbgEng to "step into" a
 function call, thus after reaching main, we scan the module for all
 functions with line numbers in the source directory, and put breakpoints
 on them. An alternative implementation would be putting breakpoints on
 every known line number.
 
 Finally, it's unclear whether arbitrary expressions can be evaluated in
 arbitrary stack frames, although this isn't something that Dexter currently
 supports.
  

debuginfo-tests/dexter/dex/debugger/dbgeng/client.py

class DebugAttach(IntFlag):
   DEBUG_ATTACH_NONINVASIVE_NO_SUSPEND =       4
   DEBUG_ATTACH_INVASIVE_NO_INITIAL_BREAK =    8
   DEBUG_ATTACH_INVASIVE_RESUME_PROCESS =   0x10
   DEBUG_ATTACH_NONINVASIVE_ALLOW_PARTIAL = 0x20
 
 # UUID for DebugClient7 interface.
 DebugClient7IID = IID(0x13586be3, 0x542e, 0x481e, IID_Data4_Type(0xb1, 0xf2, 0x84, 0x97, 0xba, 0x74, 0xf9, 0xa9 ))
 
+class DEBUG_CREATE_PROCESS_OPTIONS(Structure):
+  _fields_ = [
+    ("CreateFlags", c_ulong),
+    ("EngCreateFlags", c_ulong),
+    ("VerifierFlags", c_ulong),
+    ("Reserved", c_ulong)
+  ]
+
 class IDebugClient7(Structure):
   pass
 
 class IDebugClient7Vtbl(Structure):
   wrp = partial(WINFUNCTYPE, c_long, POINTER(IDebugClient7))
   idc_queryinterface = wrp(POINTER(IID), POINTER(c_void_p))
   idc_attachprocess = wrp(c_longlong, c_long, c_long)
   idc_detachprocesses = wrp()
+  idc_terminateprocesses = wrp()
+  idc_createprocessandattach2 = wrp(c_ulonglong, c_char_p, c_void_p, c_ulong, c_char_p, c_char_p, c_ulong, c_ulong)
   _fields_ = [
       ("QueryInterface", idc_queryinterface),
       ("AddRef", c_void_p),
       ("Release", c_void_p),
       ("AttachKernel", c_void_p),
       ("GetKernelConnectionOptions", c_void_p),
       ("SetKernelConnectionOptions", c_void_p),
       ("StartProcessServer", c_void_p),
_fields_ = [
       ("AddProcessOptions", c_void_p),
       ("RemoveProcessOptions", c_void_p),
       ("SetProcessOptions", c_void_p),
       ("OpenDumpFile", c_void_p),
       ("WriteDumpFile", c_void_p),
       ("ConnectSession", c_void_p),
       ("StartServer", c_void_p),
       ("OutputServers", c_void_p),
-      ("TerminateProcesses", c_void_p),
+      ("TerminateProcesses", idc_terminateprocesses),
       ("DetachProcesses", idc_detachprocesses),
       ("EndSession", c_void_p),
       ("GetExitCode", c_void_p),
       ("DispatchCallbacks", c_void_p),
       ("ExitDispatch", c_void_p),
       ("CreateClient", c_void_p),
       ("GetInputCallbacks", c_void_p),
       ("SetInputCallbacks", c_void_p),
_fields_ = [
       ("GetOutputLinePrefixWide", c_void_p),
       ("SetOutputLinePrefixWide", c_void_p),
       ("GetIdentityWide", c_void_p),
       ("OutputIdentityWide", c_void_p),
       ("GetEventCallbacksWide", c_void_p),
       ("SetEventCallbacksWide", c_void_p),
       ("CreateProcess2", c_void_p),
       ("CreateProcess2Wide", c_void_p),
-      ("CreateProcessAndAttach2", c_void_p),
+      ("CreateProcessAndAttach2", idc_createprocessandattach2),
       ("CreateProcessAndAttach2Wide", c_void_p),
       ("PushOutputLinePrefix", c_void_p),
       ("PushOutputLinePrefixWide", c_void_p),
       ("PopOutputLinePrefix", c_void_p),
       ("GetNumberInputCallbacks", c_void_p),
       ("GetNumberOutputCallbacks", c_void_p),
       ("GetNumberEventCallbacks", c_void_p),
       ("GetQuitLockString", c_void_p),
def AttachProcess(self, pid):
     res = self.vt.AttachProcess(self.client, 0, pid, DebugAttach.DEBUG_ATTACH_DEFAULT)
     aborter(res, "AttachProcess")
     return
 
   def DetachProcesses(self):
     res = self.vt.DetachProcesses(self.client)
     aborter(res, "DetachProcesses")
     return
+
+  def TerminateProcesses(self):
+    res = self.vt.TerminateProcesses(self.client)
+    aborter(res, "TerminateProcesses")
+    return
+
+  def CreateProcessAndAttach2(self, cmdline):
+    options = DEBUG_CREATE_PROCESS_OPTIONS()
+    options.CreateFlags = 0x2 # DEBUG_ONLY_THIS_PROCESS
+    options.EngCreateFlags  = 0
+    options.VerifierFlags = 0
+    options.Reserved = 0
+    attach_flags = 0
+    res = self.vt.CreateProcessAndAttach2(self.client, 0, cmdline.encode("ascii"), byref(options), sizeof(options), None, None, 0, attach_flags)
+    aborter(res, "CreateProcessAndAttach2")
+    return

debuginfo-tests/dexter/dex/debugger/dbgeng/control.py

class IDebugControl7Vtbl(Structure):
   idc_waitforevent = wrp(c_long, c_long)
   idc_execute = wrp(c_long, c_char_p, c_long)
   idc_setexpressionsyntax = wrp(c_ulong)
   idc_addbreakpoint2 = wrp(c_ulong, c_ulong, POINTER(POINTER(DebugBreakpoint2)))
   idc_setexecutionstatus = wrp(c_ulong)
   idc_getexecutionstatus = wrp(c_ulong_p)
   idc_getstacktraceex = wrp(c_ulonglong, c_ulonglong, c_ulonglong, PDEBUG_STACK_FRAME_EX, c_ulong, c_ulong_p)
   idc_evaluate = wrp(c_char_p, c_ulong, PDEBUG_VALUE, c_ulong_p)
+  idc_setengineoptions = wrp(c_ulong)
   _fields_ = [
       ("QueryInterface", c_void_p),
       ("AddRef", c_void_p),
       ("Release", c_void_p),
       ("GetInterrupt", c_void_p),
       ("SetInterrupt", c_void_p),
       ("GetInterruptTimeout", c_void_p),
       ("SetInterruptTimeout", c_void_p),
_fields_ = [
       ("SetEffectiveProcessorType", c_void_p),
       ("GetExecutionStatus", idc_getexecutionstatus),
       ("SetExecutionStatus", idc_setexecutionstatus),
       ("GetCodeLevel", c_void_p),
       ("SetCodeLevel", c_void_p),
       ("GetEngineOptions", c_void_p),
       ("AddEngineOptions", c_void_p),
       ("RemoveEngineOptions", c_void_p),
-      ("SetEngineOptions", c_void_p),
+      ("SetEngineOptions", idc_setengineoptions),
       ("GetSystemErrorControl", c_void_p),
       ("SetSystemErrorControl", c_void_p),
       ("GetTextMacro", c_void_p),
       ("SetTextMacro", c_void_p),
       ("GetRadix", c_void_p),
       ("SetRadix", c_void_p),
       ("Evaluate", idc_evaluate),
       ("CoerceValue", c_void_p),
def Evaluate(self, expr):
     } # And everything else is invalid.
 
     if val_type not in extract_map:
       raise Exception("Unexpected debug value type {} when evalutaing".format(val_type))
 
     # Also produce a type name...
 
     return getattr(ptr.U, extract_map[val_type][0]), extract_map[val_type][1]
+
+  def SetEngineOptions(self, opt):
+    res = self.vt.SetEngineOptions(self.control, opt)
+    aborter(res, "SetEngineOptions")
+    return

debuginfo-tests/dexter/dex/debugger/dbgeng/dbgeng.py

def __init__(self, context, *args):
         self.running = False
         self.finished = False
         self.step_info = None
         super(DbgEng, self).__init__(context, *args)
 
     def _custom_init(self):
         try:
           res = setup.setup_everything(self.context.options.executable)
-          self.client, self.hProcess = res
+          self.client = res
           self.running = True
         except Exception as e:
           raise Exception('Failed to start debuggee: {}'.format(e))
 
     def _custom_exit(self):
-        setup.cleanup(self.client, self.hProcess)
+        setup.cleanup(self.client)
 
     def _load_interface(self):
         arch = platform.architecture()[0]
         machine = platform.machine()
         if arch == '32bit' and machine == 'AMD64':
           # This python process is 32 bits, but is sitting on a 64 bit machine.
           # Bad things may happen, don't support it.
           raise LoadDebuggerException('Can\'t run Dexter dbgeng on 32 bit python in a 64 bit environment')

debuginfo-tests/dexter/dex/debugger/dbgeng/setup.py

def break_on_all_but_main(Control, Symbols, main_offset):
   for name, rec in fetch_local_function_syms(Symbols, prefix):
     if name == "main":
       continue
     bp = Control.AddBreakpoint2(offset=rec.Offset, enabled=True)
 
   # All breakpoints are currently discarded: we just sys.exit for cleanup
   return
 
-def process_creator(binfile):
-  Kernel32 = WinDLL("Kernel32")
-
-  # Another flavour of process creation
-  startupinfoa = STARTUPINFOA()
-  startupinfoa.cb = sizeof(STARTUPINFOA)
-  startupinfoa.lpReserved = None
-  startupinfoa.lpDesktop = None
-  startupinfoa.lpTitle = None
-  startupinfoa.dwX = 0
-  startupinfoa.dwY = 0
-  startupinfoa.dwXSize = 0
-  startupinfoa.dwYSize = 0
-  startupinfoa.dwXCountChars = 0
-  startupinfoa.dwYCountChars = 0
-  startupinfoa.dwFillAttribute = 0
-  startupinfoa.dwFlags = 0
-  startupinfoa.wShowWindow = 0
-  startupinfoa.cbReserved2 = 0
-  startupinfoa.lpReserved2 = None
-  startupinfoa.hStdInput = None
-  startupinfoa.hStdOutput = None
-  startupinfoa.hStdError = None
-  processinformation = PROCESS_INFORMATION()
-
-  # 0x4 below specifies CREATE_SUSPENDED.
-  ret = Kernel32.CreateProcessA(binfile.encode("ascii"), None, None, None, False, 0x4, None, None, byref(startupinfoa), byref(processinformation))
-  if ret == 0:
-    raise Exception('CreateProcess running {}'.format(binfile))
-
-  return processinformation.dwProcessId, processinformation.dwThreadId, processinformation.hProcess, processinformation.hThread
-
-def thread_resumer(hProcess, hThread):
-  Kernel32 = WinDLL("Kernel32")
-
-  # For reasons unclear to me, other suspend-references seem to be opened on
-  # the opened thread. Clear them all.
-  while True:
-    ret = Kernel32.ResumeThread(hThread)
-    if ret <= 0:
-      break
-  if ret < 0:
-    Kernel32.TerminateProcess(hProcess, 1)
-    raise Exception("Couldn't resume process after startup")
-
-  return
-
 def setup_everything(binfile):
   from . import client
   from . import symbols
   Client = client.Client()
 
-  created_pid, created_tid, hProcess, hThread = process_creator(binfile)
+  Client.Control.SetEngineOptions(0x20) # DEBUG_ENGOPT_INITIAL_BREAK
+
+  Client.CreateProcessAndAttach2(binfile)
 
   # Load lines as well as general symbols
   sym_opts = Client.Symbols.GetSymbolOptions()
   sym_opts |= symbols.SymbolOptionFlags.SYMOPT_LOAD_LINES
   Client.Symbols.SetSymbolOptions(sym_opts)
 
-  Client.AttachProcess(created_pid)
+  # Need to enter the debugger engine to let it attach properly.
+  res = Client.Control.WaitForEvent(timeout=1000)
+  if res == S_FALSE:
+    # The debugee apparently didn't do anything at all. Rather than risk
+    # hanging, bail out at this point.
+    client.TerminateProcesses()
+    raise Exception("Debuggee did not start in a timely manner")
 
-  # Need to enter the debugger engine to let it attach properly
-  Client.Control.WaitForEvent(timeout=1)
-  Client.SysObjects.set_current_thread(created_pid, created_tid)
+  # Enable line stepping.
   Client.Control.Execute("l+t")
+  # Enable C++ expression interpretation.
   Client.Control.SetExpressionSyntax(cpp=True)
 
+  # We've requested to break into the process at the earliest opportunity,
+  # and WaitForEvent'ing means we should have reached that break state.
+  # Now set a breakpoint on the main symbol, and "go" until we reach it.
   module_name = Client.Symbols.get_exefile_module_name()
   offset = Client.Symbols.GetOffsetByName("{}!main".format(module_name))
   breakpoint = Client.Control.AddBreakpoint2(offset=offset, enabled=True)
-  thread_resumer(hProcess, hThread)
   Client.Control.SetExecutionStatus(control.DebugStatus.DEBUG_STATUS_GO)
 
   # Problem: there is no guarantee that the client will ever reach main,
   # something else exciting could happen in that time, the host system may
   # be very loaded, and similar. Wait for some period, say, five seconds, and
   # abort afterwards: this is a trade-off between spurious timeouts and
   # completely hanging in the case of a environmental/programming error.
   res = Client.Control.WaitForEvent(timeout=5000)
   if res == S_FALSE:
-    Kernel32.TerminateProcess(hProcess, 1)
+    client.TerminateProcesses()
     raise Exception("Debuggee did not reach main function in a timely manner")
 
   break_on_all_but_main(Client.Control, Client.Symbols, offset)
 
   # Set the default action on all exceptions to be "quit and detach". If we
   # don't, dbgeng will merrily spin at the exception site forever.
   filts = Client.Control.GetNumberEventFilters()
   for x in range(filts[0], filts[0] + filts[1]):
     Client.Control.SetExceptionFilterSecondCommand(x, "qd")
 
-  return Client, hProcess
+  return Client
 
 def step_once(client):
   client.Control.Execute("p")
   try:
     client.Control.WaitForEvent()
   except Exception as e:
     if client.Control.GetExecutionStatus() == control.DebugStatus.DEBUG_STATUS_NO_DEBUGGEE:
       return None # Debuggee has gone away, likely due to an exception.
     raise e
   # Could assert here that we're in the "break" state
   client.Control.GetExecutionStatus()
   return probe_state(client)
 
 def main_loop(client):
   res = True
   while res is not None:
     res = step_once(client)
 
-def cleanup(client, hProcess):
-  res = client.DetachProcesses()
-  Kernel32 = WinDLL("Kernel32")
-  Kernel32.TerminateProcess(hProcess, 1)
+def cleanup(client):
+  client.TerminateProcesses()