[PATCH v22 0/1] MR5752: msvcrt: [PR53960] Fix ucrt has different struct layout than msvcrt

[cqwrteur (＠trcrsired)]

UCRT's FILE struct and fd struct have different layouts, and their locks are different.
Implementation in Wine does not match UCRT in Windows, causing ABI issues.
see:
https://bugs.winehq.org/show_bug.cgi?id=53960

--
  v22: [PATCH] msvcrt: Fix FILE in ucrt which incorrect layout

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

[PATCH v22 1/1] [PATCH v22 1/1] msvcrt: Fix FILE in ucrt which incorrect layout

[Shengdun Wang]

From: Shengdun Wang

UCRT's FILE struct and fd struct have different layouts and their locks are different.
Implementation in Wine does not match UCRT in windows, causing ABI issues.

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=53960
---
 dlls/msvcrt/file.c              | 30 ++++++++++++++-----
 dlls/msvcrt/msvcrt.h            | 10 +++++--
 dlls/msvcrt/tests/file.c        | 52 +++++++++++++++++++++++++++++++--
 include/msvcrt/corecrt_wstdio.h |  5 ++++
 include/msvcrt/stdio.h          | 21 +++++++++++++
 5 files changed, 107 insertions(+), 11 deletions(-)

diff --git a/dlls/msvcrt/file.c b/dlls/msvcrt/file.c
index 24cca6fb63e..bbd395c725c 100644
--- a/dlls/msvcrt/file.c
+++ b/dlls/msvcrt/file.c
@@ -246,7 +246,11 @@ typedef struct {
     CRITICAL_SECTION crit;
 } file_crit;

+#ifdef _UCRT
+static file_crit MSVCRT__iob[_IOB_ENTRIES] = { { { 0 } } };
+#else
 FILE MSVCRT__iob[_IOB_ENTRIES] = { { 0 } };
+#endif
 static file_crit* MSVCRT_fstream[MSVCRT_MAX_FILES/MSVCRT_FD_BLOCK_SIZE];
 static int MSVCRT_max_streams = 512, MSVCRT_stream_idx;

@@ -502,7 +506,7 @@ static inline FILE* msvcrt_get_file(int i)
         return NULL;

if(i < _IOB_ENTRIES)
-        return &MSVCRT__iob[i];
+        return (FILE*)&MSVCRT__iob[i];

