Hello R-devel,

When checking packages on Windows, a crash of the process looks like a
sudden stop in the output of the child process, which can be very
perplexing for package maintainers (e.g. [1,2]), especially if the
stars are only right on Win-Builder but not on the maintainer's PC.

On Unix-like systems, we get a loud message from the SIGSEGV handler
and (if we're lucky and the memory manager is still mostly intact) an
R-level traceback. A similar signal handler, win32_segv(), is defined
in src/main.c for use on Windows, but I am not seeing a way it could be
called in the current codebase. The file src/gnuwin32/psignal.c that's
responsible for signals on Windows handles Ctrl+C but does not emit
SIGSEGV or SIGILL. Can we make use of vectored exception handling [3]
to globally catch unhandled exceptions in the Win32 process and
transform them into raise(SIGSEGV)?

One potential source of problems is threading. The normal Unix-like
sigactionSegv(...) doesn't care; if a non-main thread causes a crash
with SIGSEGV unblocked, it will run in that thread's context and call R
API from there. On Windows, where a simple REprintf() may go into a GUI
window, the crash handler may be written in a more cautious manner:

 - Only set up the crash handler in Rterm, because this is mostly for
   the benefit of the people reading the R CMD check output
 - Compare GetCurrentThreadId() against a saved value and don't call
   R_Traceback() if it doesn't match
 - Rewrite win32_segv in terms of StringCcbPrintf to static storage and
   WriteFile(GetStdHandle(STD_ERROR_HANDLE), ...), which may be too much

Attached is a crude first draft to see if the approach is viable. If it
turns out to be a good idea, I can add the Rterm or thread ID checks, a
reentrancy guard, declare and export a special struct win32_segvinfo
from psignal.c, put all the crash reporting in win32_segv(), and move
the VEH setup into psignal.c's signal(). (Just don't want to waste the
effort if this proves ill-advised.)

Without the patch:
User@WIN-LGTSPJA3F1V MSYS /c/R/R-svn/src/gnuwin32
$ cat crash.c
void crash(void) { *(double*)42 = 42; }

User@WIN-LGTSPJA3F1V MSYS /c/R/R-svn/src/gnuwin32
$ ../../bin/R -q -s -e 'dyn.load("crash.dll"); .C("crash")'
Segmentation fault # <-- printed by MSYS2 shell

With the patch:
User@WIN-LGTSPJA3F1V MSYS /c/R/R-svn/src/gnuwin32
$ ../../bin/R -q -s -e 'dyn.load("crash.dll"); .C("crash")'
*** caught access violation at program counter 0x00007ff911c61387 ***
accessing address 0x000000000000002a, action: write

Traceback:
 1: .C("crash")
Segmentation fault

With the patch applied, I am not seeing changes in make check-devel or
package checks for V8. Couldn't test rJava yet. 

-- 
Best regards,
Ivan

[1]
https://stat.ethz.ch/pipermail/r-package-devel/2024q2/010919.html

[2]
https://stat.ethz.ch/pipermail/r-package-devel/2024q2/010872.html

[3]
https://learn.microsoft.com/en-us/windows/win32/debug/vectored-exception-handling
Index: src/gnuwin32/sys-win32.c
===================================================================
--- src/gnuwin32/sys-win32.c    (revision 86850)
+++ src/gnuwin32/sys-win32.c    (working copy)
@@ -26,6 +26,8 @@
 #include <config.h>
 #endif
 
+#include <psignal.h>
+
 #include <Defn.h>
 #include <Internal.h>
 #include <Fileio.h>
@@ -384,3 +386,69 @@
        return rval;
     }
 }
+
+static WINAPI LONG veh_report_and_raise(PEXCEPTION_POINTERS ei)
+{
+    int signal = 0;
+    const char *exception = 0;
+    switch (ei->ExceptionRecord->ExceptionCode) {
+    case EXCEPTION_ILLEGAL_INSTRUCTION:
+       exception = "illegal instruction";
+       signal = SIGILL;
+       break;
+    case EXCEPTION_ACCESS_VIOLATION:
+       exception = "access violation";
+       signal = SIGSEGV;
+       break;
+    case EXCEPTION_ARRAY_BOUNDS_EXCEEDED:
+       exception = "array bounds overflow";
+       signal = SIGSEGV;
+       break;
+    case EXCEPTION_DATATYPE_MISALIGNMENT:
+       exception = "datatype misalignment";
+       signal = SIGSEGV;
+       break;
+    case EXCEPTION_IN_PAGE_ERROR:
+       exception = "page load failure";
+       signal = SIGSEGV;
+       break;
+    case EXCEPTION_PRIV_INSTRUCTION:
+       exception = "privileged instruction";
+       signal = SIGILL;
+       break;
+    case EXCEPTION_STACK_OVERFLOW:
+       exception = "stack overflow";
+       signal = SIGSEGV;
+       break;
+    default: /* do nothing */ ;
+    }
+    if (signal) {
+       REprintf("*** caught %s at program counter %p ***\n",
+                exception, ei->ExceptionRecord->ExceptionAddress);
+       /* just two more special cases */
+       switch (ei->ExceptionRecord->ExceptionCode) {
+       case EXCEPTION_ACCESS_VIOLATION:
+       case EXCEPTION_IN_PAGE_ERROR:
+           {
+               const char * action;
+               switch (ei->ExceptionRecord->ExceptionInformation[0]) {
+                   case 0: action = "read"; break;
+                   case 1: action = "write"; break;
+                   case 8: action = "execute"; break;
+                   default: action = "<unknown action>";
+               }
+               REprintf("accessing address %p, action: %s\n",
+                        (void*)ei->ExceptionRecord->ExceptionInformation[1],
+                        action);
+               break;
+           }
+       }
+       raise(signal);
+    }
+    return EXCEPTION_CONTINUE_SEARCH;
+}
+
+void R_setup_veh_segv(void)
+{
+    (void)AddVectoredExceptionHandler(0, veh_report_and_raise);
+}
Index: src/main/main.c
===================================================================
--- src/main/main.c     (revision 86850)
+++ src/main/main.c     (working copy)
@@ -457,8 +457,6 @@
 
 
 #ifdef Win32
-static int num_caught = 0;
-
 static void win32_segv(int signum)
 {
     /* NB: stack overflow is not an access violation on Win32 */
@@ -478,12 +476,6 @@
            UNPROTECT(1);
        }
     }
-    num_caught++;
-    if(num_caught < 10) signal(signum, win32_segv);
-    if(signum == SIGILL)
-       error("caught access violation - continue with care");
-    else
-       error("caught access violation - continue with care");
 }
 #endif
 
@@ -707,6 +699,7 @@
 #ifndef Win32
     signal(SIGPIPE, handlePipe);
 #else
+    R_setup_veh_segv();
     signal(SIGSEGV, win32_segv);
     signal(SIGILL, win32_segv);
 #endif
Index: src/include/Defn.h
===================================================================
--- src/include/Defn.h  (revision 86850)
+++ src/include/Defn.h  (working copy)
@@ -2330,6 +2330,7 @@
 void R_wfixbackslash(wchar_t *s);
 void R_wfixslash(wchar_t *s);
 wchar_t *filenameToWchar(const SEXP fn, const Rboolean expand);
+void R_setup_veh_segv(void);
 #endif
 
 FILE *RC_fopen(const SEXP fn, const char *mode, const Rboolean expand);
______________________________________________
R-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel

Reply via email to