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 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.
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
_______________________________________________
Mingw-w64-public mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/mingw-w64-public
