Merge branch 'fix-fileutils-test' into 'main'

Clean up Windows Invalid Parameter Handler setup and fix fileutils test

Closes #2800

See merge request GNOME/glib!3036

glib/glib-private.c

@@ -24,6 +24,10 @@
 #include "glib-private.h"
 #include "glib-init.h"
 
+#ifdef USE_INVALID_PARAMETER_HANDLER
+#include <crtdbg.h>
+#endif
+
 /**
  * glib__private__:
  * @arg: Do not use this argument
@@ -60,7 +64,81 @@ glib__private__ (void)
     g_win32_reopen_noninherited,
     g_win32_handle_is_socket,
 #endif
+
+    g_win32_push_empty_invalid_parameter_handler,
+    g_win32_pop_invalid_parameter_handler,
   };
 
   return &table;
 }
+
+#ifdef USE_INVALID_PARAMETER_HANDLER
+/*
+ * This is the (empty) invalid parameter handler
+ * that is used for Visual C++ 2005 (and later) builds
+ * so that we can use this instead of the system automatically
+ * aborting the process, when calling _get_osfhandle(), isatty()
+ * and _commit() (via g_fsync()) and so on with an invalid file
+ * descriptor.
+ *
+ * This is necessary so that the gspawn helper and the test programs
+ * will continue to run as expected, since we are purposely or
+ * forced to use invalid FDs.
+ *
+ * Please see https://learn.microsoft.com/en-us/cpp/c-runtime-library/parameter-validation?view=msvc-170
+ * for an explanation on this.
+ */
+static void
+empty_invalid_parameter_handler (const wchar_t *expression,
+                                 const wchar_t *function,
+                                 const wchar_t *file,
+                                 unsigned int   line,
+                                 uintptr_t      pReserved)
+{
+}
+
+/* fallback to _set_invalid_parameter_handler() if we don't have _set_thread_local_invalid_parameter_handler() */
+#ifndef HAVE__SET_THREAD_LOCAL_INVALID_PARAMETER_HANDLER
+# define _set_thread_local_invalid_parameter_handler _set_invalid_parameter_handler
+#endif
+
+#endif
+/*
+ * g_win32_push_empty_invalid_parameter_handler:
+ * @handler: a possibly uninitialized GWin32InvalidParameterHandler
+ */
+void
+g_win32_push_empty_invalid_parameter_handler (GWin32InvalidParameterHandler *handler)
+{
+#ifdef USE_INVALID_PARAMETER_HANDLER
+  /* use the empty invalid parameter handler to override the default invalid parameter_handler */
+  handler->pushed_handler = empty_invalid_parameter_handler;
+  handler->old_handler = _set_thread_local_invalid_parameter_handler (handler->pushed_handler);
+
+  /* Disable the message box for assertions. */
+  handler->pushed_report_mode = 0;
+  handler->prev_report_mode = _CrtSetReportMode(_CRT_ASSERT, handler->pushed_report_mode);
+#endif
+}
+
+/*
+ * g_win32_pop_invalid_parameter_handler:
+ * @handler: a GWin32InvalidParameterHandler processed with
+ * g_win32_push_empty_invalid_parameter_handler()
+ */
+void
+g_win32_pop_invalid_parameter_handler (GWin32InvalidParameterHandler *handler)
+{
+#ifdef USE_INVALID_PARAMETER_HANDLER
+  G_GNUC_UNUSED _invalid_parameter_handler popped_handler;
+  G_GNUC_UNUSED int popped_report_mode;
+
+  /* Restore previous/default invalid parameter handler, check the value returned matches the one we previously pushed */
+  popped_handler = _set_thread_local_invalid_parameter_handler (handler->old_handler);
+  g_return_if_fail (handler->pushed_handler == popped_handler);
+
+  /* Restore the message box for assertions, check the value returned matches the one we previously pushed */
+  popped_report_mode = _CrtSetReportMode(_CRT_ASSERT, handler->prev_report_mode);
+  g_return_if_fail (handler->pushed_report_mode == popped_report_mode);
+#endif
+}

