Fix #176. Use the CRT's signal handling on native Win32 processes.

[hyc]

This allows the 7 signals that Microsoft's C runtime supports to be used, instead of only Ctrl-C/SIGINT.

This is just a slightly refreshed version of the patch I wrote for MSYS1 twenty years ago. https://sourceforge.net/p/mingw/bugs/1783/

It only looks for ucrtbase.dll and msvcrt.dll now, and it doesn't worry about 32bit vs 64bit processes since 32bit Windows is long obsolete.

[hyc]

Could also just do this in a standalone kill program if you wanted that instead.

wkill.c

[dscho]

It only looks for ucrtbase.dll and msvcrt.dll now, and it doesn't worry about 32bit vs 64bit processes since 32bit Windows is long obsolete.

This is a non-sequitur. While the 32-bit Windows is long obsolete, indeed, 32-bit Windows processes are not.

	    WriteProcessMemory(ch_spawn,pExecedHook,&rsig,sizeof(rsig),&nbytes);

This is a dark pattern, and is likely to land a MSYS2 runtime that uses that pattern much more likely in quarantine than before, and rightfully so. I know that we inject a remote thread to send signals. The difference is that we go out of our way not to inject executable code, but to call well-known entry points (CtrlRoutine/ExitProcess) instead.

Seeing that there is already this pattern, I have to wonder why you didn't simply choose to use the very same method: raise() accepts an int, which is compatible with the type of the lpParameter we can pass to the remote thread start routine. So: why go out of your way to ignore the existing code pattern and invent a new, incompatible one instead of imitating it (passing the address of raise() as the lpStartAddress)?

[hyc]

Seeing that there is already this pattern, I have to wonder why you didn't simply choose to use the very same method: raise() accepts an int, which is compatible with the type of the lpParameter we can pass to the remote thread start routine. So: why go out of your way to ignore the existing code pattern and invent a new, incompatible one instead of imitating it (passing the address of raise() as the lpStartAddress)?

That's not correct. The parameter to a ThreadProc is a pointer. As raise() takes an int, and pointer and int are different sizes, you can't just use raise() itself as the start address.

https://learn.microsoft.com/en-us/previous-versions/windows/desktop/legacy/ms686736(v=vs.85)

Also, you can't rely on just knowing the address of raise() in the target process; the only way to know it for certain is to use GetProcAddress in the target process. Especially if the target process is using a CRT DLL that the calling process doesn't use.

As for the other comment, I can still look into 32bit support. Just makes the build a little more complicated.

[dscho]

Seeing that there is already this pattern, I have to wonder why you didn't simply choose to use the very same method: raise() accepts an int, which is compatible with the type of the lpParameter we can pass to the remote thread start routine. So: why go out of your way to ignore the existing code pattern and invent a new, incompatible one instead of imitating it (passing the address of raise() as the lpStartAddress)?

That's not correct. The parameter to a ThreadProc is a pointer. As raise() takes an int, and pointer and int are different sizes, you can't just use raise() itself as the start address.

In this context, the pointer type is always at least as wide as the int type. So there needs to be at least a whiff of some thought having been put behind this change intentionally disagreeing with the existing code pattern.

However, the most worrisome part about this PR is the continued insistence on a coding pattern that is prone to land the MSYS2 runtime into pretty much every anti-malware's quarantine, even the ones which are graciously absent from the list of common false positive generators.

You are not addressing this issue. This code change will make the MSYS2 runtime look like malware. It will cause more trouble than we want to buy into.

[rimrul]

While the 32-bit Windows is long obsolete, indeed, 32-bit Windows processes are not.

That's not even all. On Windows 11 On ARM we need to consider x86, x86-64, ARM32 (until 24H2), ARM64 and ARM64EC processes.

Edit for clarity: by "need to consider" I don't necessarily mean that we need to use their CRTs signal handling, just that we need to be aware we can encounter them and should have some fallback for when we do.

[hyc]

In this context, the pointer type is always at least as wide as the int type. So there needs to be at least a whiff of some thought having been put behind this change intentionally disagreeing with the existing code pattern.

Don't be an ass. Of course this was thought about. And again, you can't reliably know the address of raise() in the target process without sending custom code over to find it. And this pattern isn't "new" - it was accepted in MSYS1 20 years ago.

[dscho]

Don't be an ass.

I bow out of this conversation. If you cannot converse with me respectfully, you cannot develop code with me.

If others with write permission to this repository want to continue discussing this change, I won't stop them. Otherwise I will close this PR.

[hyc]

If you cannot converse with me respectfully, you cannot develop code with me.

If you question whether thought was put into a contribution, it is you who needs to be more respectful.