ret = MSVCRT_fstream[i/MSVCRT_FD_BLOCK_SIZE];
     if(!ret) {
@@ -604,7 +608,9 @@ static FILE* msvcrt_alloc_fp(void)
     {
       if (i == MSVCRT_stream_idx)
       {
+#ifndef _UCRT
           if (file<MSVCRT__iob || file>=MSVCRT__iob+_IOB_ENTRIES)
+#endif
           {
               InitializeCriticalSectionEx(&((file_crit*)file)->crit, 0, RTL_CRITICAL_SECTION_FLAG_FORCE_DEBUG_INFO);
               ((file_crit*)file)->crit.DebugInfo->Spare[0] = (DWORD_PTR)(__FILE__ ": file_crit.crit");
@@ -784,14 +790,18 @@ void msvcrt_init_io(void)
         get_ioinfo_nolock(STDOUT_FILENO)->handle,
         get_ioinfo_nolock(STDERR_FILENO)->handle);

-  memset(MSVCRT__iob,0,3*sizeof(FILE));
+  memset(MSVCRT__iob,0,3*sizeof(*MSVCRT__iob));
   for (i = 0; i < 3; i++)
   {
     /* FILE structs for stdin/out/err are static and never deleted */
-    MSVCRT__iob[i]._file = get_ioinfo_nolock(i)->handle == MSVCRT_NO_CONSOLE ?
+    ((FILE*)(MSVCRT__iob+i))->_file = get_ioinfo_nolock(i)->handle == MSVCRT_NO_CONSOLE ?
         MSVCRT_NO_CONSOLE_FD : i;
-    MSVCRT__iob[i]._tmpfname = NULL;
-    MSVCRT__iob[i]._flag = (i == 0) ? _IOREAD : _IOWRT;
+    ((FILE*)(MSVCRT__iob+i))->_tmpfname = NULL;
+    ((FILE*)(MSVCRT__iob+i))->_flag = (i == 0) ? _IOREAD : _IOWRT;
+#ifdef _UCRT
+    InitializeCriticalSectionEx(&MSVCRT__iob[i].crit, 0, RTL_CRITICAL_SECTION_FLAG_FORCE_DEBUG_INFO);
+              MSVCRT__iob[i].crit.DebugInfo->Spare[0] = (DWORD_PTR)(__FILE__ ": file_crit.crit");
+#endif
   }
   MSVCRT_stream_idx = 3;
 }
@@ -936,7 +946,7 @@ static int msvcrt_int_to_base32_w(int num, wchar_t *str)
 #undef __iob_func
 FILE * CDECL __iob_func(void)
 {
- return &MSVCRT__iob[0];
+ return (FILE*)(&MSVCRT__iob[0]);
 }

#if _MSVCR_VER >= 140
@@ -945,7 +955,7 @@ FILE * CDECL __iob_func(void)
  */
 FILE * CDECL __acrt_iob_func(unsigned idx)
 {
- return &MSVCRT__iob[idx];
+ return (FILE*)(&MSVCRT__iob[idx]);
 }
 #endif

@@ -1390,7 +1400,9 @@ void msvcrt_free_io(void)
     for(j=0; j<MSVCRT_stream_idx; j++)
     {
         FILE *file = msvcrt_get_file(j);
+#ifndef _UCRT
         if(file<MSVCRT__iob || file>=MSVCRT__iob+_IOB_ENTRIES)
+#endif
         {
             ((file_crit*)file)->crit.DebugInfo->Spare[0] = 0;
             DeleteCriticalSection(&((file_crit*)file)->crit);
@@ -1461,9 +1473,11 @@ __msvcrt_long CDECL _lseek(int fd, __msvcrt_long offset, int whence)
  */
 void CDECL _lock_file(FILE *file)
 {
+#ifndef _UCRT
     if(file>=MSVCRT__iob && file<MSVCRT__iob+_IOB_ENTRIES)
         _lock(_STREAM_LOCKS+(file-MSVCRT__iob));
     else
+#endif
         EnterCriticalSection(&((file_crit*)file)->crit);
 }

@@ -1472,9 +1486,11 @@ void CDECL _lock_file(FILE *file)
  */
 void CDECL _unlock_file(FILE *file)
 {
+#ifndef _UCRT
     if(file>=MSVCRT__iob && file<MSVCRT__iob+_IOB_ENTRIES)
         _unlock(_STREAM_LOCKS+(file-MSVCRT__iob));
     else
+#endif
         LeaveCriticalSection(&((file_crit*)file)->crit);
 }

diff --git a/dlls/msvcrt/msvcrt.h b/dlls/msvcrt/msvcrt.h
index dd7fb5895a0..9014c7d3ff9 100644
--- a/dlls/msvcrt/msvcrt.h
+++ b/dlls/msvcrt/msvcrt.h
@@ -284,14 +284,20 @@ extern BOOL msvcrt_create_io_inherit_block(WORD*, BYTE**);
 #define _RT_CRNL        252
 #define _RT_BANNER      255

-extern FILE MSVCRT__iob[];
-
 #define MSVCRT_NO_CONSOLE_FD (-2)
 #define MSVCRT_NO_CONSOLE ((HANDLE)MSVCRT_NO_CONSOLE_FD)

+#ifdef _UCRT
+#define MSVCRT_stdin       stdin
+#define MSVCRT_stdout      stdout
+#define MSVCRT_stderr      stderr
+#else
+extern FILE MSVCRT__iob[];
+
 #define MSVCRT_stdin       (MSVCRT__iob+STDIN_FILENO)
 #define MSVCRT_stdout      (MSVCRT__iob+STDOUT_FILENO)
 #define MSVCRT_stderr      (MSVCRT__iob+STDERR_FILENO)
+#endif

/* internal file._flag flags */
 #define MSVCRT__USERBUF  0x0100
diff --git a/dlls/msvcrt/tests/file.c b/dlls/msvcrt/tests/file.c
index b81469009cd..fe64b0a157a 100644
--- a/dlls/msvcrt/tests/file.c
+++ b/dlls/msvcrt/tests/file.c
@@ -3123,6 +3123,54 @@ static void test_std_stream_buffering(void)
     ok(DeleteFileA("std_stream_test.tmp"), "DeleteFile failed\n");
 }

+struct test_iobuf
+{
+    char* _ptr;
+#ifdef _UCRT
+    char* _base;
+    int   _cnt;
+#else
+    int   _cnt;
+    char* _base;
+#endif
+    int   _flag;
+    int   _file;
+    int   _charbuf;
+    int   _bufsiz;
+    char* _tmpfname;
+#ifdef _UCRT
+    CRITICAL_SECTION _crit;
+#endif
+};
+
+#ifdef _UCRT
+#define TEST_IOMYBUF 0x0040
+#else
+#define TEST_IOMYBUF 0x0008
+#endif
+
+static void test_iobuf_layout(void)
+{
+    char *tempf;
+    FILE *file;
+    char *ptrold;
+    int cntold;
+    ok((test_iobuf*)stdout==(test_iobuf*)stdin+1, "stdout FILE must be adject to stdin");
+    tempf = _tempnam(".","wne");
+    file = fopen(tempf, "wb");
+    fprintf(file, "%s", "Hello World\n");
+    ptrold = file->_ptr;
+    cntold = file->_cnt;
+    fprintf(file, "%s", "Hello");
+    ok(ptrold + 5 == file->_ptr, "FILE _ptr incorrect");
+    ok(cntold - 5 == file->_cnt, "FILE _cnt incorrect");
+    ok(ptrold + cntold == file->_ptr + file->_cnt, "FILE _ptr or _cnt incorrect");
+    ok(file->_ptr + file->_cnt == file->_base + file->_bufsiz, "FILE struct incorrect");
+    ok(((file->_flag&TEST_IOMYBUF) == TEST_IOMYBUF), "file->_flag's _IOBUFFER_CRT incorrect");
+    fclose(file);
+    unlink(tempf);
+}
+
 START_TEST(file)
 {
     int arg_c;
@@ -3183,8 +3231,7 @@ START_TEST(file)
     test_fgetwc_unicode();
     test_fputwc();
     test_freopen();
-    test_ctrlz();
-    test_file_put_get();
+    test_ctrlz();file
     test_tmpnam();
     test_get_osfhandle();
     test_setmaxstdio();
@@ -3200,6 +3247,7 @@ START_TEST(file)
     test_open_hints();
     test_ioinfo_flags();
     test_std_stream_buffering();
+    test_iobuf_layout();

/* Wait for the (_P_NOWAIT) spawned processes to finish to make sure the report
      * file contains lines in the correct order
diff --git a/include/msvcrt/corecrt_wstdio.h b/include/msvcrt/corecrt_wstdio.h
index 404f7345c8e..30caf56e6ef 100644
--- a/include/msvcrt/corecrt_wstdio.h
+++ b/include/msvcrt/corecrt_wstdio.h
@@ -26,8 +26,13 @@ extern "C" {
 typedef struct _iobuf
 {
   char* _ptr;
+#ifdef _UCRT
+  char* _base;
+  int   _cnt;
+#else
   int   _cnt;
   char* _base;
+#endif
   int   _flag;
   int   _file;
   int   _charbuf;
diff --git a/include/msvcrt/stdio.h b/include/msvcrt/stdio.h
index 745257ea5cc..9c82a3b6a5a 100644
--- a/include/msvcrt/stdio.h
+++ b/include/msvcrt/stdio.h
@@ -11,6 +11,26 @@
 #include <corecrt_wstdio.h>

/* file._flag flags */
+#if defined(_UCRT)
+#define _IOREAD          0x0001
+#define _IOWRITE         0x0002
+#define _IOWRT           _IOWRITE
+#define _IOUPDATE        0x0004
+#define _IORW            _IOUPDATE
+#define _IOEOF           0x0008
+#define _IOERR           0x0010
+#define _IOCTRLZ         0x0020
+#define _IOBUFFER_CRT    0x0040
+#define _IOMYBUF         _IOBUFFER_CRT
+#define _IOBUFFER_USER   0x0080
+#define _IOBUFFER_SETVBUF 0x0100
+#define _IOBUFFER_STBUF  0x0200
+#define _IOBUFFER_NONE   0x0400
+#define _IOCOMMIT        0x0800
+#define _IOSTRING        0x1000
+#define _IOSTRG          _IOSTRING
+#define _IOALLOCATED     0x2000
+#else
 #define _IOREAD          0x0001
 #define _IOWRT           0x0002
 #define _IOMYBUF         0x0008
@@ -18,6 +38,7 @@
 #define _IOERR           0x0020
 #define _IOSTRG          0x0040
 #define _IORW            0x0080
+#endif

#define STDIN_FILENO  0
 #define STDOUT_FILENO 1

[PATCH v22 1/1] [PATCH v22 1/1] msvcrt: Fix FILE in ucrt which incorrect layout

[Marvin]

Hi,

It looks like your patch introduced the new failures shown below.
Please investigate and fix them before resubmitting your patch.
If they are not new, fixing them anyway would help a lot. Otherwise
please ask for the known failures list to be updated.

The full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=146012

Your paranoid android.

=== build (build log) ===

../wine/dlls/msvcrt/tests/file.c:3158:9: error: ���test_iobuf��� undeclared (first use in this function)
../wine/dlls/msvcrt/tests/file.c:3158:20: error: expected expression before ���)��� token
../wine/include/wine/test.h:118:83: error: too few arguments to function ���winetest_ok���
../wine/dlls/msvcrt/tests/file.c:3234:18: error: unknown type name ���file���
../wine/dlls/msvcrt/tests/file.c:3235:5: error: conflicting types for ���test_tmpnam���
Task: The exe32 Wine build failed

=== debian11 (build log) ===

../wine/dlls/msvcrt/tests/file.c:3158:9: error: ���test_iobuf��� undeclared (first use in this function)
../wine/dlls/msvcrt/tests/file.c:3158:20: error: expected expression before ���)��� token
../wine/include/wine/test.h:118:83: error: too few arguments to function ���winetest_ok���
../wine/dlls/msvcrt/tests/file.c:3234:18: error: unknown type name ���file���
../wine/dlls/msvcrt/tests/file.c:3235:5: error: conflicting types for ���test_tmpnam���
Task: The win32 Wine build failed

=== debian11b (build log) ===

../wine/dlls/msvcrt/tests/file.c:3158:9: error: ���test_iobuf��� undeclared (first use in this function)
../wine/dlls/msvcrt/tests/file.c:3158:20: error: expected expression before ���)��� token
../wine/include/wine/test.h:118:83: error: too few arguments to function ���winetest_ok���
../wine/dlls/msvcrt/tests/file.c:3234:18: error: unknown type name ���file���
../wine/dlls/msvcrt/tests/file.c:3235:5: error: conflicting types for ���test_tmpnam���
Task: The wow64 Wine build failed