all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* SIGTRAP in kill emulation on Windows
@ 2016-08-25 15:08 Alain Schneble
  2016-08-25 16:18 ` Eli Zaretskii
  0 siblings, 1 reply; 12+ messages in thread
From: Alain Schneble @ 2016-08-25 15:08 UTC (permalink / raw)
  To: emacs-devel

[-- Attachment #1: Type: text/plain, Size: 323 bytes --]

Hello

The following patch provides support for SIGTRAP in the kill emulation
on Windows.  It implements mapping SIGTRAP to a call to
DebugBreakProcess in C code.  Patches that implement the discussed use
cases in LISP will be submitted separately as this change is useful on
its own, e.g. (signal-process PROCESS 'TRAP).


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: SIGTRAP in kill emulation on Windows --]
[-- Type: text/x-patch, Size: 2331 bytes --]

From fca0f7d3969ee3ad68046e3a0a2d4ccfef738692 Mon Sep 17 00:00:00 2001
From: Alain Schneble <a.s@realize.ch>
Date: Thu, 25 Aug 2016 15:19:09 +0200
Subject: [PATCH] Support SIGTRAP in kill emulation on Windows

* src/w32proc.c (sys_kill): Translate SIGTRAP signal into a call to
  DebugBreakProcess to cause a breakpoint exception to occur in the
  specified process. Windows versions prior to Windows XP that do not
  support DebugBreakProcess return -1 and errno set to EINVAL as before.
---
 src/w32proc.c | 37 ++++++++++++++++++++++++++++++++++---
 1 file changed, 34 insertions(+), 3 deletions(-)

diff --git a/src/w32proc.c b/src/w32proc.c
index 11a121f..ab2a667 100644
--- a/src/w32proc.c
+++ b/src/w32proc.c
@@ -2507,9 +2507,9 @@ sys_kill (pid_t pid, int sig)
   if (pid < 0)
     pid = -pid;
 
-  /* Only handle signals that will result in the process dying */
+  /* Only handle signals that can be mapped to a similar behavior on Windows */
   if (sig != 0
-      && sig != SIGINT && sig != SIGKILL && sig != SIGQUIT && sig != SIGHUP)
+      && sig != SIGINT && sig != SIGKILL && sig != SIGQUIT && sig != SIGHUP && sig != SIGTRAP)
     {
       errno = EINVAL;
       return -1;
@@ -2552,7 +2552,11 @@ sys_kill (pid_t pid, int sig)
 	 close the selected frame, which does not necessarily
 	 terminates Emacs.  But then we are not supposed to call
 	 sys_kill with our own PID.  */
-      proc_hand = OpenProcess (PROCESS_TERMINATE, 0, pid);
+
+      DWORD desiredAccess =
+	(sig == SIGTRAP) ? PROCESS_ALL_ACCESS : PROCESS_TERMINATE;
+
+      proc_hand = OpenProcess (desiredAccess, 0, pid);
       if (proc_hand == NULL)
         {
 	  errno = EPERM;
@@ -2648,6 +2652,33 @@ sys_kill (pid_t pid, int sig)
 	  rc = -1;
 	}
     }
+  else if (sig == SIGTRAP)
+    {
+#if _WIN32_WINNT >= _WIN32_WINNT_WINXP
+      if (!DebugBreakProcess (proc_hand))
+	{
+	  DWORD err = GetLastError ();
+
+	  DebPrint (("sys_kill.DebugBreakProcess return %d "
+		     "for pid %lu\n", err, pid));
+
+	  switch (err)
+	    {
+	    case ERROR_ACCESS_DENIED:
+	      errno = EPERM;
+	      break;
+	    default:
+	      errno = EINVAL;
+	      break;
+	    }
+
+	  rc = -1;
+	}
+#else
+      errno = EINVAL;
+      rc = -1;
+#endif
+    }
   else
     {
       if (NILP (Vw32_start_process_share_console) && cp && cp->hwnd)
-- 
2.8.1.windows.1


[-- Attachment #3: Type: text/plain, Size: 54 bytes --]


Many thanks for considering and reviewing it.
Alain


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: SIGTRAP in kill emulation on Windows
  2016-08-25 15:08 SIGTRAP in kill emulation on Windows Alain Schneble
@ 2016-08-25 16:18 ` Eli Zaretskii
  2016-08-25 20:02   ` Alain Schneble
  0 siblings, 1 reply; 12+ messages in thread
From: Eli Zaretskii @ 2016-08-25 16:18 UTC (permalink / raw)
  To: Alain Schneble; +Cc: emacs-devel

> From: Alain Schneble <a.s@realize.ch>
> Date: Thu, 25 Aug 2016 17:08:51 +0200
> 
> The following patch provides support for SIGTRAP in the kill emulation
> on Windows.  It implements mapping SIGTRAP to a call to
> DebugBreakProcess in C code.  Patches that implement the discussed use
> cases in LISP will be submitted separately as this change is useful on
> its own, e.g. (signal-process PROCESS 'TRAP).

Thanks.  I think this needs an entry in NEWS.

> +  else if (sig == SIGTRAP)
> +    {
> +#if _WIN32_WINNT >= _WIN32_WINNT_WINXP
> +      if (!DebugBreakProcess (proc_hand))

We don't put in Emacs sources compile-time conditionals that depend on
the version of the OS on which Emacs is built: that would preclude the
(very popular on Windows) practice of building Emacs on one system,
and then using it on many others, possibly running other versions of
the OS.

Instead, functions that might be unavailable at run time are called
through a function pointer, which is assigned the value when it is
first needed, by looking up the function address in the corresponding
DLL (and loading the DLL if needed, which is not the case here).
Please see an example of how that is done in
w32proc.c:w32_compare_strings (and in many places in w32.c).

> +	{
> +	  DWORD err = GetLastError ();
> +
> +	  DebPrint (("sys_kill.DebugBreakProcess return %d "
> +		     "for pid %lu\n", err, pid));
> +
> +	  switch (err)
> +	    {
> +	    case ERROR_ACCESS_DENIED:
> +	      errno = EPERM;
> +	      break;
> +	    default:
> +	      errno = EINVAL;
> +	      break;
> +	    }
> +
> +	  rc = -1;
> +	}
> +#else
> +      errno = EINVAL;
> +      rc = -1;

When the function is not available, you should assign ENOTSUP to
errno, not EINVAL, to make the error message more accurate.

Please test what this functionality does when PID specifies a child
process of Emacs (which is tracked via the child_procs[] array), and
also when it specifies the calling Emacs process itself.  If any of
these use cases cause any kind of trouble, we may wish to disallow
such usage, to prevent users from shooting themselves in the foot.



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: SIGTRAP in kill emulation on Windows
  2016-08-25 16:18 ` Eli Zaretskii
