[PATCH v5 0/1] MR3397: Draft: ucrtbase: Do not modify rewind's stack arguments on i386.

[Victor Hermann Chiletto (＠vitorhnn)]

[A mod for Fallout: New Vegas relies on rewind not modifying the first argument passed to it.](https://github.com/jazzisparis/UIO-User-Interface-Organizer/blob/426d96391f9...) Since it only links to api-ms-win-crt-stdio I've kept the thunk usage to just ucrtbase.

--
  v5: msvcrt, ucrtbase: Introduce rewind_preserve_stack.
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/3397

[PATCH v5 1/1] msvcrt, ucrtbase: Introduce rewind_preserve_stack.

[Victor Chiletto]

From: Victor Chiletto <v(a)hnn.net.br>

This is a small thunk to rewind that preserves the first argument
passed. This is needed because on some versions of GCC, rewind's call to
_unlock_file is tail-call optimized, which modifies the stack.
---
 dlls/msvcrt/file.c          | 17 +++++++++++++
 dlls/ucrtbase/tests/misc.c  | 48 +++++++++++++++++++++++++++++++++++++
 dlls/ucrtbase/ucrtbase.spec |  3 ++-
 3 files changed, 67 insertions(+), 1 deletion(-)

diff --git a/dlls/msvcrt/file.c b/dlls/msvcrt/file.c
index 58a607e3bbe..c9fab88402e 100644
--- a/dlls/msvcrt/file.c
+++ b/dlls/msvcrt/file.c
@@ -45,6 +45,10 @@
 
 #include "wine/debug.h"
 
+#if defined (__i386__)
+#include "wine/asm.h"
+#endif
+
 WINE_DEFAULT_DEBUG_CHANNEL(msvcrt);
 
 #undef _fstat
@@ -1658,6 +1662,19 @@ int CDECL clearerr_s(FILE* file)
   return 0;
 }
 
+#if defined(__i386__)
+/* Stack preserving thunk for rewind
+ * needed for the UIO mod for Fallout: New Vegas
+ */
+__ASM_GLOBAL_FUNC(rewind_preserve_stack,
+                  "pushl 4(%esp)\n\t"
+                  __ASM_CFI(".cfi_adjust_cfa_offset 4\n\t")
+                  "call "__ASM_NAME("rewind") "\n\t"
+                  "addl $4,%esp\n\t"
+                  __ASM_CFI(".cfi_adjust_cfa_offset -4\n\t")
+                  "ret")
+#endif
+
 /*********************************************************************
  *		rewind (MSVCRT.@)
  */
diff --git a/dlls/ucrtbase/tests/misc.c b/dlls/ucrtbase/tests/misc.c
index 069de0787b7..0ad1804d0d6 100644
--- a/dlls/ucrtbase/tests/misc.c
+++ b/dlls/ucrtbase/tests/misc.c
@@ -38,6 +38,10 @@
 #include <winbase.h>
 #include "wine/test.h"
 
+#if defined(__i386__) && defined(__GNUC__)
+#include "wine/asm.h"
+#endif
+
 #define DEFINE_EXPECT(func) \
     static BOOL expect_ ## func = FALSE, called_ ## func = FALSE
 
@@ -1591,6 +1595,47 @@ static void test_fopen_exclusive( void )
     unlink(path);
 }
 
+#if defined(__i386__)
+#include "pshpack1.h"
+struct rewind_thunk {
+    BYTE push_esp[4]; /* push [esp+0x4] */
+    BYTE call_rewind; /* call */
+    DWORD rewind_addr; /* relative addr of rewind */
+    BYTE pop_eax; /* pop eax */
+    BYTE ret; /* ret */
+};
+#include "poppack.h"
+
+static FILE * (CDECL *test_rewind_wrapper)(FILE *fp);
+
+static void test_rewind_i386_abi(void)
+{
+    FILE *fp_in, *fp_out;
+
+    struct rewind_thunk *thunk = VirtualAlloc(NULL, sizeof(*thunk), MEM_COMMIT, PAGE_EXECUTE_READWRITE);
+
+    thunk->push_esp[0] = 0xff;
+    thunk->push_esp[1] = 0x74;
+    thunk->push_esp[2] = 0x24;
+    thunk->push_esp[3] = 0x04;
+
+    thunk->call_rewind = 0xe8;
+    thunk->rewind_addr = (BYTE *) rewind - (BYTE *) (&thunk->rewind_addr + 1);
+
+    thunk->pop_eax = 0x58;
+    thunk->ret = 0xc3;
+
+    test_rewind_wrapper = (void *) thunk;
+
+    fp_in = fopen("rewind_abi.tst", "wb");
+    fp_out = test_rewind_wrapper(fp_in);
+    ok(fp_in == fp_out, "rewind modified the first argument in the stack\n");
+
+    fclose(fp_in);
+    unlink("rewind_abi.tst");
+}
+#endif
+
 START_TEST(misc)
 {
     int arg_c;
@@ -1633,4 +1678,7 @@ START_TEST(misc)
     test_thread_storage();
     test_fenv();
     test_fopen_exclusive();
+#if defined(__i386__)
+    test_rewind_i386_abi();
+#endif
 }
diff --git a/dlls/ucrtbase/ucrtbase.spec b/dlls/ucrtbase/ucrtbase.spec
index bb505b42afb..ff4d8845f1e 100644
--- a/dlls/ucrtbase/ucrtbase.spec
+++ b/dlls/ucrtbase/ucrtbase.spec
@@ -2470,7 +2470,8 @@
 @ cdecl remquof(float float ptr)
 @ cdecl remquol(double double ptr) remquo
 @ cdecl rename(str str)
-@ cdecl rewind(ptr)
+@ cdecl -arch=i386 rewind(ptr) rewind_preserve_stack
+@ cdecl -arch=!i386 rewind(ptr) rewind
 @ cdecl rint(double) MSVCRT_rint
 @ cdecl rintf(float)
 @ cdecl rintl(double) MSVCRT_rint
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/3397

[PATCH v5 0/1] MR3397: ucrtbase: Do not modify rewind's stack arguments on i386. - approved

[Piotr Caban (＠piotr)]

This merge request was approved by Piotr Caban.
-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/3397

[PATCH v5 0/1] MR3397: ucrtbase: Do not modify rewind's stack arguments on i386.

[Piotr Caban (＠piotr)]

Piotr Caban (@piotr) commented about dlls/ucrtbase/ucrtbase.spec:
> @ cdecl remquof(float float ptr)
>  @ cdecl remquol(double double ptr) remquo
>  @ cdecl rename(str str)
> -@ cdecl rewind(ptr)
> +@ cdecl -arch=i386 rewind(ptr) rewind_preserve_stack
> +@ cdecl -arch=!i386 rewind(ptr) rewind
@ cdecl -arch=!i386 rewind(ptr)

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/3397#note_49647