On Saturday 22 March 2025 15:35:48 Lasse Collin wrote:
> On 2025-03-20 Pali Rohár wrote:
> > On Wednesday 12 March 2025 16:33:42 Lasse Collin wrote:
> > > stat.h has "struct stat *" and stat64i32.c has "struct _stat64i32
> > > *". On Linux and "clang -fsanitize=function", this kind of argument
> > > type mismatch doesn't work if the function is called indirectly
> > > using a function pointer. It might not be common to take the
> > > address of stat, but it should still work. (I didn't test how this
> > > works on Windows, sorry.)
> >
> > __MINGW_ASM_CALL is just a function symbol redirection. I used
> > __MINGW_ASM_CALL specially for making aliases of those functions as
> > they have same ABI.
> >
> > Basically whatever function you call (on 64-bit) the result should be
> > exactly same and I think that also pointers for those functions should
> > be same.
> >
> > Is clang really complaining?
>
> I verified in MSYS2's CLANG64 environment that -fsanitize=function and
> -fsanitize=undefined do complain on Windows too.
>
> file1.c:
>
> struct foo { int x; };
> int foo(struct foo *f);
>
> int main(void)
> {
> struct foo f = { 5 };
> int (*cb)(struct foo *f) = &foo;
> return cb(&f);
> }
>
> file2.c:
>
> struct wrongname { int x; };
>
> int foo(struct wrongname *f)
> {
> return f->x;
> }
>
> Testing:
>
> $ clang -O2 -fsanitize=undefined file1.c file2.c
> $ ./a
> file1.c:8:12: runtime error: call to function foo through pointer
> to incorrect function type 'int (*)(struct foo *)'
> (C:\msys64\home\....): note: foo defined here SUMMARY:
> UndefinedBehaviorSanitizer: undefined-behavior file1.c:8:12
>
> If I build with "clang -O2 -fsanitize=cfi -flto" then the program
> terminates with an illegal instruction instead.
Ah, that is bad :-( Is not there some attribute to mark the function to
add either structure name alias or turn off the runtime check for
particular function?
> I don't know if the above needs to work with the functions provided by
> mingw-w64, so me mentioning this may have been a distraction. If only
> file1.c is built with -fsanitize=undefined, there's no problem (file1.c
> could be user's app being sanitized and file2.c could represent
> mingw-w64's stat function).
>
> > > Otherwise I suspect that extra code is needed. For example, stat32.c
> > > could use memcpy to convert struct _stat32 to struct stat like
> > > mingw-w64-crt/stdio/_stat.c does in the current mingw-w64 code.
> > > These wrappers could set _TIME_BITS and _FILE_OFFSET_BITS to get
> > > the correct copy of struct stat.
> >
> > I think that it is stupid to use memcpy for cases which are 1:1 ABI
> > binary compatible.
>
> I agree it's stupid. As long as the type name mismatch is at a
> translation unit boundary, strict aliasing shouldn't cause issues,
> at least without -flto. Maybe I'm overcautious with this kind of
> things (sometimes a trick has worked at first and trouble has appeared
> much later).
>
> Since you feel OK with the current method, maybe proceed with the
> current patches and fix things later if problems actually appear in
> reality. I made the ftw patch with this assumption.
>
> > Also those aliases in def files are doing same thing. Just marking
> > symbols as are ABI binary compatible.
>
> -flto's hand doesn't reach into the DLLs. Thus, I don't think those
> redirects can cause strict aliasing problems.
>
> > We had already start to use __MINGW_ASM_CALL for symbol redirection
> > instead of conditional #ifdef or inline functions, because of C++23
> > exports and C++ inline mechanism (which differs from C). It is needed
> > that emitted symbol in every object file has same meaning. And so,
> > the __MINGW_ASM_CALL is doing it.
>
> OK. :-) I did <ftw.h> similarly to your <sys/stat.h> changes.
>
> > > (3)
> > > fstat, stat, and wstat aren't declared in <sys/stat.h> when
> > > __CRT__NO_INLINE is defined, thus those declarations are missing if
> > > compiling without optimizations. I changed those to _CRTBLD, but
> > > this broke ftw.c (not ftw64.c). <ftw.h> uses "struct stat" and
> > > "struct stat64". I almost fixed the ftw issue, but I didn't finish
> > > because figuring out the stat redirects should be finished first
> > > (*if* they need to be changed).
> >
> > I know about this issue and that is why I defined __CRT__NO_INLINE. As
> > this problem is already there, it could be fixed later.
>
> OK, sorry. I had thought that the misuse of __CRT__NO_INLINE in
> <sys/stat.h> was a somewhat severe bug, and fixing it as a side effect
> could have been easy enough here.
As other people pointed out, this needs to be fixed. I proposed one
solution in other thread.
> In <ftw.h> I used __CRT_BUILDING_FTW. It can be renamed, of course. A
> separate macro for <ftw.h> is convenient because building ftw*.c
> depends on the seeing the correct stat struct and function from
> <sys/stat.h>.
I'm proposing alternative solution to this ftw problem, without
introduction of any new macro. What do you think about it?
It is attached in ftw.patch file.
Also it fixes the ABI problem which you have there. In both 32-bit
msvcrt and 32-bit UCRT builds is "stat" symbol ABI compatible with
stat32. Even the fact that 32-bit UCRT uses stat function API compatible
with stat64i32. So it is needed to distinguish between ABI symbols and
API functions. In your original change you used 32-bit UCRT ftw symbol
compatible with ftw64i32, which breaks API/ABI compatibility with stat
symbol and struct stat.
> > I will include your first change directly into my patches as it is
> > basically fixup of one of my change.
>
> The attached patches are on top of your new patches. Differences to the
> previous round:
>
> - No longer fix function name in __mingw_fix_wstat_path.c. Only fix
> const correctness.
>
> - Drop __CRT__NO_INLINE changes.
>
> - Add stdio/__mingw_fix_stat.h so that the internal function
> declarations don't need to be copied to many files.
>
> - In "Don't remove a trailing '\' if it is a DBCS trail byte", move
> the loop so that it's only run when there is a trailing (back)slash.
>
> - Add the ftw patch.
>
> - Drop S_IFBLK patch because an equivalent patch was just merged. :-)
>
> Things I noticed about your patches but I didn't make any changes:
>
> - With MSVCRT, _stat32 sets timestamps to -1 if the time doesn't fit
> into 32-bit time_t. The 32i64 wrappers truncate the timestamps
> instead. Maybe this doesn't matter, but I mention it in case you
> think it does. With file size it's different anyway: _stat32 uses a
> truncated st_size if file is too large.
>
> - The new file mingw-w64-crt/include/filetime_to_time64.h isn't listed
> in mingw-w64-crt/Makefile.am. It should be if "make dist" needs to
> work.
>
> - Commit message typos:
>
> * "Additionaly define struct stat64 for LFS"
> Additionally
>
> * "Function _time64 is avaulable since msvcr70.dll."
> available
>
> Thanks!
>
> --
> Lasse Collin
I completely forgot about these things. I will fix it in next version of
patch series.
diff --git a/mingw-w64-crt/misc/ftw.c b/mingw-w64-crt/misc/ftw.c
index 54a6d8b8801c..f520ef63da78 100644
--- a/mingw-w64-crt/misc/ftw.c
+++ b/mingw-w64-crt/misc/ftw.c
@@ -3,7 +3,6 @@
* No warranty is given; refer to the file DISCLAIMER within this package.
*/
-#define __CRT_BUILDING_FTW
#include <stdlib.h>
#include <unistd.h>
#include <malloc.h>
@@ -16,6 +15,10 @@
#include <dirent.h>
#include <ftw.h>
+int __cdecl stat32(const char *_Filename, struct _stat32 *_Stat);
+int __cdecl stat32i64(const char *_Filename, struct _stat32i64 *_Stat);
+int __cdecl stat64i32(const char *_Filename, struct _stat64i32 *_Stat);
+
typedef struct dir_data_t {
DIR *h;
char *buf;
@@ -31,14 +34,14 @@ typedef struct ctx_t {
dir_data_t **dirs;
char *buf;
struct FTW ftw;
- int (*fcb) (const char *, const struct stat *, int , struct FTW *);
+ int (*fcb) (const char *, const STRUCT_STAT *, int , struct FTW *);
size_t cur_dir, msz_dir, buf_sz;
int flags;
dev_t dev;
} ctx_t;
static int add_object (ctx_t *);
-static int do_dir (ctx_t *, struct stat *, dir_data_t *);
+static int do_dir (ctx_t *, STRUCT_STAT *, dir_data_t *);
static int do_entity (ctx_t *, dir_data_t *, const char *, size_t);
static int do_it (const char *, int, void *, int, int);
@@ -220,7 +223,7 @@ open_directory (ctx_t *ctx, dir_data_t *dirp)
static int
do_entity (ctx_t *ctx, dir_data_t *dir, const char *name, size_t namlen)
{
- struct stat st;
+ STRUCT_STAT st;
char *h;
size_t cnt_sz;
int ret = 0, flag = 0;
@@ -244,7 +247,7 @@ do_entity (ctx_t *ctx, dir_data_t *dir, const char *name, size_t namlen)
name = ctx->buf;
- if (stat (name, &st) < 0)
+ if (FUNC_STAT (name, &st) < 0)
{
if (errno != EACCES && errno != ENOENT)
ret = -1;
@@ -252,7 +255,7 @@ do_entity (ctx_t *ctx, dir_data_t *dir, const char *name, size_t namlen)
flag = FTW_NS;
if (!(ctx->flags & FTW_PHYS))
- stat (name, &st);
+ FUNC_STAT (name, &st);
}
else
flag = (S_ISDIR (st.st_mode) ? FTW_D : FTW_F);
@@ -276,7 +279,7 @@ do_entity (ctx_t *ctx, dir_data_t *dir, const char *name, size_t namlen)
static int
-do_dir (ctx_t *ctx, struct stat *st, __UNUSED_PARAM(dir_data_t *old_dir))
+do_dir (ctx_t *ctx, STRUCT_STAT *st, __UNUSED_PARAM(dir_data_t *old_dir))
{
dir_data_t dir;
struct dirent *d;
@@ -373,7 +376,7 @@ static int
do_it (const char *dir, __UNUSED_PARAM(int is_nftw), void *fcb, int descriptors, int flags)
{
struct ctx_t ctx;
- struct stat st;
+ STRUCT_STAT st;
int ret = 0;
int sv_e;
char *cp;
@@ -412,12 +415,12 @@ do_it (const char *dir, __UNUSED_PARAM(int is_nftw), void *fcb, int descriptors,
ctx.ftw.level = 0;
ctx.ftw.base = cp - ctx.buf;
ctx.flags = flags;
- ctx.fcb = (int (*) (const char *, const struct stat *, int , struct FTW *)) fcb;
+ ctx.fcb = (int (*) (const char *, const STRUCT_STAT *, int , struct FTW *)) fcb;
ctx.objs = NULL;
if (!ret)
{
- if (stat (ctx.buf, &st) < 0)
+ if (FUNC_STAT (ctx.buf, &st) < 0)
ret = -1;
else if (S_ISDIR (st.st_mode))
{
@@ -446,17 +449,17 @@ do_it (const char *dir, __UNUSED_PARAM(int is_nftw), void *fcb, int descriptors,
}
int
-ftw (const char *path, int (*fcb) (const char *, const struct stat *, int), int descriptors);
+FUNC_FTW (const char *path, int (*fcb) (const char *, const STRUCT_STAT *, int), int descriptors);
int
-ftw (const char *path, int (*fcb) (const char *, const struct stat *, int), int descriptors)
+FUNC_FTW (const char *path, int (*fcb) (const char *, const STRUCT_STAT *, int), int descriptors)
{
return do_it (path, 0, fcb, descriptors, 0);
}
int
-nftw (const char *path, int (*fcb) (const char *, const struct stat *, int , struct FTW *), int descriptors, int flags);
+FUNC_NFTW (const char *path, int (*fcb) (const char *, const STRUCT_STAT *, int , struct FTW *), int descriptors, int flags);
int
-nftw (const char *path, int (*fcb) (const char *, const struct stat *, int , struct FTW *), int descriptors, int flags)
+FUNC_NFTW (const char *path, int (*fcb) (const char *, const STRUCT_STAT *, int , struct FTW *), int descriptors, int flags)
{
return do_it (path, 1, fcb, descriptors, flags);
}
diff --git a/mingw-w64-crt/misc/ftw32.c b/mingw-w64-crt/misc/ftw32.c
index dbdeb2d0a701..74d70a42ba0c 100644
--- a/mingw-w64-crt/misc/ftw32.c
+++ b/mingw-w64-crt/misc/ftw32.c
@@ -3,25 +3,16 @@
* No warranty is given; refer to the file DISCLAIMER within this package.
*/
-#ifndef _WIN64
-
-#define _USE_32BIT_TIME_T
-
-#define nftw nftw32
-#define ftw ftw32
-
+#define FUNC_FTW ftw32
+#define FUNC_NFTW nftw32
+#define FUNC_STAT stat32
+#define STRUCT_STAT struct _stat32
#include "ftw.c"
-/* This is the default ABI in 32-bit non-UCRT builds. */
-#ifndef _UCRT
+/* On 32-bit systems is stat ABI compatible with stat32 */
+#ifndef _WIN64
#undef nftw
#undef ftw
-
-int __attribute__ ((alias ("nftw32"))) __cdecl
-nftw (const char *, int (*) (const char *, const struct stat *, int, struct FTW *), int, int);
-
-int __attribute__ ((alias ("ftw32"))) __cdecl
-ftw (const char *, int (*) (const char *, const struct stat *, int), int);
-#endif
-
+int __attribute__ ((alias ("nftw32"))) __cdecl nftw(const char *, int (*) (const char *, const struct stat *, int, struct FTW *), int, int);
+int __attribute__ ((alias ("ftw32"))) __cdecl ftw(const char *, int (*) (const char *, const struct stat *, int), int);
#endif
diff --git a/mingw-w64-crt/misc/ftw32i64.c b/mingw-w64-crt/misc/ftw32i64.c
index 5b70e2f0e8f2..20985fb70581 100644
--- a/mingw-w64-crt/misc/ftw32i64.c
+++ b/mingw-w64-crt/misc/ftw32i64.c
@@ -3,14 +3,8 @@
* No warranty is given; refer to the file DISCLAIMER within this package.
*/
-#ifndef _WIN64
-
-#define _USE_32BIT_TIME_T
-#define _FILE_OFFSET_BITS 64
-
-#define nftw nftw32i64
-#define ftw ftw32i64
-
+#define FUNC_FTW ftw32i64
+#define FUNC_NFTW nftw32i64
+#define FUNC_STAT stat32i64
+#define STRUCT_STAT struct _stat32i64
#include "ftw.c"
-
-#endif
diff --git a/mingw-w64-crt/misc/ftw64.c b/mingw-w64-crt/misc/ftw64.c
index a2d8ded1181a..72e22a214e2a 100644
--- a/mingw-w64-crt/misc/ftw64.c
+++ b/mingw-w64-crt/misc/ftw64.c
@@ -3,10 +3,8 @@
* No warranty is given; refer to the file DISCLAIMER within this package.
*/
-#define _TIME_BITS 64
-#define _FILE_OFFSET_BITS 64
-
-#define nftw nftw64
-#define ftw ftw64
-
+#define FUNC_FTW ftw64
+#define FUNC_NFTW nftw64
+#define FUNC_STAT stat64
+#define STRUCT_STAT struct stat64
#include "ftw.c"
diff --git a/mingw-w64-crt/misc/ftw64i32.c b/mingw-w64-crt/misc/ftw64i32.c
index b4fca4d2ae35..6014be81faa7 100644
--- a/mingw-w64-crt/misc/ftw64i32.c
+++ b/mingw-w64-crt/misc/ftw64i32.c
@@ -3,21 +3,16 @@
* No warranty is given; refer to the file DISCLAIMER within this package.
*/
-#define _TIME_BITS 64
-
-#define nftw nftw64i32
-#define ftw ftw64i32
-
+#define FUNC_FTW ftw64i32
+#define FUNC_NFTW nftw64i32
+#define FUNC_STAT stat64i32
+#define STRUCT_STAT struct _stat64i32
#include "ftw.c"
-/* This is the default ABI in all 64-bit builds and all UCRT builds. */
-#if defined(_WIN64) || defined(_UCRT)
+/* On 64-bit systems is stat ABI compatible with stat64i32 */
+#ifdef _WIN64
#undef nftw
#undef ftw
-
-int __attribute__ ((alias ("nftw64i32"))) __cdecl
-nftw (const char *, int (*) (const char *, const struct stat *, int, struct FTW *), int, int);
-
-int __attribute__ ((alias ("ftw64i32"))) __cdecl
-ftw (const char *, int (*) (const char *, const struct stat *, int), int);
+int __attribute__ ((alias ("nftw64i32"))) __cdecl nftw(const char *, int (*) (const char *, const struct stat *, int, struct FTW *), int, int);
+int __attribute__ ((alias ("ftw64i32"))) __cdecl ftw(const char *, int (*) (const char *, const struct stat *, int), int);
#endif
diff --git a/mingw-w64-headers/crt/ftw.h b/mingw-w64-headers/crt/ftw.h
index f491ca5a640c..216aeb4d3a29 100644
--- a/mingw-w64-headers/crt/ftw.h
+++ b/mingw-w64-headers/crt/ftw.h
@@ -53,7 +53,15 @@ extern "C" {
/* Continue with FTW_DP callback for current directory (if FTW_DEPTH) and then its siblings. */
#define FTW_SKIP_SIBLINGS 3
-#ifndef __CRT_BUILDING_FTW
+#ifdef _CRTBLD
+ /*
+ * When building mingw-w64 CRT files it is required that the ftw and
+ * nftw functions are not declared with __MINGW_ASM_CALL redirection.
+ * Otherwise the mingw-w64 would provide broken ftw and nftw symbols.
+ */
+ int ftw (const char *, int (*) (const char *, const struct stat *, int), int);
+ int nftw (const char *, int (*) (const char *, const struct stat *, int , struct FTW *), int, int);
+#else
#if defined(_FILE_OFFSET_BITS) && (_FILE_OFFSET_BITS == 64)
#ifdef _USE_32BIT_TIME_T
int ftw (const char *, int (*) (const char *, const struct stat *, int), int) __MINGW_ASM_CALL(ftw32i64);
@@ -70,11 +78,11 @@ extern "C" {
int ftw (const char *, int (*) (const char *, const struct stat *, int), int) __MINGW_ASM_CALL(ftw64i32);
int nftw (const char *, int (*) (const char *, const struct stat *, int , struct FTW *), int, int) __MINGW_ASM_CALL(nftw64i32);
#endif
+#endif
#endif
int ftw64 (const char *, int (*) (const char *, const struct stat64 *, int), int);
int nftw64 (const char *, int (*) (const char *, const struct stat64 *, int , struct FTW *), int, int);
-#endif
#ifdef __cplusplus
}
_______________________________________________
Mingw-w64-public mailing list
Mingw-w64-public@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mingw-w64-public