@ 2016-08-25 20:02   ` Alain Schneble
  2016-08-25 20:09     ` Alain Schneble
  2016-08-26  8:24     ` Eli Zaretskii
  0 siblings, 2 replies; 12+ messages in thread
From: Alain Schneble @ 2016-08-25 20:02 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

[-- Attachment #1: Type: text/plain, Size: 1689 bytes --]

Eli Zaretskii <eliz@gnu.org> writes:

> Thanks.  I think this needs an entry in NEWS.

Done.

> We don't put in Emacs sources compile-time conditionals that depend on
> the version of the OS on which Emacs is built: that would preclude the
> (very popular on Windows) practice of building Emacs on one system,
> and then using it on many others, possibly running other versions of
> the OS.

Thanks for this explanation and sorry that I didn't thought about this.
I changed it to use run-time dynamic linking.

> When the function is not available, you should assign ENOTSUP to
> errno, not EINVAL, to make the error message more accurate.

Done.

> Please test what this functionality does when PID specifies a child
> process of Emacs (which is tracked via the child_procs[] array), and
> also when it specifies the calling Emacs process itself.  If any of
> these use cases cause any kind of trouble, we may wish to disallow
> such usage, to prevent users from shooting themselves in the foot.

Sending SIGTRAP to a process -- whether it's an unrelated, child or the
calling Emacs process itself -- does not seem to have any effect as long
as no debugger is attached to the receiving process.  At least this is
what I observe on Windows 10.  Shall I test on other OS versions as
well?  If so, I would have to install say an Win XP first as I do not
have one available right now.  Unfortunately, on MSDN the remark on
DebugBreakProcess says it causes the receiving process to terminate in
most cases, see
https://msdn.microsoft.com/en-us/library/windows/desktop/ms679298(v=vs.85).aspx

But with all processes I tried, none of them was ever terminated.

Many thanks for your help.
Alain


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Support SIGTRAP in kill emulation on Windows --]
[-- Type: text/x-patch, Size: 5175 bytes --]

From ecf00b52957bb0bf15735180a67cc9ce7027ff19 Mon Sep 17 00:00:00 2001
From: Alain Schneble <a.s@realize.ch>
Date: Thu, 25 Aug 2016 21:31:48 +0200
Subject: [PATCH] Support SIGTRAP in kill emulation on Windows

* src/w32proc.c (sys_kill): Translate SIGTRAP signal into a call to
DebugBreakProcess to cause a breakpoint exception to occur in the
specified process.  Windows versions prior to Windows XP that do not
support DebugBreakProcess return -1 and errno set to ENOTSUP (as opposed
to EINVAL before this change).
* src/w32proc.c: Add typedef for DebugBreakProcess function pointer and
global variable to track state of run-time dynamic linking of this
function.
* etc/NEWS: Add entry to document that 'signal-process' now supports
SIGTRAP.
---
 etc/NEWS      |  8 ++++++++
 src/w32.c     |  3 +++
 src/w32proc.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++++---
 3 files changed, 60 insertions(+), 3 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 494a091..3d055fc 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -599,6 +599,14 @@ still apply.)
 Previously, on MS-Windows this function converted slash characters in
 file names into backslashes.  It no longer does that.
 
+---
+** 'signal-process' supports SIGTRAP on Windows XP and later.
+The 'kill' emulation on Windows now maps SIGTRAP to a call to Win32
+'DebugBreakProcess'.  This causes the receiving process to break
+execution and return control to the debugger.  If no debugger is
+attached to the receiving process, the call is typically ignored by
+the receiving process.
+
 \f
 * Installation Changes in Emacs 25.1
 
diff --git a/src/w32.c b/src/w32.c
index 1db3426..355e7ca 100644
--- a/src/w32.c
+++ b/src/w32.c
@@ -330,6 +330,7 @@ static BOOL g_b_init_set_named_security_info_a;
 static BOOL g_b_init_get_adapters_info;
 
 BOOL g_b_init_compare_string_w;
+BOOL g_b_init_debug_break_process;
 
 /*
   BEGIN: Wrapper functions around OpenProcessToken
@@ -9636,6 +9637,7 @@ globals_of_w32 (void)
   g_b_init_get_process_working_set_size = 0;
   g_b_init_global_memory_status = 0;
   g_b_init_global_memory_status_ex = 0;
+  g_b_init_debug_break_process = 0;
   g_b_init_equal_sid = 0;
   g_b_init_copy_sid = 0;
   g_b_init_get_length_sid = 0;
@@ -9653,6 +9655,7 @@ globals_of_w32 (void)
   g_b_init_set_named_security_info_a = 0;
   g_b_init_get_adapters_info = 0;
   g_b_init_compare_string_w = 0;
+  g_b_init_debug_break_process = 0;
   num_of_processors = 0;
   /* The following sets a handler for shutdown notifications for
      console apps. This actually applies to Emacs in both console and
diff --git a/src/w32proc.c b/src/w32proc.c
index 11a121f..bb637bc 100644
--- a/src/w32proc.c
+++ b/src/w32proc.c
@@ -66,6 +66,8 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 	    + (filedata).file_base))
 
 extern BOOL g_b_init_compare_string_w;
+extern BOOL g_b_init_debug_break_process;
+
 int sys_select (int, SELECT_TYPE *, SELECT_TYPE *, SELECT_TYPE *,
 		struct timespec *, void *);
 
@@ -2494,6 +2496,9 @@ find_child_console (HWND hwnd, LPARAM arg)
   return TRUE;
 }
 
+typedef BOOL (WINAPI * DebugBreakProcess_Proc) (
+    HANDLE hProcess);
+
 /* Emulate 'kill', but only for other processes.  */
 int
 sys_kill (pid_t pid, int sig)
@@ -2507,9 +2512,9 @@ sys_kill (pid_t pid, int sig)
   if (pid < 0)
     pid = -pid;
 
-  /* Only handle signals that will result in the process dying */
+  /* Only handle signals that can be mapped to a similar behavior on Windows */
   if (sig != 0
-      && sig != SIGINT && sig != SIGKILL && sig != SIGQUIT && sig != SIGHUP)
+      && sig != SIGINT && sig != SIGKILL && sig != SIGQUIT && sig != SIGHUP && sig != SIGTRAP)
     {
       errno = EINVAL;
       return -1;
@@ -2552,7 +2557,11 @@ sys_kill (pid_t pid, int sig)
 	 close the selected frame, which does not necessarily
 	 terminates Emacs.  But then we are not supposed to call
 	 sys_kill with our own PID.  */
-      proc_hand = OpenProcess (PROCESS_TERMINATE, 0, pid);
+
+      DWORD desiredAccess =
+	(sig == SIGTRAP) ? PROCESS_ALL_ACCESS : PROCESS_TERMINATE;
+
+      proc_hand = OpenProcess (desiredAccess, 0, pid);
       if (proc_hand == NULL)
         {
 	  errno = EPERM;
@@ -2648,6 +2657,43 @@ sys_kill (pid_t pid, int sig)
 	  rc = -1;
 	}
     }
+  else if (sig == SIGTRAP)
+    {
+      static DebugBreakProcess_Proc s_pfn_Debug_Break_Process = NULL;
+
+      if (g_b_init_debug_break_process == 0)
+	{
+	  g_b_init_debug_break_process = 1;
+	  s_pfn_Debug_Break_Process = (DebugBreakProcess_Proc)
+	    GetProcAddress (GetModuleHandle ("kernel32.dll"),
+			    "DebugBreakProcess");
+	}
+
+      if (s_pfn_Debug_Break_Process == NULL)
+	{
+	  errno = ENOTSUP;
+	  rc = -1;
+	}
+      else if (!s_pfn_Debug_Break_Process (proc_hand))
+	{
+	  DWORD err = GetLastError ();
+
+	  DebPrint (("sys_kill.DebugBreakProcess return %d "
+		     "for pid %lu\n", err, pid));
+
+	  switch (err)
+	    {
+	    case ERROR_ACCESS_DENIED:
+	      errno = EPERM;
+	      break;
+	    default:
+	      errno = EINVAL;
+	      break;
+	    }
+
+	  rc = -1;
+	}
+    }
   else
     {
       if (NILP (Vw32_start_process_share_console) && cp && cp->hwnd)
-- 
2.8.1.windows.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: SIGTRAP in kill emulation on Windows
  2016-08-25 20:02   ` Alain Schneble
