Re: [PATCH 5/5] First version of REPL Ctrl-C trampoline for Woe32

[Jose E. Marchesi]

Hi Georgiy.

A concern I have with this patch is this:

AC_PROG_CC
AC_PROG_CXX

I wonder if that will make the build system to use a C++ compiler to
build the C code.  That wouldn't be good.

Regarding the hack itself... I don't like involving C++, but if there is
no other way to implement the Ctrl-C support in Windows, then I guess we
will have to live with it, provided it is restricted to the windows
build.  Interruptible poke is important.

I am CCing Bruno.  He may have some ideas about this.

> This is the least bad solution that I could think of.
>
> On Woe32, Ctrl-C (and similar terminal interrupt) signals are delivered
> on a separate thread. Furthermore, these signals only show up outside of
> cooked-mode; so, when we're inside readline(), no Ctrl-C can arrive
> -- instead, readline sees a ^C character, which it ignores.
>
> Signal delivery on a separate thread would be totally fine... if we
> could then forward that signal to the main thread. However, in all of my
> attempts, pthread_kill appears to act as a no-op.
>
> Furthermore, my experiments with redirecting ^C characters as seen by
> readline into interrupt signals -- thus, getting an interrupt signal on
> the main thread -- showed that setjmp/longjmp cannot be trusted on
> Windows. sigsetjmp/siglongjmp do not exist, of course.
>
> This leaves the horrific "solution" shown in this patch. Instead of
> setjmp, a C++ try/catch block is used, because it appears that C++
> exceptions (or their underlying mechanism, SEH/VEH) is the only reliable
> means of forcing an unwind on Windows.
>
> That is one half of the puzzle; we have a "longjmp" of sorts. However,
> we still don't have signals.
>
> Instead, we have a debugger; we can attach a debugger to our own main
> thread, stop it, set RIP to point into a function that throws an
> exception, resume the thread and detach the debugger. At this point, the
> main thread has now thrown an exception, which is caught by the
> try/catch block and thus execution resumes at the "setjmp" point.
>
> May God have mercy on my soul.
>
> 2021-03-25  Georgiy Tugai  <georgiy@crossings.link>
>
>       * configure.ac: Add C++ compiler support, check for platforms in
>       need of the workaround (currently, *mingw)
>       * poke/Makefile.am: Add pk-repl-tramp.cpp to SOURCES if workaround
>       enabled
>       * poke/pk-repl-tramp.cpp: New file, provides a "trampoline" for
>       calling pk_cmd_exec via a C++ try/catch. In order to interrupt
>       from another thread, the main thread is debugged and RIP is set to
>       a function which throws the caught exception, forcing an unwind to
>       the try block.
>       * poke/pk-repl.c: Use the trampoline to call pk_cmd_exec when
>       it is enabled
> ---
>  ChangeLog              |  14 +++++
>  configure.ac           |   8 +++
>  poke/Makefile.am       |   4 ++
>  poke/pk-repl-tramp.cpp | 139 +++++++++++++++++++++++++++++++++++++++++
>  poke/pk-repl.c         |  19 ++++++
>  5 files changed, 184 insertions(+)
>  create mode 100644 poke/pk-repl-tramp.cpp
>
> diff --git a/ChangeLog b/ChangeLog
> index 5b6c3abd..57473b85 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,17 @@
> +2021-03-25  Georgiy Tugai  <georgiy@crossings.link>
> +
> +     * configure.ac: Add C++ compiler support, check for platforms in
> +     need of the workaround (currently, *mingw)
> +     * poke/Makefile.am: Add pk-repl-tramp.cpp to SOURCES if workaround
> +     enabled
> +     * poke/pk-repl-tramp.cpp: New file, provides a "trampoline" for
> +     calling pk_cmd_exec via a C++ try/catch. In order to interrupt
> +     from another thread, the main thread is debugged and RIP is set to
> +     a function which throws the caught exception, forcing an unwind to
> +     the try block.
> +     * poke/pk-repl.c: Use the trampoline to call pk_cmd_exec when
> +     it is enabled
> +
>  2021-03-25  Georgiy Tugai  <georgiy@crossings.link>
>
>       * poke/poke.c (finalize): GC_disable just before compiler cleanup
> diff --git a/configure.ac b/configure.ac
> index 2b19683d..63a71132 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -37,6 +37,8 @@ dnl AB_INIT
>  
> AC_DEFINE([PACKAGE_BUGZILLA],["https://sourceware.org/bugzilla/describecomponents.cgi?product=poke"],[URL
>  for entering bugs in the project's bugzilla])
>
>  AC_PROG_CC
> +dnl C++ compiler only needed for Windows workarounds; see 
> poke/pk-repl-tramp.cpp
> +AC_PROG_CXX
>  gl_EARLY
>  libpoke_EARLY
>  gui_EARLY
> @@ -59,6 +61,12 @@ dnl System
>  AC_CANONICAL_HOST
>  canonical=$host
>
> +AM_CONDITIONAL([POKE_REPL_TRAMPOLINE], [(test "x$host" = "xx86_64-mingw32") 
> || (test "x$host" = "xi686-pc-mingw32")])
> +if (test "x$host" = "xx86_64-mingw32") || (test "x$host" = 
> "xi686-pc-mingw32"); then
> +  AC_DEFINE([POKE_REPL_TRAMPOLINE], 1,
> +            [Defined if building poke with the Windows REPL trampoline 
> workaround enabled])
> +fi
> +
>  gl_INIT
>  libpoke_INIT
>  gui_INIT
> diff --git a/poke/Makefile.am b/poke/Makefile.am
> index c468ad85..fc27269d 100644
> --- a/poke/Makefile.am
> +++ b/poke/Makefile.am
> @@ -46,6 +46,10 @@ poke_SOURCES = poke.c poke.h \
>
>  poke_SOURCES += ../common/pk-utils.c ../common/pk-utils.h
>
> +if POKE_REPL_TRAMPOLINE
> +poke_SOURCES += pk-repl-tramp.cpp
> +endif
> +
>  poke_CPPFLAGS = -I$(top_builddir)/gl -I$(top_srcdir)/gl \
>                  -I$(top_srcdir)/common \
>                  -I$(top_srcdir)/libpoke -I$(top_builddir)/libpoke \
> diff --git a/poke/pk-repl-tramp.cpp b/poke/pk-repl-tramp.cpp
> new file mode 100644
> index 00000000..f9e52167
> --- /dev/null
> +++ b/poke/pk-repl-tramp.cpp
> @@ -0,0 +1,139 @@
> +/* pk-repl-tramp.cpp - A REPL trampoline for broken platforms like Windows */
> +
> +/* This program is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 3 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <config.h>
> +
> +#include <signal.h>
> +#include <pthread.h>
> +#include <unistd.h>
> +
> +#if _WIN32 && POKE_REPL_TRAMPOLINE
> +// Technique based on 
> https://www.codeproject.com/Articles/71529/Exception-Injection-Throwing-an-Exception-in-Other
> +// Archived at https://archive.is/Wu9M2
> +class ThreadAbort
> +{
> +  __declspec (noreturn) static void Throw();
> +public:
> +  static bool RaiseInThread(DWORD threadId);
> +  static void DontOptimize();
> +};
> +
> +__declspec (noreturn) void ThreadAbort::Throw()
> +{
> +  // just throw
> +  throw ThreadAbort();
> +}
> +
> +void WinPerror(const char* e)
> +{
> +  LPVOID lpMsgBuf;
> +  DWORD dw = GetLastError();
> +
> +  FormatMessage(
> +                FORMAT_MESSAGE_ALLOCATE_BUFFER |
> +                FORMAT_MESSAGE_FROM_SYSTEM |
> +                FORMAT_MESSAGE_IGNORE_INSERTS,
> +                NULL,
> +                dw,
> +                MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT),
> +                (LPTSTR) &lpMsgBuf,
> +                0, NULL );
> +  printf("%s: %s\n", e, lpMsgBuf);
> +
> +  LocalFree(lpMsgBuf);
> +}
> +
> +void ThreadAbort::DontOptimize()
> +{
> +  // By this awkward method we may convince the compiler that during the 
> runtime
> +  // the exception *may* be thrown.
> +  // However it may not actually.
> +  volatile int i=0;
> +  if (i)
> +    Throw();
> +}
> +
> +bool ThreadAbort::RaiseInThread(DWORD threadId)
> +{
> +  printf("RaiseInThread\n");
> +  bool ok = false;
> +
> +  HANDLE hThread = OpenThread(THREAD_SUSPEND_RESUME | THREAD_GET_CONTEXT | 
> THREAD_SET_CONTEXT,
> +                        FALSE,
> +                        threadId);
> +  if (!hThread) {
> +    WinPerror("OpenThread");
> +    return false;
> +  }
> +
> +  if (SuspendThread(hThread) == INFINITE)
> +    {
> +      WinPerror("SuspendThread");
> +      goto cleanup;
> +    }
> +
> +  // Get its context (processor registers)
> +  CONTEXT ctx;
> +  ctx.ContextFlags = CONTEXT_CONTROL | CONTEXT_INTEGER;
> +  if (GetThreadContext(hThread, &ctx))
> +    {
> +#if _WIN64
> +      ctx.Rsp &= -16;
> +      ctx.Rsp -= 8;
> +      ctx.Rip = (uintptr_t) Throw;
> +#else
> +      // Jump into our Throw() function
> +      ctx.Eip = (uintptr_t) Throw;
> +#endif
> +      if (SetThreadContext(hThread, &ctx))
> +        ok = true;
> +      else
> +        WinPerror("SetThreadContext");
> +    }
> +  else
> +    {
> +      WinPerror("GetThreadContext");
> +    }
> +
> +  // go ahead
> +  if (!ResumeThread(hThread))
> +    WinPerror("ResumeThread");
> +
> + cleanup:
> +  if (hThread)
> +    CloseHandle(hThread);
> +  return ok;
> +}
> +
> +extern "C" int pk_cmd_exec (const char *str);
> +
> +extern "C" int pk_tramp_cmd_exec (const char *str)
> +{
> +  try {
> +    ThreadAbort::DontOptimize();
> +    return pk_cmd_exec(str);
> +  } catch (ThreadAbort&) {
> +    printf("ThreadAbort\n");
> +    fflush(stdout);
> +    return -1;
> +  }
> +}
> +
> +extern "C" void pk_tramp_abort(DWORD threadId)
> +{
> +  ThreadAbort::RaiseInThread(threadId);
> +}
> +#endif
> diff --git a/poke/pk-repl.c b/poke/pk-repl.c
> index e1062c56..93b821b8 100644
> --- a/poke/pk-repl.c
> +++ b/poke/pk-repl.c
> @@ -41,6 +41,9 @@
>  /* The thread that contains the non-local entry point for reentering
>     the REPL.  */
>  static pthread_t volatile ctrlc_thread;
> +#if POKE_REPL_TRAMPOLINE
> +static DWORD volatile ctrlc_thread_win;
> +#endif
>  #if HAVE_SIGLONGJMP
>  /* The non-local entry point for reentering the REPL.  */
>  static sigjmp_buf /*volatile*/ ctrlc_buf;
> @@ -211,6 +214,11 @@ banner (void)
>
>  }
>
> +#if POKE_REPL_TRAMPOLINE
> +int pk_tramp_cmd_exec (const char *str);
> +void pk_tramp_abort(DWORD threadId);
> +#endif
> +
>  static _GL_ASYNC_SAFE void
>  poke_sigint_handler (int sig)
>  {
> @@ -228,7 +236,11 @@ poke_sigint_handler (int sig)
>                && sigaddset (&mask, SIGINT) == 0)
>              pthread_sigmask (SIG_BLOCK, &mask, NULL);
>
> +#if POKE_REPL_TRAMPOLINE
> +          pk_tramp_abort(ctrlc_thread_win);
> +#else
>            pthread_kill (ctrlc_thread, SIGINT);
> +#endif
>          }
>        else
>          {
> @@ -352,6 +364,9 @@ pk_repl (void)
>    rl_filename_quoting_function = escape_metacharacters;
>
>    ctrlc_thread = pthread_self ();
> +#if POKE_REPL_TRAMPOLINE
> +  ctrlc_thread_win = GetCurrentThreadId();
> +#endif
>  #if HAVE_SIGLONGJMP
>    sigsetjmp (ctrlc_buf, 1);
>  #endif
> @@ -390,7 +405,11 @@ pk_repl (void)
>            add_history (line);
>  #endif
>
> +#if POKE_REPL_TRAMPOLINE
> +          pk_tramp_cmd_exec (line);
> +#else
>            pk_cmd_exec (line);
> +#endif
>          }
>        free (line);
>      }
> --
> 2.26.2