microsoft / windows-rs

Rust for Windows
https://kennykerr.ca/rust-getting-started/
Apache License 2.0
10.31k stars 483 forks source link

Is it possible to debug the underlying Win32 API call? #1121

Closed ibigbug closed 3 years ago

ibigbug commented 3 years ago

I'm using this binding to call bunch of Win32 APIs most of which work fine.

However I'm having issue with calling CreateProcessW with EXTENDED_STARTUPINFO_PRESENT, the error message from GetLastErr is: ""The parameter is incorrect.", which is useless at all. (I know this is from the Win32).

I've been researching online and see this might be related to a incorrect STARTUPINFOEXW.lpAttributeList setup.

If I don't use EXTENDED_STARTUPINFO_PRESENT, I can create the process.

This is more related to a Win32 question I guess, that's said, is there anyway that I could hook a debugger into the Win32 call or maybe other way to find out what's going on?

ibigbug commented 3 years ago

not suggesting anyone to debug this for me, just to provide more information incase useful, the code is at: https://github.com/ibigbug/PowerSession/blob/ac2841b213ddf9c31d19b0d360d9ec5516ee1d04/terminal/src/windows/process.rs#L102

kennykerr commented 3 years ago

If you have symbols (which are available publicly) you can get a little further and if you have sources to match the symbols (which are not generally available publicly) then you can (with some effort) debug into the Windows API. Generally however, the error codes are sufficient to figure out what went wrong when you combine the error code with the relevant documentation. It's not always ideal but is the best I can suggest. If you suspect there is a problem with the Rust bindings you can also attempt to call the same API from C++ to rule that out as a possibility.

ibigbug commented 3 years ago

thanks.

the error message is quite generic, which can be any reason from my understanding and very hard for me to dig into.

I've been reading the docs in C++ and what I'm doing is basically translating the C++ sample code into identical Rust logic.

I don't suspect the Rust bindings issue, but one thing that I suspect is although the code compiles, how can I make sure all the "translation" is correct? for example, this https://github.com/ibigbug/PowerSession/blob/ac2841b213ddf9c31d19b0d360d9ec5516ee1d04/terminal/src/windows/process.rs#L39 vs. this. https://github.com/microsoft/terminal/blob/3d7480e9b73fb0b5f8d6836fdce77183e04d0b25/samples/ConPTY/EchoCon/EchoCon/EchoCon.cpp#L127

can't see any logic difference in their sample C++ code any my Rust translation, but still the error message isn't really helpful.

riverar commented 3 years ago

This doesn't seem right. You're telling CreateProcess that you're passing in an extended startup information structure, but then passing in that structure's inner non-extended startup info structure.

From the docs:

EXTENDED_STARTUPINFO_PRESENT 0x00080000 - The process is created with extended startup information; the lpStartupInfo parameter specifies a STARTUPINFOEX structure.

        CreateProcessW(
            PWSTR::NULL,
            command,
            null_mut(),
            null_mut(),
            false,
            EXTENDED_STARTUPINFO_PRESENT | CREATE_UNICODE_ENVIRONMENT,
            null_mut(),
            working_dir,
-           &mut startup_info.StartupInfo,
+           &mut startup_info,
            &mut p_info,
        )