glib/glib-private.h

@@ -121,6 +121,27 @@ GMainContext *          g_get_worker_context            (void);
 gboolean                g_check_setuid                  (void);
 GMainContext *          g_main_context_new_with_next_id (guint next_id);
 
+#if (defined (HAVE__SET_THREAD_LOCAL_INVALID_PARAMETER_HANDLER) || \
+     defined (HAVE__SET_INVALID_PARAMETER_HANDLER)) && \
+    defined (HAVE__CRT_SET_REPORT_MODE)
+# define USE_INVALID_PARAMETER_HANDLER
+#endif
+
+#ifdef USE_INVALID_PARAMETER_HANDLER
+struct _GWin32InvalidParameterHandler
+{
+  _invalid_parameter_handler old_handler;
+  _invalid_parameter_handler pushed_handler;
+  int prev_report_mode;
+  int pushed_report_mode;
+};
+#else
+struct _GWin32InvalidParameterHandler
+{
+  int unused_really;
+};
+#endif
+
 #ifdef G_OS_WIN32
 GLIB_AVAILABLE_IN_ALL
 gchar *_glib_get_locale_dir    (void);
@@ -129,8 +150,13 @@ gchar *_glib_get_locale_dir    (void);
 GDir * g_dir_open_with_errno (const gchar *path, guint flags);
 GDir * g_dir_new_from_dirp (gpointer dirp);
 
+typedef struct _GWin32InvalidParameterHandler GWin32InvalidParameterHandler;
+void g_win32_push_empty_invalid_parameter_handler (GWin32InvalidParameterHandler *items);
+void g_win32_pop_invalid_parameter_handler (GWin32InvalidParameterHandler *items);
+
 #define GLIB_PRIVATE_CALL(symbol) (glib__private__()->symbol)
 
