Skip to content

The windows variant of ConsoleHandler.c uses pointers in a fashion likely unsound on 64bit systems.

Motivation

In particular we have:

StgInt console_handler = STG_SIG_DFL;

...
void startSignalHandlers(Capability *cap)
{
    StgStablePtr handler;

    if (console_handler < 0) {
        return;
    }
...

which invoked regularly from the non-threaded runtime. So far so good.

However we sometimes assign a !pointer! to this variable.

int
rts_InstallConsoleEvent(int action, StgStablePtr *handler)
{
    StgInt previous_hdlr = console_handler;

    switch (action) {
    case STG_SIG_IGN:
        console_handler = STG_SIG_IGN;
        if ( !SetConsoleCtrlHandler(NULL, true) ) {
            errorBelch("warning: unable to ignore console events");
        }
        break;
    case STG_SIG_DFL:
        console_handler = STG_SIG_IGN;
        if ( !SetConsoleCtrlHandler(NULL, false) ) {
            errorBelch("warning: unable to restore default console event "
                       "handling");
        }
        break;
    case STG_SIG_HAN:
#if defined(THREADED_RTS)
        // handler is stored in an MVar in the threaded RTS
        console_handler = STG_SIG_HAN;
#else
vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
!       console_handler = (StgInt)*handler;      !
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

#endif
        if (previous_hdlr < 0 || previous_hdlr == STG_SIG_HAN) {
          /* Only install generic_handler() once */
          if ( !SetConsoleCtrlHandler(generic_handler, true) ) {
            errorBelch("warning: unable to install console event handler");
          }
        }
        break;
    }

I suspect the goal is/was to check if console_handler is one of these values:

/* arguments to stg_sig_install() */
#define STG_SIG_DFL   (-1)
#define STG_SIG_IGN   (-2)
#define STG_SIG_ERR   (-3)
#define STG_SIG_HAN   (-4)
#define STG_SIG_RST   (-5)

While comparing against these specific values is quite unlikely to fail comparing broadly as (<0) seems likely to fail at some point.

Proposal

We should just use two different variables here and avoid comparisons like this on a variable that might be a pointer.

To upload designs, you'll need to enable LFS and have an admin enable hashed storage. More information