(Also check the structure's cb is 72. If not, could be a problem with the bindings.)

ibigbug commented 3 years ago

but then passing in that structure's inner non-extended startup info structure.

It took me a while to understand that, as the CreateProcessW binding only takes lpstartupinfo: *mut STARTUPINFOW, https://microsoft.github.io/windows-docs-rs/doc/bindings/Windows/Win32/System/Threading/fn.CreateProcessW.html

making the change will not compile:

PS D:\projects\PowerSession> cargo b
   Compiling terminal v0.1.0 (D:\projects\PowerSession\terminal)
error[E0308]: mismatched types
   --> terminal\src\windows\process.rs:111:13
    |
111 |             &mut startup_info,
    |             ^^^^^^^^^^^^^^^^^ expected struct `STARTUPINFOW`, found `&mut Threading::STARTUPINFOEXW`
    |
    = note:    expected raw pointer `*mut STARTUPINFOW`
            found mutable reference `&mut &mut Threading::STARTUPINFOEXW`

For more information about this error, try `rustc --explain E0308`.
error: could not compile `terminal` due to previous error

My understanding the reason it works is I'm telling CreateProcess EXTENDED_STARTUPINFO_PRESENT and I pass in the address of the startup_info.StartupInfo, in the CreateProcessW, it will see the EXTENDED_STARTUPINFO_PRESENT flag and the start_info.StartupInfo.cb = std::mem::size_of::<STARTUPINFOEXW>() as u32; is the size of a STARTUPINFOEXW, so that it is confident to read a bit further of the memory after the StartupInfo as the extended data.

(Also check the structure's cb is 72. If not, could be a problem with the bindings.)

for some reason, the cb of STARTUPINFOEXW is showing 112, did I use the size_of wrong? image

riverar commented 3 years ago

My understanding the reason it works is I'm telling CreateProcess EXTENDED_STARTUPINFO_PRESENT and I pass in the address of the startup_info.StartupInfo, in the CreateProcessW, it will see the EXTENDED_STARTUPINFO_PRESENT flag and the start_info.StartupInfo.cb = std::mem::size_of::<STARTUPINFOEXW>() as u32; is the size of a STARTUPINFOEXW, so that it is confident to read a bit further of the memory after the StartupInfo as the extended data.

That should work, true. ~Does hardcoding 72 still result in error? (The structure may not be laid out in memory correctly, so no guarantees of course.)~

riverar commented 3 years ago

Sorry, didn't finish my coffee and realized I was doing a sizeof on ARM target 🤡 which is 72. 112 is correct for amd64 targets.

riverar commented 3 years ago

I'll take a deeper look at this for you today.

On the topic of debugging, Rust bindings / OS APIs are very debuggable with WinDbg. This is not easy to do; you'll need knowledge in some assembly and Windows internals. There is no code or debug spew to look at.

The generic workflow would be something like:

  1. get the target loaded into the debugger
  2. ensure Windows symbol resolution is ready to go (e.g. .symfix X:\Symbols)
  3. set a breakpoint in your app, or for CreateProcessW (e.g. bp kernel32!CreateProcessW)
  4. start the app and begin your adventure

Alternatively, you can create a simple repro repository and share that with folks on StackOverflow or Microsoft Q&A (hint: tag me) and get someone else to help out. Debugging As A Service ™ 🤠

ibigbug commented 3 years ago

thanks :)

the steps are very useful, I'll take a try and see what I can find as well as create a minimum repro

ibigbug commented 3 years ago

@riverar created a post on https://docs.microsoft.com/en-us/answers/questions/548558/windows-rs-createprocessw-with-extended-startupinf.html

and SO: https://stackoverflow.com/q/69152395/1109167

ibigbug commented 3 years ago

latest update: the CreateProcessW can start "notepad.exe" with no issue, but will fail with "powershell.exe"/"cmd.exe"/"ping.exe".

regarding this, does it make this look closer like a binding issue, or even further away to be the usage of the API?

riverar commented 3 years ago

@ibigbug Yep, was able to get this working with Notepad too so the bindings look okay. Tagging @DHowett for awareness; Microsoft could use better documentation around this flag in general.

ibigbug commented 3 years ago

@riverar thanks. I agree on the general documentation. I've had several experiences having trouble using MSFT docs and in this particular case, I think we are somehow landing on the std io handles related stuff is the cause, but I can't neither check out the source code of CreateProcessW to see what does it require or what could go wrong, nor learn the potential pitfalls from reading the doc - only thing I can do is blindly guess and trial and errors, and unfoturenately the community doesn't have a lot information either.

ibigbug commented 3 years ago

Update, I found the problem, I had to:

    success = unsafe {
        UpdateProcThreadAttribute(
            si.lpAttributeList,
            0,
            PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE,
-           (&mut h_pc as *mut HPCON).cast::<_>(),
+            h_pc.0 as _,

to make it is able to launch the process.

so why (&mut h_pc as *mut HPCON).cast::<_>(), wouldn't work with the binding?