On Friday 19 December 2025 15:20:29 Martin Storsjö wrote:
> On Fri, 19 Dec 2025, Pali Rohár wrote:
>
> > On Friday 19 December 2025 14:30:00 Martin Storsjö wrote:
> > > On Thu, 18 Dec 2025, Pali Rohár wrote:
> > >
> > > > msvcrt _fstat function is broken and for directory fd it returns S_ISREG
> > > > type. 32-bit pre-msvcr110 _fstat does not properly signal size overflow.
> > > > UCRT seems to work correctly.
> > > >
> > > > Add mingw-w64 fstat wrappers around msvcrt _fstat functions which change
> > > > S_IFMT to S_IFDIR if winapi filehandle has FILE_ATTRIBUTE_DIRECTORY
> > > > attribute set. Provide wrapper for all 4 fstat size_t/time_t variants.
> > > > For 32-bit pre-msvcr110 builds, adds similar workaround like for
> > > > existing
> > > > mingw-w64 stat functions.
> > > > ---
> > > > mingw-w64-crt/Makefile.am | 10 +++-
> > > > mingw-w64-crt/def-include/crt-aliases.def.in | 18 +++----
> > > > .../api-ms-win-crt-filesystem-l1-1-0.def.in | 3 ++
> > > > mingw-w64-crt/lib-common/msvcrt.def.in | 4 --
> > > > mingw-w64-crt/misc/crtdll_fstat.c | 5 --
> > > > .../stdio/__mingw_fix_fstat_finish.c | 18 +++++++
> > > > mingw-w64-crt/stdio/__mingw_fix_stat.h | 7 +++
> > > > mingw-w64-crt/stdio/_fstat64.c | 3 --
> > > > mingw-w64-crt/stdio/fstat32i64.c | 16 ++++++
> > > > mingw-w64-crt/stdio/fstat64.c | 15 ++++++
> > > > mingw-w64-crt/stdio/msvcr110plus_fstat32.c | 24 +++++++++
> > > > mingw-w64-crt/stdio/msvcr110plus_fstat64i32.c | 24 +++++++++
> > > > mingw-w64-crt/stdio/msvcr110pre_fstat32.c | 50 +++++++++++++++++++
> > > > mingw-w64-crt/stdio/msvcr110pre_fstat64i32.c | 50 +++++++++++++++++++
> > > > mingw-w64-crt/testcases/t_fstat.c | 49 ++++++++++++++++--
> > > > mingw-w64-headers/crt/sys/stat.h | 8 +--
> > > > 16 files changed, 272 insertions(+), 32 deletions(-)
> > > > create mode 100644 mingw-w64-crt/stdio/__mingw_fix_fstat_finish.c
> > > > create mode 100644 mingw-w64-crt/stdio/fstat32i64.c
> > > > create mode 100644 mingw-w64-crt/stdio/fstat64.c
> > > > create mode 100644 mingw-w64-crt/stdio/msvcr110plus_fstat32.c
> > > > create mode 100644 mingw-w64-crt/stdio/msvcr110plus_fstat64i32.c
> > > > create mode 100644 mingw-w64-crt/stdio/msvcr110pre_fstat32.c
> > > > create mode 100644 mingw-w64-crt/stdio/msvcr110pre_fstat64i32.c
> > >
> > > The added test is flaky in the CI environment. In one case, I got the
> > > following error:
> > >
> > > FAIL: t_fstat_t64
> > > =================
> > >
> > > Assertion failed: CreateDirectoryA(path, NULL), file
> > > ../../testcases/t_fstat.c, line 33
> > > fstat(0): mode = 00010000
> > > fstat64(0): mode = 00010000
> > > CreateDirectoryA: path=D:\a\_temp\msys64\tmp\mingw-w64-fstat-test-2
> > > FAIL t_fstat_t64.exe (exit status: 3)
> >
> > I'm happy that I added assert for every function call. In above case the
> > CreateDirectoryA() for creating temporary directory failed.
> >
> > Hm... what we should do if creating the temporary directory fails? Or do
> > you have some suggestion how to create a temporary directory in the test
> > and it would not be flaky?
> >
> > Now, when I'm thinking about it, it seems that msvcrt and WinAPI does
> > not provide any function which can create temporary directory in
> > non-racy way.
>
> Sorry, no idea offhand, and I don't really have time to try to debug your
> tests for you at the moment, or to figure out why this failed. (As this is
> the only test with this attempted temp pattern, it's odd how it failed; or
> does our testsuite run several versions of the same test in parallel
> somehow? I don't know.) The only thing I know is that we can't commit it in
> this form at least.
This seems to be race condition. Directory name is generated by
tempnam() function and it can exists at the time of later
CreateDirectoryA() call.
I found that in POSIX is mkdtemp() function which creates race-free
temporary directory from string template, but it is not in msvcrt/UCRT
and mingw-w64 does not implement it (it has only mkstemp for files).
I have two ideas now:
1. Implement mkdtemp for mingw-w64 as copy of mingw-w64 mkstemp and
replacing "creating file" by "creating directory". This does not fully
address the test issue as the POSIX mkdtemp is taking template string,
which would be needed in the test to generate from GetTempPathA().
OR
2. Provide test-only function named e.g. tmpdir() available only in
mingw-w64-crt/testcases/ and its implementation would be same as
existing C89 function in mingw-w64-crt/stdio/tmpfile.c just it will
return file descriptor of newly created directory which test can use it.
> > > This feels like a separate change to the rest? Before this, there was no
> > > "fstat32" symbol in the ucrt import library, and now there is one - and
> > > this
> > > does not fall under the scope of change that the commit message describes.
> >
> > Ok, this needs better description.
> >
> > Currently our mingw-w64 header files defines non-underscore fstat and
> > fstat64 functions as ASM redirects to underscore _fstat32, _fstat32i64,
> > _fstat64i32, _fstat64 functions based on LFS64 and TIME64 macros.
> >
> > So it means that for fixing fstat and fstat64 we need to introduce four
> > new wrappers around those underscored functions.
> >
> > And this change introduced 4 new symbols fstat32, fstat32i64, fstat64i32
> > and fstat64 to which are ASM-redirected fstat and fstat64 functions from
> > mingw-w64 header files.
> >
> > And together with those 4 new symbols was added their implementation for
> > msvcrt (via new source .c files) and ucrt (just symbol alias as it seems
> > that are not buggy).
> >
> > Splitting the change is quite artificial. I can do it, but it would not
> > provide any usage as without the implementation of those 4 new symbols
> > it would not do nothing. And if I split header and crt changes then we
> > would have change which will get compile errors (because of missing
> > implementation).
>
> Ok, I see. In that case I guess it makes sense to keep it all in one change,
> but it needs to be explained like this then, as it's quite hard to get an
> overview of all the changes here.
I will adjust commit message to include all this information.
> > > > diff --git a/mingw-w64-crt/stdio/__mingw_fix_stat.h
> > > > b/mingw-w64-crt/stdio/__mingw_fix_stat.h
> > > > index 909606204b4d..1317dbffe2e8 100644
> > > > --- a/mingw-w64-crt/stdio/__mingw_fix_stat.h
> > > > +++ b/mingw-w64-crt/stdio/__mingw_fix_stat.h
> > > > @@ -11,5 +11,12 @@ char* __mingw_fix_stat_path (const char* _path);
> > > > wchar_t* __mingw_fix_wstat_path (const wchar_t* _path);
> > > > int __mingw_fix_stat_finish(int ret, const void *orig_path, void
> > > > *used_path,
> > > > unsigned short mode);
> > > > +int __mingw_fix_fstat_finish(int ret, int fd, unsigned short *mode);
> > > > +
> > > > +#define __MINGW_FIXED_FSTAT(fstat_func, fd, obj) ({ \
> > > > + int _fstat_ret = fstat_func(fd, obj); \
> > > > + _fstat_ret = __mingw_fix_fstat_finish(_fstat_ret, fd,
> > > > &(obj)->st_mode); \
> > > > + _fstat_ret; \
> > > > +})
> > >
> > > What is this construct? Is this the GCC extension for statement
> > > expression?
> > >
> > > Our supported compilers do accept it, but can we avoid using it if we
> > > could?
> > >
> > > // Martin
> >
> > Yes, this is GCC extension for statement expression. I used it here
> > because it allows me to write function-like macro with return value,
> > liek any function. So it simplify usage and from usage perspective it
> > look like a function with return value. So construction like
> > "return __MINGW_FIXED_FSTAT(func, fd, obj);" or
> > "int ret = __MINGW_FIXED_FSTAT(func, fd, obj);" is working fine.
> >
> > We can avoid it, but then usage would be more complicated, it would be
> > needed pass "return value" from macro via additional pointer macro
> > argument.
> >
> > I thought that it would be more readable code, and would prevent
> > duplicated code repetition, that is why I used.
>
> Yeah, I see that... Not a definite rejection, but I would indeed prefer if
> we could use some more portable syntax even if it would lead to slightly
> less ergonomical code.
>
> // Martin
We are already using lot of those gcc builtins and gcc asm (for
redirects), so code is already gcc/clang specific.
I can try to think how to rewrite that code.
_______________________________________________
Mingw-w64-public mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/mingw-w64-public
