On Thu, 25 Apr 2024, Pali Rohár wrote:
On Wednesday 24 April 2024 23:41:24 Martin Storsjö wrote:
On Mon, 22 Apr 2024, Pali Rohár wrote:
diff --git a/mingw-w64-crt/lib-common/ucrtbase.def.in
b/mingw-w64-crt/lib-common/ucrtbase.def.in
index 2d0552f277ef..c8fb1e33b1d7 100644
--- a/mingw-w64-crt/lib-common/ucrtbase.def.in
+++ b/mingw-w64-crt/lib-common/ucrtbase.def.in
@@ -327,12 +327,10 @@ _filelength
_filelengthi64
_fileno
_findclose
-_findfirst == _findfirst64
Note how we unconditionally had these macros pointing at _findfirst64 in
ucrt, regardless of architecture.
I must admit that this is an interesting issue.
I looked at the mingw-w64-headers/crt/io.h file now and it is not set
unconditionally as you wrote.
I didn't say that the headers unconditionally uses it this way. I just
said that the exist, most probably buggy, def file line right above what
I quoted, unconditionally does "_findfirst == _findfirst64" for all
architectures.
Yes it's probably wrong - but we shouldn't be changing it in a refactoring
patch, that's all I tried to say here.
Anyway, this def file defines symbol alias "_findfirst" for linking. And
every c/c++ source file which includes headers files would result in
object file which will use either "_findfirst32" or "_findfirst64i32".
Not "_findfirst" symbol. So in my opinion, this change should be safe.
It may very well be safe in practice, but I don't want such a subtle,
unannounced change buried in a patch that refactors many hundreds of
lines, where the rest mostly is a pure refactoring.
I think that meaning of the symbols should be same independently of the
used CRT library. And for me this looks like a bug that UCRT was
resolving _findfirst to different ABI function than msvcrt, at link
time.
I disagree here. UCRT has a different ABI than msvcrt, overall, with
time_t defaulting to 64 bit on UCRT on 32 bit platforms, contrary to
msvcrt.
I think the default symbol here should match what the headers default to,
when used in a UCRT configuration.
I guess that it was rare to hit this issue as it required to either not
include mingw-w64 header files and declare functions manually. Or write
application in other language which does not use mingw-w64 C header
files (e.g. in assembly or something similar) but uses mingw-w64 CRT
libraries for linking.
Yes, this is not something that would be encountered often in practice, I
agree with that.
Please check what I wrote above. If you agree that this is a bug, I can
prepare a separate fix for the UCRT (and then rebase 10/10 on top of
it).
I agree that the current state probably is a bug.
As the headers default to 64 bit time_t, then _findfirst should probably
resolve to _findfirst64i32, for both 32 and 64 bit. Similarly for the
other symbols.
That's most probably my original intent in how I set these up - that we
point at the "64 bit version", but I missed that it really is
_findfirst64i32 and not _findfirst64.
I'd appreciate a patch that fixes these aliases, consistently for
ucrtbase.def.in and the api-ms-win-crt-*.def.in, and then we can rebase
patch 10/10 on top of it, where I'd expect no changes in the preprocessed
output of the ucrt def files.
Ou, and another files for which I forget api-ms-win-crt-*.def.in. And
this is because they do not include that common .def.in file.
I can look at them, but I would prefer to do it after all existing stuff
is resolved as basically api-ms-win-crt-*.def.in does not use any common
aliases.
In my opinion, it would be better to change *crt*.def.in and
api-ms-win-crt-*.def.in files to use common alias (would need refactor).
Yes, that would indeed require a bit of refactoring. I'm not requesting
you do it, but if you feel you want to take it on, feel free.
Just make sure that the aliases in ucrtbase.def.in (or transitively from
msvcrt-common.def.in) match api-ms-win-crt-*.def.in.
// Martin
_______________________________________________
Mingw-w64-public mailing list
Mingw-w64-public@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mingw-w64-public