@ 2016-08-25 20:09     ` Alain Schneble
  2016-08-26  8:24     ` Eli Zaretskii
  1 sibling, 0 replies; 12+ messages in thread
From: Alain Schneble @ 2016-08-25 20:09 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

[-- Attachment #1: Type: text/plain, Size: 107 bytes --]

Alain Schneble <a.s@realize.ch> writes:

Sorry, the former patch was not the latest version.  This is it:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Support SIGTRAP in kill emulation on Windows --]
[-- Type: text/x-patch, Size: 4883 bytes --]

From a0945f367998a12b0b1150c9105ae262664f4171 Mon Sep 17 00:00:00 2001
From: Alain Schneble <a.s@realize.ch>
Date: Thu, 25 Aug 2016 22:00:49 +0200
Subject: [PATCH] Support SIGTRAP in kill emulation on Windows

* src/w32proc.c (sys_kill): Translate SIGTRAP signal into a call to
DebugBreakProcess to cause a breakpoint exception to occur in the
specified process.  Windows versions prior to Windows XP that do not
support DebugBreakProcess return -1 and errno set to ENOTSUP (as opposed
to EINVAL before this change).
* src/w32proc.c: Add typedef for DebugBreakProcess function pointer and
global variable to track state of run-time dynamic linking of this
function.
* etc/NEWS: Add entry to document that 'signal-process' now supports
SIGTRAP.
---
 etc/NEWS      |  8 ++++++++
 src/w32.c     |  2 ++
 src/w32proc.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++++---
 3 files changed, 59 insertions(+), 3 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 494a091..3d055fc 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -599,6 +599,14 @@ still apply.)
 Previously, on MS-Windows this function converted slash characters in
 file names into backslashes.  It no longer does that.
 
