Page MenuHomePhabricator

[lldb] Add a testcase for nested exceptions on Windows
Needs ReviewPublic

Authored by mstorsjo on Jun 23 2022, 1:48 AM.

Details

Summary

This adds a testcase for
4d123783957e547009e55346bf3a8ae43a88fa14 / D128201. Before that commit,
lldb would crash here, while now lldb manages to stop after the
exception.

Diff Detail

Event Timeline

mstorsjo created this revision.Jun 23 2022, 1:48 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 23 2022, 1:48 AM
mstorsjo requested review of this revision.Jun 23 2022, 1:48 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 23 2022, 1:48 AM

For this testcase to work, it needs to be able to open a window - so it can't run entirely in headless mode. But I guess that a bunch of other tests in LLDB also already do that, since running the LLDB tests on Windows pops up a dozen of windows temporarily.

For this testcase to work, it needs to be able to open a window - so it can't run entirely in headless mode. But I guess that a bunch of other tests in LLDB also already do that, since running the LLDB tests on Windows pops up a dozen of windows temporarily.

I think those just come from the tests (or more like test runners) which forget to suppress a console window from opening (see LLDB_LAUNCH_INFERIORS_WITHOUT_CONSOLE) -- this is probably the first "gui" application. This isn't necessarily a showstopper, but I am surprised that it is necessary. I'm hardly a windows expert, but I would expect that it should be possible to generate an exception (even nested exception, which to me sounds like the equivalent of getting a signal inside a signal handler) in a "regular" text-mode application. Are you sure that is not possible?

lldb/test/Shell/Process/Windows/wndproc_exception.cpp
7

Is there something reasonable we could assert here? The process exit status for instance?

For this testcase to work, it needs to be able to open a window - so it can't run entirely in headless mode. But I guess that a bunch of other tests in LLDB also already do that, since running the LLDB tests on Windows pops up a dozen of windows temporarily.

I think those just come from the tests (or more like test runners) which forget to suppress a console window from opening (see LLDB_LAUNCH_INFERIORS_WITHOUT_CONSOLE) -- this is probably the first "gui" application. This isn't necessarily a showstopper, but I am surprised that it is necessary. I'm hardly a windows expert, but I would expect that it should be possible to generate an exception (even nested exception, which to me sounds like the equivalent of getting a signal inside a signal handler) in a "regular" text-mode application. Are you sure that is not possible?

I didn't say that this isn't necessarily possible - this is the reduced testcase that @alvinhochun provided in https://github.com/mstorsjo/llvm-mingw/issues/292#issuecomment-1160239522 (that I shrunk down further a bit).

I'm not all that familiar with exactly what happens in these APIs here that makes this a nested exception and what other non-GUI APIs would produce the same effect. Maybe any API where Windows system code calls back to a user provided callback? Does @stella.stamenova know?

lldb/test/Shell/Process/Windows/wndproc_exception.cpp
7

This in itself requires the %lldb command to succeed - the exit status for each RUN line needs to be successful (unless a failure is expected and it can be inverted with the not command). Or did you mean checking the process exit code of the child process (that intentionally does crash)?

When this test case is run, it prints the following to the terminal:

(lldb) run
Process 3168 stopped
* thread #1, stop reason = Exception 0xc000041d encountered at address 0x7ff68e6f1227
    frame #0: 0x00007ff68e6f1227 wndproc_exception.cpp.tmp.exe
->  0x7ff68e6f1227: movl   $0x1, (%rax)
    0x7ff68e6f122d: movq   $0x0, 0x50(%rsp)
    0x7ff68e6f1236: jmp    0x7ff68e6f1259
    0x7ff68e6f123b: movq   0x48(%rsp), %r9
Process 3168 launched: 'C:\dev\llvm-project\llvm\build-msvc\tools\lldb\test\Shell\Process\Windows\Output\wndproc_exception.cpp.tmp.exe' (x86_64)

I guess we could check for Exception 0xc000041d encountered maybe, although I'm afraid of making the testcase unnecessarily brittle too.

mstorsjo added inline comments.Jun 23 2022, 2:42 AM
lldb/test/Shell/Process/Windows/wndproc_exception.cpp
7

If running with lldb-server enabled, it prints a different exception code, 0xc0000005 (which is STATUS_ACCESS_VIOLATION) which probably is the proper nested exception, while without lldb-server, it prints 0xc000041d (STATUS_FATAL_USER_CALLBACK_EXCEPTION).

So for the purpose of this testcase, just to make sure that it doesn't crash in this case, it'd be better to not specify exactly which exception code is to be returned.

I'm not all that familiar with exactly what happens in these APIs here that makes this a nested exception and what other non-GUI APIs would produce the same effect. Maybe any API where Windows system code calls back to a user provided callback? Does @stella.stamenova know?

FWIW, I tested with doing a similar crash in the callback from EnumerateLoadedModules, but that doesn't trigger the same case.

I tested running the lldb tests within docker, which is a headless windows environment, and this test does indeed fail there (it hangs).

It may be possible to stuff a pointer to an EXCEPTION_RECORD into another EXCEPTION_RECORD and use RtlRaiseException to generate the exception, but you'll have to test how it actually works.

alvinhochun added inline comments.Jun 23 2022, 6:39 AM
lldb/test/Shell/Process/Windows/wndproc_exception.cpp
7

It may be that lldb-server is catching the first chance STATUS_ACCESS_VIOLATION exception before the exception is passed to the exception handler...

alvinhochun added inline comments.Jun 23 2022, 7:31 AM
lldb/test/Shell/Process/Windows/wndproc_exception.cpp
7

If you test with windbg or gdb, you will get the access violation / sigsegv first, and only after continuing the debuggee you will get the 0xc000041d exception.

It may be possible to stuff a pointer to an EXCEPTION_RECORD into another EXCEPTION_RECORD and use RtlRaiseException to generate the exception, but you'll have to test how it actually works.

That would be pretty cool.

lldb/test/Shell/Process/Windows/wndproc_exception.cpp
7

This in itself requires the %lldb command to succeed - the exit status for each RUN line needs to be successful (unless a failure is expected and it can be inverted with the not command). Or did you mean checking the process exit code of the child process (that intentionally does crash)?

Yes, that's what I mean, but I don't think we need to match the precise exception number (although probably only one of those two numbers is really "correct"). We could just match the stop reason = Exception part.

It may be possible to stuff a pointer to an EXCEPTION_RECORD into another EXCEPTION_RECORD and use RtlRaiseException to generate the exception, but you'll have to test how it actually works.

That would be pretty cool.

Yeah - I guess it's two separate kinds of testcases; this one would be more of a macro-testcase, "does this real-world case work - whichever way lldb happens to handle it" (nested exception or not?) while that would be more of a clinical unit test for specifically testing nested exceptions.

It may be possible to stuff a pointer to an EXCEPTION_RECORD into another EXCEPTION_RECORD and use RtlRaiseException to generate the exception, but you'll have to test how it actually works.

That would be pretty cool.

Yeah - I guess it's two separate kinds of testcases; this one would be more of a macro-testcase, "does this real-world case work - whichever way lldb happens to handle it" (nested exception or not?) while that would be more of a clinical unit test for specifically testing nested exceptions.

That's true. However, if I had to choose between the two, I would always go for the one with the fewest moving parts. Lldb has a lot of problems with reproducibility of tests, so I am always looking for ways to make tests more specific.