Page MenuHomePhabricator

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

Authored by jmorse on Feb 11 2020, 7:28 AM.

Details

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.

Diff Detail

Event Timeline

jmorse created this revision.Feb 11 2020, 7:28 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 11 2020, 7:28 AM
Orlando added inline comments.Feb 11 2020, 8:19 AM
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 added a comment.Feb 11 2020, 11:27 AM

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.

@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 accepted this revision.Feb 12 2020, 7:55 AM

LGTM

This revision is now accepted and ready to land.Feb 12 2020, 7:55 AM
rnk added a comment.Feb 12 2020, 11:08 AM

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.

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 marked an inline comment as done.Feb 13 2020, 3:58 AM
jmorse added inline comments.
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.

This revision was automatically updated to reflect the committed changes.