+
 typedef struct {
   /* See gwakeup.c */
   GWakeup *             (* g_wakeup_new)                (void);
@@ -182,6 +208,10 @@ typedef struct {
 
 #endif
 
+  /* See glib-private.c */
+  void (* g_win32_push_empty_invalid_parameter_handler) (GWin32InvalidParameterHandler *items);
+
+  void (* g_win32_pop_invalid_parameter_handler)        (GWin32InvalidParameterHandler *items);
 
   /* Add other private functions here, initialize them in glib-private.c */
 } GLibPrivateVTable;

glib/gmessages.c

@@ -192,6 +192,7 @@
 #include "gcharset.h"
 #include "gconvert.h"
 #include "genviron.h"
+#include "glib-private.h"
 #include "gmain.h"
 #include "gmem.h"
 #include "gprintfint.h"
@@ -219,22 +220,6 @@
 #define ENABLE_VIRTUAL_TERMINAL_PROCESSING 0x0004
 #endif
 
-#if defined (_MSC_VER) && (_MSC_VER >=1400)
-/* This is ugly, but we need it for isatty() in case we have bad fd's,
- * otherwise Windows will abort() the program on msvcrt80.dll and later
- */
-#include <crtdbg.h>
-
-_GLIB_EXTERN void
-myInvalidParameterHandler(const wchar_t *expression,
-                          const wchar_t *function,
-                          const wchar_t *file,
-                          unsigned int   line,
-                          uintptr_t      pReserved)
-{
-}
-#endif
-
 #include "gwin32.h"
 #endif
 
@@ -2111,12 +2096,7 @@ g_log_writer_supports_color (gint output_fd)
 {
 #ifdef G_OS_WIN32
   gboolean result = FALSE;
-
-#if (defined (_MSC_VER) && _MSC_VER >= 1400)
-  _invalid_parameter_handler oldHandler, newHandler;
-  int prev_report_mode = 0;
-#endif
-
+  GWin32InvalidParameterHandler handler;
 #endif
 
   g_return_val_if_fail (output_fd >= 0, FALSE);
@@ -2143,17 +2123,7 @@ g_log_writer_supports_color (gint output_fd)
    */
 #ifdef G_OS_WIN32
 
-#if (defined (_MSC_VER) && _MSC_VER >= 1400)
-  /* Set up our empty invalid parameter handler, for isatty(),
-   * in case of bad fd's passed in for isatty(), so that
-   * msvcrt80.dll+ won't abort the program
-   */
-  newHandler = myInvalidParameterHandler;
-  oldHandler = _set_invalid_parameter_handler (newHandler);
-
-  /* Disable the message box for assertions. */
-  prev_report_mode = _CrtSetReportMode(_CRT_ASSERT, 0);
-#endif
+  g_win32_push_empty_invalid_parameter_handler (&handler);
 
   if (g_win32_check_windows_version (10, 0, 0, G_WIN32_OS_ANY))
     {
@@ -2185,10 +2155,7 @@ g_log_writer_supports_color (gint output_fd)
     result = win32_is_pipe_tty (output_fd);
 
 reset_invalid_param_handler:
-#if defined (_MSC_VER) && (_MSC_VER >= 1400)
-      _CrtSetReportMode(_CRT_ASSERT, prev_report_mode);
-      _set_invalid_parameter_handler (oldHandler);
-#endif
+  g_win32_pop_invalid_parameter_handler (&handler);
 
   return result;
 #else

glib/gspawn-win32-helper.c

@@ -23,20 +23,6 @@
 
 #include <fcntl.h>
 
-/* For _CrtSetReportMode, we don't want Windows CRT (2005 and later)
- * to terminate the process if a bad file descriptor is passed into
- * _get_osfhandle().  This is necessary because we use _get_osfhandle()
- * to check the validity of the fd before we try to call close() on
- * it as attempting to close an invalid fd will cause the Windows CRT
- * to abort() this program internally.
- *
- * Please see http://msdn.microsoft.com/zh-tw/library/ks2530z6%28v=vs.80%29.aspx
- * for an explanation on this.
- */
-#if (defined (_MSC_VER) && _MSC_VER >= 1400)
-#include <crtdbg.h>
-#endif
-
 #undef G_LOG_DOMAIN
 #include "glib.h"
 #define GSPAWN_HELPER
@@ -168,30 +154,6 @@ checked_dup2 (int oldfd, int newfd, int report_fd)
   return newfd;
 }
 
-#if (defined (_MSC_VER) && _MSC_VER >= 1400)
-/*
- * This is the (empty) invalid parameter handler
- * that is used for Visual C++ 2005 (and later) builds
- * so that we can use this instead of the system automatically
- * aborting the process.
- *
- * This is necessary as we use _get_oshandle() to check the validity
- * of the file descriptors as we close them, so when an invalid file
- * descriptor is passed into that function as we check on it, we get
- * -1 as the result, instead of the gspawn helper program aborting.
- *
- * Please see http://msdn.microsoft.com/zh-tw/library/ks2530z6%28v=vs.80%29.aspx
- * for an explanation on this.
- */
-extern void
-myInvalidParameterHandler(const wchar_t *expression,
-                          const wchar_t *function,
-                          const wchar_t *file,
-                          unsigned int   line,
-                          uintptr_t      pReserved);
-#endif
-
-
 #ifndef HELPER_CONSOLE
 int _stdcall
 WinMain (struct HINSTANCE__ *hInstance,
@@ -220,16 +182,7 @@ main (int ignored_argc, char **ignored_argv)
   char **argv;
   wchar_t **wargv;
   char c;
-
-#if (defined (_MSC_VER) && _MSC_VER >= 1400)
-  /* set up our empty invalid parameter handler */
-  _invalid_parameter_handler oldHandler, newHandler;
-  newHandler = myInvalidParameterHandler;
-  oldHandler = _set_invalid_parameter_handler(newHandler);
-
-  /* Disable the message box for assertions. */
-  _CrtSetReportMode(_CRT_ASSERT, 0);
-#endif
+  GWin32InvalidParameterHandler handler;
 
   /* Fetch the wide-char argument vector */
   wargv = CommandLineToArgvW (GetCommandLineW(), &argc);
@@ -398,11 +351,13 @@ main (int ignored_argc, char **ignored_argv)
   /* argv[ARG_CLOSE_DESCRIPTORS] is "y" if file descriptors from 3
    *  upwards should be closed
    */
+  GLIB_PRIVATE_CALL (g_win32_push_empty_invalid_parameter_handler) (&handler);
   if (argv[ARG_CLOSE_DESCRIPTORS][0] == 'y')
     for (i = 3; i < 1000; i++)	/* FIXME real limit? */
       if (!g_hash_table_contains (fds, GINT_TO_POINTER (i)))
         if (_get_osfhandle (i) != -1)
           close (i);
+  GLIB_PRIVATE_CALL (g_win32_pop_invalid_parameter_handler) (&handler);
 
   /* We don't want our child to inherit the error report and
    * helper sync fds.

glib/tests/fileutils.c

@@ -35,6 +35,7 @@
 /* Test our stdio wrappers here; this disables redefining (e.g.) g_open() to open() */
 #define G_STDIO_WRAP_ON_UNIX
 #include <glib/gstdio.h>
+#include "glib-private.h"
 
 #ifdef G_OS_UNIX
 #include <unistd.h>
@@ -2462,8 +2463,13 @@ assert_fd_was_closed (int fd)
    * was still valid */
   if (g_test_undefined ())
     {
-      int result = g_fsync (fd);
-      int errsv = errno;
+      int result, errsv;
+      GWin32InvalidParameterHandler handler;
+
+      GLIB_PRIVATE_CALL (g_win32_push_empty_invalid_parameter_handler) (&handler);
+      result = g_fsync (fd);
+      errsv = errno;
+      GLIB_PRIVATE_CALL (g_win32_pop_invalid_parameter_handler) (&handler);
 
       g_assert_cmpint (result, !=, 0);
       g_assert_cmpint (errsv, ==, EBADF);

glib/tests/meson.build

@@ -26,10 +26,7 @@ glib_tests = {
     'can_fail' : host_system == 'darwin',
   },
   'error' : {},
-  'fileutils' : {
-    # FIXME: https://gitlab.gnome.org/GNOME/glib/-/issues/2800
-    'can_fail' : host_system == 'windows' and cc.get_id() == 'msvc',
-  },
+  'fileutils' : {},
   'gdatetime' : {
     'suite' : ['slow'],
     'can_fail' : host_system == 'windows',

meson.build

@@ -2373,6 +2373,20 @@ install_data('m4macros/glib-2.0.m4', 'm4macros/glib-gettext.m4', 'm4macros/gsett
   install_tag : 'devel',
 )
 
+# Check whether we support overriding the invalid parameter handler on Windows for _get_osfhandle(),
+# g_fsync() (i.e. _commit()), etc
+if host_system == 'windows'
+  if cc.has_function('_set_thread_local_invalid_parameter_handler', prefix: '#include <stdlib.h>')
+    glib_conf.set('HAVE__SET_THREAD_LOCAL_INVALID_PARAMETER_HANDLER', 1)
+  endif
+  if cc.has_function('_set_invalid_parameter_handler', prefix: '#include <stdlib.h>')
+    glib_conf.set('HAVE__SET_INVALID_PARAMETER_HANDLER', 1)
+  endif
+  if cc.has_header_symbol('crtdbg.h', '_CrtSetReportMode')
+    glib_conf.set('HAVE__CRT_SET_REPORT_MODE', 1)
+  endif
+endif
+
 configure_file(output : 'config.h', configuration : glib_conf)
 
 if host_system == 'windows'