+---
+** 'signal-process' supports SIGTRAP on Windows XP and later.
+The 'kill' emulation on Windows now maps SIGTRAP to a call to Win32
+'DebugBreakProcess'.  This causes the receiving process to break
+execution and return control to the debugger.  If no debugger is
+attached to the receiving process, the call is typically ignored by
+the receiving process.
+
 \f
 * Installation Changes in Emacs 25.1
 
diff --git a/src/w32.c b/src/w32.c
index 1db3426..0f22387 100644
--- a/src/w32.c
+++ b/src/w32.c
@@ -330,6 +330,7 @@ static BOOL g_b_init_set_named_security_info_a;
 static BOOL g_b_init_get_adapters_info;
 
 BOOL g_b_init_compare_string_w;
+BOOL g_b_init_debug_break_process;
 
 /*
   BEGIN: Wrapper functions around OpenProcessToken
@@ -9653,6 +9654,7 @@ globals_of_w32 (void)
   g_b_init_set_named_security_info_a = 0;
   g_b_init_get_adapters_info = 0;
   g_b_init_compare_string_w = 0;
+  g_b_init_debug_break_process = 0;
   num_of_processors = 0;
   /* The following sets a handler for shutdown notifications for
      console apps. This actually applies to Emacs in both console and
diff --git a/src/w32proc.c b/src/w32proc.c
index 11a121f..bb637bc 100644
--- a/src/w32proc.c
+++ b/src/w32proc.c
@@ -66,6 +66,8 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 	    + (filedata).file_base))
 
 extern BOOL g_b_init_compare_string_w;
+extern BOOL g_b_init_debug_break_process;
+
 int sys_select (int, SELECT_TYPE *, SELECT_TYPE *, SELECT_TYPE *,
 		struct timespec *, void *);
 
@@ -2494,6 +2496,9 @@ find_child_console (HWND hwnd, LPARAM arg)
   return TRUE;
 }
 
+typedef BOOL (WINAPI * DebugBreakProcess_Proc) (
+    HANDLE hProcess);
+
 /* Emulate 'kill', but only for other processes.  */
 int
 sys_kill (pid_t pid, int sig)
@@ -2507,9 +2512,9 @@ sys_kill (pid_t pid, int sig)
   if (pid < 0)
     pid = -pid;
 
-  /* Only handle signals that will result in the process dying */
+  /* Only handle signals that can be mapped to a similar behavior on Windows */
   if (sig != 0
-      && sig != SIGINT && sig != SIGKILL && sig != SIGQUIT && sig != SIGHUP)
+      && sig != SIGINT && sig != SIGKILL && sig != SIGQUIT && sig != SIGHUP && sig != SIGTRAP)
     {
       errno = EINVAL;
       return -1;
@@ -2552,7 +2557,11 @@ sys_kill (pid_t pid, int sig)
 	 close the selected frame, which does not necessarily
 	 terminates Emacs.  But then we are not supposed to call
 	 sys_kill with our own PID.  */
-      proc_hand = OpenProcess (PROCESS_TERMINATE, 0, pid);
+
+      DWORD desiredAccess =
+	(sig == SIGTRAP) ? PROCESS_ALL_ACCESS : PROCESS_TERMINATE;
+
+      proc_hand = OpenProcess (desiredAccess, 0, pid);
       if (proc_hand == NULL)
         {
 	  errno = EPERM;
@@ -2648,6 +2657,43 @@ sys_kill (pid_t pid, int sig)
 	  rc = -1;
 	}
     }
