Re: [Mingw-w64-public] [PATCH] headers: Use asm() for redirecting time functions, instead of inline functions
On Wed, 8 May 2024, Jacek Caban wrote: On 8.05.2024 14:22, Martin Storsjö wrote: Prior to 1652e9241b5d8a5a779c6582b1c3c4f4a7cc66e5, the inline functions always were static. Due to reexporting such symbols in C++20 modules (for the C++23 std module), the reexported symbols must not be static, so the inline functions were changed from static inline to __mingw_ovr, which practically is static inline in C mode, but regular inline in C++ mode. By using regular inline in C++ mode, each use of the functions can (but doesn't need to) emit an external symbol for the inline function, and the callers may either call that function or inline the body of the function. This can have two potential issues; if different translation units are configured differently (with the _USE_32BIT_TIME_T define), but both use the same external symbol for it, the linker will only keep one of them (as they're both inline, and supposed to be the same). In practice, this is rare for _USE_32BIT_TIME_T though. Secondly, such an external symbol may conflict with the actual import library. Such a case was reported at https://github.com/mstorsjo/llvm-mingw/issues/427. (Practically, the issue there was that some built object files defined an external "_time" symbol, i.e. regular "time" with i386 cdecl underscore prefix, from the non-static inline functions. The object also files reference _time32 with dllimport, which via the weak aliases produced by llvm-dlltool end up pulling in the __imp__time symbol, which also brings in a conflicting "_time" symbol.) In short - inline functions can be problematic. Where possible, it's less problematic to use asm(), via __MINGW_ASM_CALL(), to redirect calls directly towards the right function. This has a slight drawback, that this ends up calling the thunks (as the declarations lack dllimport), while we previously could inline the call directly to a dllimported function (avoiding the thunk, fetching the target address via the __imp_ prefixed symbol). We could, easily, add the dllimport attributes on these declarations, but that triggers a GCC bug for how those symbol names are mangled on i386, see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114984. (The bug seems to be noted and mentioned as early as 2007, in https://sourceware.org/pipermail/cygwin/2007-February/154845.html, but it doesn't seem to have been fixed since.) Signed-off-by: Martin Storsjö --- mingw-w64-headers/crt/time.h | 62 1 file changed, 28 insertions(+), 34 deletions(-) Looks good to me. Thanks, pushed now. // Martin ___ Mingw-w64-public mailing list Mingw-w64-public@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mingw-w64-public
Re: [Mingw-w64-public] [PATCH] headers: Use asm() for redirecting time functions, instead of inline functions
On 8.05.2024 14:22, Martin Storsjö wrote: Prior to 1652e9241b5d8a5a779c6582b1c3c4f4a7cc66e5, the inline functions always were static. Due to reexporting such symbols in C++20 modules (for the C++23 std module), the reexported symbols must not be static, so the inline functions were changed from static inline to __mingw_ovr, which practically is static inline in C mode, but regular inline in C++ mode. By using regular inline in C++ mode, each use of the functions can (but doesn't need to) emit an external symbol for the inline function, and the callers may either call that function or inline the body of the function. This can have two potential issues; if different translation units are configured differently (with the _USE_32BIT_TIME_T define), but both use the same external symbol for it, the linker will only keep one of them (as they're both inline, and supposed to be the same). In practice, this is rare for _USE_32BIT_TIME_T though. Secondly, such an external symbol may conflict with the actual import library. Such a case was reported at https://github.com/mstorsjo/llvm-mingw/issues/427. (Practically, the issue there was that some built object files defined an external "_time" symbol, i.e. regular "time" with i386 cdecl underscore prefix, from the non-static inline functions. The object also files reference _time32 with dllimport, which via the weak aliases produced by llvm-dlltool end up pulling in the __imp__time symbol, which also brings in a conflicting "_time" symbol.) In short - inline functions can be problematic. Where possible, it's less problematic to use asm(), via __MINGW_ASM_CALL(), to redirect calls directly towards the right function. This has a slight drawback, that this ends up calling the thunks (as the declarations lack dllimport), while we previously could inline the call directly to a dllimported function (avoiding the thunk, fetching the target address via the __imp_ prefixed symbol). We could, easily, add the dllimport attributes on these declarations, but that triggers a GCC bug for how those symbol names are mangled on i386, see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114984. (The bug seems to be noted and mentioned as early as 2007, in https://sourceware.org/pipermail/cygwin/2007-February/154845.html, but it doesn't seem to have been fixed since.) Signed-off-by: Martin Storsjö --- mingw-w64-headers/crt/time.h | 62 1 file changed, 28 insertions(+), 34 deletions(-) Looks good to me. Thanks, Jacek ___ Mingw-w64-public mailing list Mingw-w64-public@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mingw-w64-public
Re: [Mingw-w64-public] [PATCH] headers: Use asm() for redirecting time functions, instead of inline functions
Hi,
thanks for working on this and unifying this inlining issues. Looks good to me.
Thanks
Kai
Am Mi., 8. Mai 2024 um 14:23 Uhr schrieb Martin Storsjö :
>
> Prior to 1652e9241b5d8a5a779c6582b1c3c4f4a7cc66e5, the inline
> functions always were static. Due to reexporting such symbols
> in C++20 modules (for the C++23 std module), the reexported symbols
> must not be static, so the inline functions were changed
> from static inline to __mingw_ovr, which practically is static
> inline in C mode, but regular inline in C++ mode.
>
> By using regular inline in C++ mode, each use of the functions
> can (but doesn't need to) emit an external symbol for the
> inline function, and the callers may either call that function
> or inline the body of the function.
>
> This can have two potential issues; if different translation units
> are configured differently (with the _USE_32BIT_TIME_T define),
> but both use the same external symbol for it, the linker will only
> keep one of them (as they're both inline, and supposed to be the
> same). In practice, this is rare for _USE_32BIT_TIME_T though.
>
> Secondly, such an external symbol may conflict with the actual
> import library. Such a case was reported at
> https://github.com/mstorsjo/llvm-mingw/issues/427.
>
> (Practically, the issue there was that some built object files
> defined an external "_time" symbol, i.e. regular "time" with i386
> cdecl underscore prefix, from the non-static inline functions. The
> object also files reference _time32 with dllimport, which via the
> weak aliases produced by llvm-dlltool end up pulling in the
> __imp__time symbol, which also brings in a conflicting "_time" symbol.)
>
> In short - inline functions can be problematic. Where possible,
> it's less problematic to use asm(), via __MINGW_ASM_CALL(), to
> redirect calls directly towards the right function.
>
> This has a slight drawback, that this ends up calling the thunks
> (as the declarations lack dllimport), while we previously could
> inline the call directly to a dllimported function (avoiding the
> thunk, fetching the target address via the __imp_ prefixed symbol).
>
> We could, easily, add the dllimport attributes on these declarations,
> but that triggers a GCC bug for how those symbol names are mangled
> on i386, see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114984. (The
> bug seems to be noted and mentioned as early as 2007, in
> https://sourceware.org/pipermail/cygwin/2007-February/154845.html,
> but it doesn't seem to have been fixed since.)
>
> Signed-off-by: Martin Storsjö
> ---
> mingw-w64-headers/crt/time.h | 62
> 1 file changed, 28 insertions(+), 34 deletions(-)
>
> diff --git a/mingw-w64-headers/crt/time.h b/mingw-w64-headers/crt/time.h
> index f8401903c..d94a9f619 100644
> --- a/mingw-w64-headers/crt/time.h
> +++ b/mingw-w64-headers/crt/time.h
> @@ -213,26 +213,20 @@ extern "C" {
>
> #if !defined (RC_INVOKED) && !defined (_INC_WTIME_INL)
> #define _INC_WTIME_INL
> - wchar_t *__cdecl _wctime(const time_t *)
> __MINGW_ATTRIB_DEPRECATED_SEC_WARN;
> -#ifndef __CRT__NO_INLINE
> #ifndef _USE_32BIT_TIME_T
> - __CRT_INLINE wchar_t *__cdecl _wctime(const time_t *_Time) { return
> _wctime64(_Time); }
> + wchar_t *__cdecl _wctime(const time_t *_Time)
> __MINGW_ATTRIB_DEPRECATED_SEC_WARN __MINGW_ASM_CALL(_wctime64);
> #else
> - __CRT_INLINE wchar_t *__cdecl _wctime(const time_t *_Time) { return
> _wctime32(_Time); }
> + wchar_t *__cdecl _wctime(const time_t *_Time)
> __MINGW_ATTRIB_DEPRECATED_SEC_WARN __MINGW_ASM_CALL(_wctime32);
> #endif
> -#endif /* __CRT__NO_INLINE */
> #endif
>
> #if !defined (RC_INVOKED) && !defined (_INC_WTIME_S_INL)
> #define _INC_WTIME_S_INL
> - errno_t __cdecl _wctime_s(wchar_t *, size_t, const time_t *);
> -#ifndef __CRT__NO_INLINE
> #ifndef _USE_32BIT_TIME_T
> - __CRT_INLINE errno_t __cdecl _wctime_s (wchar_t *_Buffer,size_t
> _SizeInWords,const time_t *_Time) { return _wctime64_s
> (_Buffer,_SizeInWords,_Time); }
> + errno_t __cdecl _wctime_s (wchar_t *_Buffer,size_t _SizeInWords,const
> time_t *_Time) __MINGW_ASM_CALL(_wctime64_s);
> #else
> - __CRT_INLINE errno_t __cdecl _wctime_s (wchar_t *_Buffer,size_t
> _SizeInWords,const time_t *_Time) { return _wctime32_s
> (_Buffer,_SizeInWords,_Time); }
> + errno_t __cdecl _wctime_s (wchar_t *_Buffer,size_t _SizeInWords,const
> time_t *_Time) __MINGW_ASM_CALL(_wctime32_s);
> #endif
> -#endif /* __CRT__NO_INLINE */
> #endif
>
> #define _WTIME_DEFINED
> @@ -241,33 +235,33 @@ extern "C" {
> #ifndef RC_INVOKED
>
> #ifdef _USE_32BIT_TIME_T
> -__mingw_ovr time_t __CRTDECL time(time_t *_Time) { return _time32(_Time); }
> +time_t __CRTDECL time(time_t *_Time) __MINGW_ASM_CALL(_time32);
> #ifdef _UCRT
> -__mingw_ovr int __CRTDECL timespec_get(struct timespec* _Ts, int _Base) {
> return _timespec32_get((struct _timespec32*)_Ts, _Base); }
> -#endif
> -__mingw_ovr double __CRTDECL difftime(time_t _Time1,time_t _Time2) { return
>
[Mingw-w64-public] [PATCH] headers: Use asm() for redirecting time functions, instead of inline functions
Prior to 1652e9241b5d8a5a779c6582b1c3c4f4a7cc66e5, the inline
functions always were static. Due to reexporting such symbols
in C++20 modules (for the C++23 std module), the reexported symbols
must not be static, so the inline functions were changed
from static inline to __mingw_ovr, which practically is static
inline in C mode, but regular inline in C++ mode.
By using regular inline in C++ mode, each use of the functions
can (but doesn't need to) emit an external symbol for the
inline function, and the callers may either call that function
or inline the body of the function.
This can have two potential issues; if different translation units
are configured differently (with the _USE_32BIT_TIME_T define),
but both use the same external symbol for it, the linker will only
keep one of them (as they're both inline, and supposed to be the
same). In practice, this is rare for _USE_32BIT_TIME_T though.
Secondly, such an external symbol may conflict with the actual
import library. Such a case was reported at
https://github.com/mstorsjo/llvm-mingw/issues/427.
(Practically, the issue there was that some built object files
defined an external "_time" symbol, i.e. regular "time" with i386
cdecl underscore prefix, from the non-static inline functions. The
object also files reference _time32 with dllimport, which via the
weak aliases produced by llvm-dlltool end up pulling in the
__imp__time symbol, which also brings in a conflicting "_time" symbol.)
In short - inline functions can be problematic. Where possible,
it's less problematic to use asm(), via __MINGW_ASM_CALL(), to
redirect calls directly towards the right function.
This has a slight drawback, that this ends up calling the thunks
(as the declarations lack dllimport), while we previously could
inline the call directly to a dllimported function (avoiding the
thunk, fetching the target address via the __imp_ prefixed symbol).
We could, easily, add the dllimport attributes on these declarations,
but that triggers a GCC bug for how those symbol names are mangled
on i386, see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114984. (The
bug seems to be noted and mentioned as early as 2007, in
https://sourceware.org/pipermail/cygwin/2007-February/154845.html,
but it doesn't seem to have been fixed since.)
Signed-off-by: Martin Storsjö
---
mingw-w64-headers/crt/time.h | 62
1 file changed, 28 insertions(+), 34 deletions(-)
diff --git a/mingw-w64-headers/crt/time.h b/mingw-w64-headers/crt/time.h
index f8401903c..d94a9f619 100644
--- a/mingw-w64-headers/crt/time.h
+++ b/mingw-w64-headers/crt/time.h
@@ -213,26 +213,20 @@ extern "C" {
#if !defined (RC_INVOKED) && !defined (_INC_WTIME_INL)
#define _INC_WTIME_INL
- wchar_t *__cdecl _wctime(const time_t *) __MINGW_ATTRIB_DEPRECATED_SEC_WARN;
-#ifndef __CRT__NO_INLINE
#ifndef _USE_32BIT_TIME_T
- __CRT_INLINE wchar_t *__cdecl _wctime(const time_t *_Time) { return
_wctime64(_Time); }
+ wchar_t *__cdecl _wctime(const time_t *_Time)
__MINGW_ATTRIB_DEPRECATED_SEC_WARN __MINGW_ASM_CALL(_wctime64);
#else
- __CRT_INLINE wchar_t *__cdecl _wctime(const time_t *_Time) { return
_wctime32(_Time); }
+ wchar_t *__cdecl _wctime(const time_t *_Time)
__MINGW_ATTRIB_DEPRECATED_SEC_WARN __MINGW_ASM_CALL(_wctime32);
#endif
-#endif /* __CRT__NO_INLINE */
#endif
#if !defined (RC_INVOKED) && !defined (_INC_WTIME_S_INL)
#define _INC_WTIME_S_INL
- errno_t __cdecl _wctime_s(wchar_t *, size_t, const time_t *);
-#ifndef __CRT__NO_INLINE
#ifndef _USE_32BIT_TIME_T
- __CRT_INLINE errno_t __cdecl _wctime_s (wchar_t *_Buffer,size_t
_SizeInWords,const time_t *_Time) { return _wctime64_s
(_Buffer,_SizeInWords,_Time); }
+ errno_t __cdecl _wctime_s (wchar_t *_Buffer,size_t _SizeInWords,const time_t
*_Time) __MINGW_ASM_CALL(_wctime64_s);
#else
- __CRT_INLINE errno_t __cdecl _wctime_s (wchar_t *_Buffer,size_t
_SizeInWords,const time_t *_Time) { return _wctime32_s
(_Buffer,_SizeInWords,_Time); }
+ errno_t __cdecl _wctime_s (wchar_t *_Buffer,size_t _SizeInWords,const time_t
*_Time) __MINGW_ASM_CALL(_wctime32_s);
#endif
-#endif /* __CRT__NO_INLINE */
#endif
#define _WTIME_DEFINED
@@ -241,33 +235,33 @@ extern "C" {
#ifndef RC_INVOKED
#ifdef _USE_32BIT_TIME_T
-__mingw_ovr time_t __CRTDECL time(time_t *_Time) { return _time32(_Time); }
+time_t __CRTDECL time(time_t *_Time) __MINGW_ASM_CALL(_time32);
#ifdef _UCRT
-__mingw_ovr int __CRTDECL timespec_get(struct timespec* _Ts, int _Base) {
return _timespec32_get((struct _timespec32*)_Ts, _Base); }
-#endif
-__mingw_ovr double __CRTDECL difftime(time_t _Time1,time_t _Time2) { return
_difftime32(_Time1,_Time2); }
-__mingw_ovr struct tm *__CRTDECL localtime(const time_t *_Time) { return
_localtime32(_Time); }
-__mingw_ovr errno_t __CRTDECL localtime_s(struct tm *_Tm,const time_t *_Time)
{ return _localtime32_s(_Tm,_Time); }
-__mingw_ovr struct tm *__CRTDECL gmtime(const time_t *_Time) { return
_gmtime32(_Time); }
-__mingw_ovr errno_t