+  else if (sig == SIGTRAP)
+    {
+      static DebugBreakProcess_Proc s_pfn_Debug_Break_Process = NULL;
+
+      if (g_b_init_debug_break_process == 0)
+	{
+	  g_b_init_debug_break_process = 1;
+	  s_pfn_Debug_Break_Process = (DebugBreakProcess_Proc)
+	    GetProcAddress (GetModuleHandle ("kernel32.dll"),
+			    "DebugBreakProcess");
+	}
+
+      if (s_pfn_Debug_Break_Process == NULL)
+	{
+	  errno = ENOTSUP;
+	  rc = -1;
+	}
+      else if (!s_pfn_Debug_Break_Process (proc_hand))
+	{
+	  DWORD err = GetLastError ();
+
+	  DebPrint (("sys_kill.DebugBreakProcess return %d "
+		     "for pid %lu\n", err, pid));
+
+	  switch (err)
+	    {
+	    case ERROR_ACCESS_DENIED:
+	      errno = EPERM;
+	      break;
+	    default:
+	      errno = EINVAL;
+	      break;
+	    }
+
+	  rc = -1;
+	}
+    }
   else
     {
       if (NILP (Vw32_start_process_share_console) && cp && cp->hwnd)
-- 
2.8.1.windows.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: SIGTRAP in kill emulation on Windows
  2016-08-25 20:02   ` Alain Schneble
  2016-08-25 20:09     ` Alain Schneble
@ 2016-08-26  8:24     ` Eli Zaretskii
  2016-08-26  8:58       ` Alain Schneble
  1 sibling, 1 reply; 12+ messages in thread
From: Eli Zaretskii @ 2016-08-26  8:24 UTC (permalink / raw)
  To: Alain Schneble; +Cc: emacs-devel

> From: Alain Schneble <a.s@realize.ch>
> CC: <emacs-devel@gnu.org>
> Date: Thu, 25 Aug 2016 22:02:37 +0200
> 
> > Please test what this functionality does when PID specifies a child
> > process of Emacs (which is tracked via the child_procs[] array), and
> > also when it specifies the calling Emacs process itself.  If any of
> > these use cases cause any kind of trouble, we may wish to disallow
> > such usage, to prevent users from shooting themselves in the foot.
> 
> Sending SIGTRAP to a process -- whether it's an unrelated, child or the
> calling Emacs process itself -- does not seem to have any effect as long
> as no debugger is attached to the receiving process.

Do you see the same with the debugbreak program you've built earlier?
If not, then that program does something that your patch doesn't.

Also, were all the programs you tried built with MinGW?  If so,
perhaps try with other programs, like Python or one of the programs
that came with Windows.  I think the results of the call depend on the
exception handlers installed by the program when it starts, so
different programs and different ways of building programs might have
different effects.

And finally, what happens if you signal the calling Emacs process that
is itself being debugged?  I guess the debugger will kick in, but if
you then continue execution, does Emacs continues to run normally?

> At least this is what I observe on Windows 10.  Shall I test on
> other OS versions as well?

We could ask people here to test on other versions and report their
results.

> If so, I would have to install say an Win XP first as I do not
> have one available right now.

I could test on XP here.

> Unfortunately, on MSDN the remark on
> DebugBreakProcess says it causes the receiving process to terminate in
> most cases, see
> https://msdn.microsoft.com/en-us/library/windows/desktop/ms679298(v=vs.85).aspx
> 
> But with all processes I tried, none of them was ever terminated.

That's a pity; on GNU/Linux the target program does terminate.  If
there's nothing that can be done, we will just have to document this
quirk, at least for many/most programs Emacs users will meet on their
systems.

> >From ecf00b52957bb0bf15735180a67cc9ce7027ff19 Mon Sep 17 00:00:00 2001
> From: Alain Schneble <a.s@realize.ch>
> Date: Thu, 25 Aug 2016 21:31:48 +0200
> Subject: [PATCH] Support SIGTRAP in kill emulation on Windows

The (latest version of) your patch looks fine to me, modulo the issues
mentioned above.  If nothing new and no other comments come up within
a week, let's install this, with the necessary updates for the NEWS
entry.

Thanks.



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: SIGTRAP in kill emulation on Windows
  2016-08-26  8:24     ` Eli Zaretskii
@ 2016-08-26  8:58       ` Alain Schneble
  2016-08-26  9:13         ` Eli Zaretskii
  0 siblings, 1 reply; 12+ messages in thread
From: Alain Schneble @ 2016-08-26  8:58 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> Sending SIGTRAP to a process -- whether it's an unrelated, child or the
>> calling Emacs process itself -- does not seem to have any effect as long
>> as no debugger is attached to the receiving process.
>
> Do you see the same with the debugbreak program you've built earlier?
> If not, then that program does something that your patch doesn't.

Yes, debugbreak and the proposed implementation behave identically.

> Also, were all the programs you tried built with MinGW?  If so,
> perhaps try with other programs, like Python or one of the programs
> that came with Windows.  I think the results of the call depend on the
> exception handlers installed by the program when it starts, so
> different programs and different ways of building programs might have
> different effects.

I tried it with different programs (e.g. notepad, calculator as well as
programs built with MinGW such as Emacs and find).  Yes, I think as well
that it depends on the exception handlers installed by the programs.  So
there might be programs out there that will terminate.  But I had no
luck in finding one so far...

> And finally, what happens if you signal the calling Emacs process that
> is itself being debugged?  I guess the debugger will kick in, but if
> you then continue execution, does Emacs continues to run normally?

Exactly, the control will be passed to the debugger.  I did not see any
strange behavior when resuming it ( (gdb) cont ).  After all, this will
be used by one of the use cases I have in mind, to be able to switch
back to the debugger from within the debuggee.

>> At least this is what I observe on Windows 10.  Shall I test on
>> other OS versions as well?
>
> We could ask people here to test on other versions and report their
> results.

FWIW, I tested on Windows 7 in addition to Windows 10.  Same behavior
on both systems.

>> If so, I would have to install say an Win XP first as I do not
>> have one available right now.
>
> I could test on XP here.

Thanks.  That would save me some time.

>> Unfortunately, on MSDN the remark on
>> DebugBreakProcess says it causes the receiving process to terminate in
>> most cases, see
>> https://msdn.microsoft.com/en-us/library/windows/desktop/ms679298(v=vs.85).aspx
>> 
>> But with all processes I tried, none of them was ever terminated.
>
> That's a pity; on GNU/Linux the target program does terminate.  If
> there's nothing that can be done, we will just have to document this
> quirk, at least for many/most programs Emacs users will meet on their
> systems.

Yes, and on GNU/Linux I observed that it prints a backtrace prior to
termination.  But that's kind of irrelevant here I guess.  Where would
be the best place to document this quirk (c, signal-process, info
manual)?

FWIW, there is probably something we could do about it -- query if the
process in question is attached to a debugger.  If not, we could
terminate it.

>> >From ecf00b52957bb0bf15735180a67cc9ce7027ff19 Mon Sep 17 00:00:00 2001
>> From: Alain Schneble <a.s@realize.ch>
>> Date: Thu, 25 Aug 2016 21:31:48 +0200
>> Subject: [PATCH] Support SIGTRAP in kill emulation on Windows
>
> The (latest version of) your patch looks fine to me, modulo the issues
> mentioned above.  If nothing new and no other comments come up within
> a week, let's install this, with the necessary updates for the NEWS
> entry.

Thanks, Eli.




^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: SIGTRAP in kill emulation on Windows
  2016-08-26  8:58       ` Alain Schneble
@ 2016-08-26  9:13         ` Eli Zaretskii
  2016-08-26  9:31           ` Eli Zaretskii
  2016-08-26  9:47           ` Alain Schneble
  0 siblings, 2 replies; 12+ messages in thread
From: Eli Zaretskii @ 2016-08-26  9:13 UTC (permalink / raw)
  To: Alain Schneble; +Cc: emacs-devel

> From: Alain Schneble <a.s@realize.ch>
> CC: <emacs-devel@gnu.org>
> Date: Fri, 26 Aug 2016 10:58:58 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> Sending SIGTRAP to a process -- whether it's an unrelated, child or the
> >> calling Emacs process itself -- does not seem to have any effect as long
> >> as no debugger is attached to the receiving process.
> >
> > Do you see the same with the debugbreak program you've built earlier?
> > If not, then that program does something that your patch doesn't.
> 
> Yes, debugbreak and the proposed implementation behave identically.

Then I guess that's a limitation we will have to live with.  It
doesn't sound like a grave one to me: after all, what exactly SIGTRAP
does when no debugger is attached is not very important, since that is
not the primary use case for that signal, AFAIU.  If a Lisp program
wants to kill the process, it doesn't need to use SIGTRAP.

> >> But with all processes I tried, none of them was ever terminated.
> >
> > That's a pity; on GNU/Linux the target program does terminate.  If
> > there's nothing that can be done, we will just have to document this
> > quirk, at least for many/most programs Emacs users will meet on their
> > systems.
> 
> Yes, and on GNU/Linux I observed that it prints a backtrace prior to
> termination.

That depends on ulimit and suchlikes, I think: on a GNU/Linux system I
tried that, the program was simply dumped to the shell prompt without
printing anything.

> But that's kind of irrelevant here I guess.  Where would be the best
> place to document this quirk (c, signal-process, info manual)?

In the Info manual, I think.

> FWIW, there is probably something we could do about it -- query if the
> process in question is attached to a debugger.  If not, we could
> terminate it.

That's for the application to decide, IMO.  Signal delivery is too
low-level to replace one signal with another one.



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: SIGTRAP in kill emulation on Windows
  2016-08-26  9:13         ` Eli Zaretskii
@ 2016-08-26  9:31           ` Eli Zaretskii
  2016-08-26  9:47           ` Alain Schneble
  1 sibling, 0 replies; 12+ messages in thread
From: Eli Zaretskii @ 2016-08-26  9:31 UTC (permalink / raw)
  To: a.s; +Cc: emacs-devel

> Date: Fri, 26 Aug 2016 12:13:07 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: emacs-devel@gnu.org
> 
> > But that's kind of irrelevant here I guess.  Where would be the best
> > place to document this quirk (c, signal-process, info manual)?
> 
> In the Info manual, I think.

Actually, perhaps not even there.  In NEWS might be enough, I think.



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: SIGTRAP in kill emulation on Windows
  2016-08-26  9:13         ` Eli Zaretskii
  2016-08-26  9:31           ` Eli Zaretskii
@ 2016-08-26  9:47           ` Alain Schneble
  2016-08-26 10:19             ` Alain Schneble
  1 sibling, 1 reply; 12+ messages in thread
From: Alain Schneble @ 2016-08-26  9:47 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Alain Schneble <a.s@realize.ch>
>> CC: <emacs-devel@gnu.org>
>> Date: Fri, 26 Aug 2016 10:58:58 +0200
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> Yes, debugbreak and the proposed implementation behave identically.
>
> Then I guess that's a limitation we will have to live with.  It
> doesn't sound like a grave one to me: after all, what exactly SIGTRAP
> does when no debugger is attached is not very important, since that is
> not the primary use case for that signal, AFAIU.  If a Lisp program
> wants to kill the process, it doesn't need to use SIGTRAP.

I agree.

>> Yes, and on GNU/Linux I observed that it prints a backtrace prior to
>> termination.
>
> That depends on ulimit and suchlikes, I think: on a GNU/Linux system I
> tried that, the program was simply dumped to the shell prompt without
> printing anything.

Ok, thanks for the hint.

>> But that's kind of irrelevant here I guess.  Where would be the best
>> place to document this quirk (c, signal-process, info manual)?
>
> Actually, perhaps not even there.  In NEWS might be enough, I think.

Ok, I'll arrange a new patch with that added to the NEWS entry.

>> FWIW, there is probably something we could do about it -- query if the
>> process in question is attached to a debugger.  If not, we could
>> terminate it.
>
> That's for the application to decide, IMO.  Signal delivery is too
> low-level to replace one signal with another one.

That's a good point.  I agree with you.




^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: SIGTRAP in kill emulation on Windows
  2016-08-26  9:47           ` Alain Schneble
@ 2016-08-26 10:19             ` Alain Schneble
  2016-10-08 13:54               ` Eli Zaretskii
  0 siblings, 1 reply; 12+ messages in thread
From: Alain Schneble @ 2016-08-26 10:19 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

[-- Attachment #1: Type: text/plain, Size: 423 bytes --]

Alain Schneble <a.s@realize.ch> writes:

>>> But that's kind of irrelevant here I guess.  Where would be the best
>>> place to document this quirk (c, signal-process, info manual)?
>>
>> Actually, perhaps not even there.  In NEWS might be enough, I think.
>
> Ok, I'll arrange a new patch with that added to the NEWS entry.

Here is a new version of the patch with the additional comment in the
NEWS entry.

Thanks.
Alain


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: SIGTRAP in kill emulation on Windows --]
[-- Type: text/x-patch, Size: 5030 bytes --]

From e9943ab1846c1b021bbafd1cb1e4dbc2e5fff528 Mon Sep 17 00:00:00 2001
From: Alain Schneble <a.s@realize.ch>
Date: Fri, 26 Aug 2016 12:13:51 +0200
Subject: [PATCH] Support SIGTRAP in kill emulation on Windows

* src/w32proc.c (sys_kill): Translate SIGTRAP signal into a call to
DebugBreakProcess to cause a breakpoint exception to occur in the
specified process.  Windows versions prior to Windows XP that do not
support DebugBreakProcess return -1 and errno set to ENOTSUP (as opposed
to EINVAL before this change).
* src/w32proc.c: Add typedef for DebugBreakProcess function pointer and
global variable to track state of run-time dynamic linking of this
function.
* etc/NEWS: Add entry to document that 'signal-process' now supports
SIGTRAP.
---
 etc/NEWS      | 10 ++++++++++
 src/w32.c     |  2 ++
 src/w32proc.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++++---
 3 files changed, 61 insertions(+), 3 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 494a091..4b7647c 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -599,6 +599,16 @@ still apply.)
 Previously, on MS-Windows this function converted slash characters in
 file names into backslashes.  It no longer does that.
 
+---
+** 'signal-process' supports SIGTRAP on Windows XP and later.
+The 'kill' emulation on Windows now maps SIGTRAP to a call to Win32
+'DebugBreakProcess'.  This causes the receiving process to break
+execution and return control to the debugger.  If no debugger is
+attached to the receiving process, the call is typically ignored.
+This is in contrast to the default action on POSIX Systems, where it
+causes the receiving process to terminate with a core dump if no
+debugger has been attached to it.
+
 \f
 * Installation Changes in Emacs 25.1
 
diff --git a/src/w32.c b/src/w32.c
index 1db3426..0f22387 100644
--- a/src/w32.c
+++ b/src/w32.c
@@ -330,6 +330,7 @@ static BOOL g_b_init_set_named_security_info_a;
 static BOOL g_b_init_get_adapters_info;
 
 BOOL g_b_init_compare_string_w;
+BOOL g_b_init_debug_break_process;
 
 /*
   BEGIN: Wrapper functions around OpenProcessToken
@@ -9653,6 +9654,7 @@ globals_of_w32 (void)
   g_b_init_set_named_security_info_a = 0;
   g_b_init_get_adapters_info = 0;
   g_b_init_compare_string_w = 0;
+  g_b_init_debug_break_process = 0;
   num_of_processors = 0;
   /* The following sets a handler for shutdown notifications for
      console apps. This actually applies to Emacs in both console and
diff --git a/src/w32proc.c b/src/w32proc.c
index 11a121f..bb637bc 100644
--- a/src/w32proc.c
+++ b/src/w32proc.c
@@ -66,6 +66,8 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 	    + (filedata).file_base))
 
 extern BOOL g_b_init_compare_string_w;
+extern BOOL g_b_init_debug_break_process;
+
 int sys_select (int, SELECT_TYPE *, SELECT_TYPE *, SELECT_TYPE *,
 		struct timespec *, void *);
 
@@ -2494,6 +2496,9 @@ find_child_console (HWND hwnd, LPARAM arg)
   return TRUE;
 }
 
+typedef BOOL (WINAPI * DebugBreakProcess_Proc) (
+    HANDLE hProcess);
+
 /* Emulate 'kill', but only for other processes.  */
 int
 sys_kill (pid_t pid, int sig)
@@ -2507,9 +2512,9 @@ sys_kill (pid_t pid, int sig)
   if (pid < 0)
     pid = -pid;
 
-  /* Only handle signals that will result in the process dying */
+  /* Only handle signals that can be mapped to a similar behavior on Windows */
   if (sig != 0
-      && sig != SIGINT && sig != SIGKILL && sig != SIGQUIT && sig != SIGHUP)
+      && sig != SIGINT && sig != SIGKILL && sig != SIGQUIT && sig != SIGHUP && sig != SIGTRAP)
     {
       errno = EINVAL;
       return -1;
@@ -2552,7 +2557,11 @@ sys_kill (pid_t pid, int sig)
 	 close the selected frame, which does not necessarily
 	 terminates Emacs.  But then we are not supposed to call
 	 sys_kill with our own PID.  */
-      proc_hand = OpenProcess (PROCESS_TERMINATE, 0, pid);
+
+      DWORD desiredAccess =
+	(sig == SIGTRAP) ? PROCESS_ALL_ACCESS : PROCESS_TERMINATE;
+
+      proc_hand = OpenProcess (desiredAccess, 0, pid);
       if (proc_hand == NULL)
         {
 	  errno = EPERM;
@@ -2648,6 +2657,43 @@ sys_kill (pid_t pid, int sig)
 	  rc = -1;
 	}
     }
+  else if (sig == SIGTRAP)
+    {
+      static DebugBreakProcess_Proc s_pfn_Debug_Break_Process = NULL;
+
+      if (g_b_init_debug_break_process == 0)
+	{
+	  g_b_init_debug_break_process = 1;
+	  s_pfn_Debug_Break_Process = (DebugBreakProcess_Proc)
+	    GetProcAddress (GetModuleHandle ("kernel32.dll"),
+			    "DebugBreakProcess");
+	}
+
+      if (s_pfn_Debug_Break_Process == NULL)
+	{
+	  errno = ENOTSUP;
+	  rc = -1;
+	}
+      else if (!s_pfn_Debug_Break_Process (proc_hand))
+	{
+	  DWORD err = GetLastError ();
+
+	  DebPrint (("sys_kill.DebugBreakProcess return %d "
+		     "for pid %lu\n", err, pid));
+
+	  switch (err)
+	    {
+	    case ERROR_ACCESS_DENIED:
+	      errno = EPERM;
+	      break;
+	    default:
+	      errno = EINVAL;
+	      break;
+	    }
+
+	  rc = -1;
+	}
+    }
   else
     {
       if (NILP (Vw32_start_process_share_console) && cp && cp->hwnd)
-- 
2.8.1.windows.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: SIGTRAP in kill emulation on Windows
  2016-08-26 10:19             ` Alain Schneble
@ 2016-10-08 13:54               ` Eli Zaretskii
  2016-10-08 14:36                 ` Alain Schneble
  0 siblings, 1 reply; 12+ messages in thread
From: Eli Zaretskii @ 2016-10-08 13:54 UTC (permalink / raw)
  To: Alain Schneble; +Cc: emacs-devel

> From: Alain Schneble <a.s@realize.ch>
> CC: <emacs-devel@gnu.org>
> Date: Fri, 26 Aug 2016 12:19:18 +0200
> 
> Alain Schneble <a.s@realize.ch> writes:
> 
> >>> But that's kind of irrelevant here I guess.  Where would be the best
> >>> place to document this quirk (c, signal-process, info manual)?
> >>
> >> Actually, perhaps not even there.  In NEWS might be enough, I think.
> >
> > Ok, I'll arrange a new patch with that added to the NEWS entry.
> 
> Here is a new version of the patch with the additional comment in the
> NEWS entry.

Thanks, pushed to the master branch.



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: SIGTRAP in kill emulation on Windows
  2016-10-08 13:54               ` Eli Zaretskii
@ 2016-10-08 14:36                 ` Alain Schneble
  0 siblings, 0 replies; 12+ messages in thread
From: Alain Schneble @ 2016-10-08 14:36 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Alain Schneble <a.s@realize.ch>
>> CC: <emacs-devel@gnu.org>
>> Date: Fri, 26 Aug 2016 12:19:18 +0200
>> 
>> Alain Schneble <a.s@realize.ch> writes:
>> 
>> >>> But that's kind of irrelevant here I guess.  Where would be the best
>> >>> place to document this quirk (c, signal-process, info manual)?
>> >>
>> >> Actually, perhaps not even there.  In NEWS might be enough, I think.
>> >
>> > Ok, I'll arrange a new patch with that added to the NEWS entry.
>> 
>> Here is a new version of the patch with the additional comment in the
>> NEWS entry.
>
> Thanks, pushed to the master branch.

Thank you.

Alain




^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2016-10-08 14:36 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-08-25 15:08 SIGTRAP in kill emulation on Windows Alain Schneble
2016-08-25 16:18 ` Eli Zaretskii
2016-08-25 20:02   ` Alain Schneble
2016-08-25 20:09     ` Alain Schneble
2016-08-26  8:24     ` Eli Zaretskii
2016-08-26  8:58       ` Alain Schneble
2016-08-26  9:13         ` Eli Zaretskii
2016-08-26  9:31           ` Eli Zaretskii
2016-08-26  9:47           ` Alain Schneble
2016-08-26 10:19             ` Alain Schneble
2016-10-08 13:54               ` Eli Zaretskii
2016-10-08 14:36                 ` Alain Schneble

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.