Re: [Mingw-w64-public] [PATCH] crt: Fix comments in msvcrt-common.def.in
On Sun, 3 Mar 2024, Pali Rohár wrote: On Sunday 03 March 2024 23:38:57 Martin Storsjö wrote: On Sun, 3 Mar 2024, Pali Rohár wrote: Ok, do you need me to resend this patch? Or would you fix this one missing semicolon during applying patch? I can fix the missing semicolon. And... I do not know how to better write the commit message, so I would be happy if give me example of how to rephrase it. Does this sound like a good commit message? If it's ok with you, I can push it in that form: ---8<--- crt: Fix comments in msvcrt-common.def.in The ADD_UNDERSCORE macro adds an alias, without a leading underscore, for an existing export with a leading underscore. For some functions, we're not providing these aliases, e.g. because those symbols are defined by other means, e.g. in source files (such as CRT_fp10.c and CRT_fp8.c). This patch improves the style of those cases, which are listed as commented out instances of the ADD_UNDERSCORE macro. ---8<--- // Martin That is nice, I like it! Thank you very much. Pushed in that form, thanks! // 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] Use rand_s in mkstemp function
Hi,
On Mon, 4 Mar 2024, Mateusz Mikuła wrote:
rand is not random enough and may lead to clashing temporary directories
with multiple parallel link processes as it was observed on Rust's CI.
It can be reproduced with these commands (run them all in without long pauses):
for n in {1..15000}; do rm -f lib/libLLVMAVRAsmParser.a && \
ar qc lib/libLLVMAVRAsmParser.a
lib/Target/AVR/AsmParser/CMakeFiles/LLVMAVRAsmParser.dir/AVRAsmParser.cpp.obj
&& \
ranlib.exe lib/libLLVMAVRAsmParser.a; done &
for n in {1..15000}; do rm -f lib/libLLVMSparcCodeGen.a && \
ar qc lib/libLLVMSparcCodeGen.a lib/Target/Sparc/CMakeFiles/LLVMSparcCodeGen.dir/*.obj
&& \
ranlib.exe lib/libLLVMSparcCodeGen.a; done
echo "done"
fg
Before the patch it will fail with an error: ranlib.exe: could not create
temporary file whilst writing archive: no more archived files.
Thanks, I've run into this issue occasionally when building LLVM on msys2
as well, but I've failed to reproduce it when I've tried to look closer at
it (as I've missed the issue that one needs to build two archives at the
same time in order to trigger it).
If the issue is that the randomness clashes, shouldn't that be something
that, as part of the contract of mkstemp, the function should retry until
it finds a non-conflicting combination? But, thinking further, is the
issue that two processes end up trying the same sequence of pseudo random
files, which all then end up clashing, and mkstemp returns an error as it
was unable to find a unique file name? I guess that's plausible. In that
case, I guess this patch is fine (with Liu Hao's suggestion), as a way to
reduce the risk of running into this.
// 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] Use rand_s in mkstemp function
在 2024-03-04 08:04, Mateusz Mikuła 写道: -template_name[j] = letters[rand () % 62]; +if (rand_s ()) return -1; +template_name[j] = letters[r % 62]; Does it make sense that if `rand_s()` fails, use `rand()` as a fallback? We have a custom implementation of `rand_s()` in 'secapi/rand_s.c' which may return `EINVAL` on Windows 2000. -- Best regards, LIU Hao OpenPGP_signature.asc Description: OpenPGP digital signature ___ Mingw-w64-public mailing list Mingw-w64-public@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mingw-w64-public
[Mingw-w64-public] [PATCH] Use rand_s in mkstemp function
rand is not random enough and may lead to clashing temporary directories
with multiple parallel link processes as it was observed on Rust's CI.
It can be reproduced with these commands (run them all in without long pauses):
for n in {1..15000}; do rm -f lib/libLLVMAVRAsmParser.a && \
ar qc lib/libLLVMAVRAsmParser.a
lib/Target/AVR/AsmParser/CMakeFiles/LLVMAVRAsmParser.dir/AVRAsmParser.cpp.obj
&& \
ranlib.exe lib/libLLVMAVRAsmParser.a; done &
for n in {1..15000}; do rm -f lib/libLLVMSparcCodeGen.a && \
ar qc lib/libLLVMSparcCodeGen.a
lib/Target/Sparc/CMakeFiles/LLVMSparcCodeGen.dir/*.obj && \
ranlib.exe lib/libLLVMSparcCodeGen.a; done
echo "done"
fg
Before the patch it will fail with an error: ranlib.exe: could not create
temporary file whilst writing archive: no more archived files.
The changes in this patch were proposed by Josh Stone on Rust's Zulip server
and I was asked to forward it.
Co-Authored-By: Josh Stone
Signed-off-by: Mateusz Mikuła
---
mingw-w64-crt/misc/mkstemp.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/mingw-w64-crt/misc/mkstemp.c b/mingw-w64-crt/misc/mkstemp.c
index 6b327f2fc..1698f49f4 100644
--- a/mingw-w64-crt/misc/mkstemp.c
+++ b/mingw-w64-crt/misc/mkstemp.c
@@ -1,3 +1,4 @@
+#define _CRT_RAND_S
#include
#include
#include
@@ -25,6 +26,7 @@
int __cdecl mkstemp (char *template_name)
{
int i, j, fd, len, index;
+unsigned int r;
/* These are the (62) characters used in temporary filenames. */
static const char letters[] =
"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789";
@@ -45,7 +47,8 @@ int __cdecl mkstemp (char *template_name)
*/
for (i = 0; i >= 0; i++) {
for(j = index; j < len; j++) {
-template_name[j] = letters[rand () % 62];
+if (rand_s ()) return -1;
+template_name[j] = letters[r % 62];
}
fd = _sopen(template_name,
_O_RDWR | _O_CREAT | _O_EXCL | _O_BINARY,
--
2.43.0.windows.1
___
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] crt: Fix comments in msvcrt-common.def.in
On Sunday 03 March 2024 23:38:57 Martin Storsjö wrote: > On Sun, 3 Mar 2024, Pali Rohár wrote: > > > Ok, do you need me to resend this patch? Or would you fix this one > > missing semicolon during applying patch? > > I can fix the missing semicolon. > > > And... I do not know how to better write the commit message, so I would > > be happy if give me example of how to rephrase it. > > Does this sound like a good commit message? If it's ok with you, I can push > it in that form: > > ---8<--- > crt: Fix comments in msvcrt-common.def.in > > The ADD_UNDERSCORE macro adds an alias, without a leading underscore, for an > existing export with a leading underscore. > > For some functions, we're not providing these aliases, e.g. because those > symbols are defined by other means, e.g. in source files (such as CRT_fp10.c > and CRT_fp8.c). > > This patch improves the style of those cases, which are listed as commented > out instances of the ADD_UNDERSCORE macro. > ---8<--- > > // Martin That is nice, I like it! Thank you very much. ___ 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] crt: Fix comments in msvcrt-common.def.in
On Sun, 3 Mar 2024, Pali Rohár wrote: Ok, do you need me to resend this patch? Or would you fix this one missing semicolon during applying patch? I can fix the missing semicolon. And... I do not know how to better write the commit message, so I would be happy if give me example of how to rephrase it. Does this sound like a good commit message? If it's ok with you, I can push it in that form: ---8<--- crt: Fix comments in msvcrt-common.def.in The ADD_UNDERSCORE macro adds an alias, without a leading underscore, for an existing export with a leading underscore. For some functions, we're not providing these aliases, e.g. because those symbols are defined by other means, e.g. in source files (such as CRT_fp10.c and CRT_fp8.c). This patch improves the style of those cases, which are listed as commented out instances of the ADD_UNDERSCORE macro. ---8<--- // 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] crt: Fix comments in msvcrt-common.def.in
On Thursday 29 February 2024 10:16:47 Martin Storsjö wrote:
> On Thu, 29 Feb 2024, Pali Rohár wrote:
>
> > On Wednesday 28 February 2024 23:24:25 Martin Storsjö wrote:
> > > On Wed, 28 Feb 2024, Pali Rohár wrote:
> > >
> > > > All commented lines refers to non-underscore aliases for underscored
> > > > variant of functions. Aliases for some reasons are not prevent, e.g.
> > > > because some aliases are defined in other files (CRT_fp10.c and
> > > > CRT_fp8.c).
> > >
> > > I don't understand what the second sentence here is saying, can you
> > > rephrase
> > > it?
>
> Ah, now I see, with s/prevent/present/, the sentence is a bit more
> understandable.
>
> > Ok, I will try to explain it a bit more. ADD_UNDERSCORE is a macro which
> > just adds a symbol alias. It does not export any new symbol.
> >
> > #define ADD_UNDERSCORE(symbol) symbol == _ ## symbol
> >
> > Symbol "fpreset" is already defined in CRT_fp10.c and CRT_fp8.c files as
> > an alias to the "_fpreset" symbol.
> >
> > So ADD_UNDERSCORE(fpreset) should not be added into msvcrt-common.def.in
> > because same job is already done by __attribute__((alias("_fpreset"))).
>
> Thanks, that's more understandable :-)
>
> >
> > > > ---
> > > > mingw-w64-crt/def-include/msvcrt-common.def.in | 15 ---
> > > > 1 file changed, 8 insertions(+), 7 deletions(-)
> > > >
> > >
> > > > @@ -134,13 +135,13 @@ ADD_UNDERSCORE(y0)
> > > > ADD_UNDERSCORE(y1)
> > > > ADD_UNDERSCORE(yn)
> > > > ADD_UNDERSCORE(chgsign)
> > > > -;scalb
> > > > +ADD_UNDERSCORE(scalb)
> > >
> > > This change here is, accidentally?, actually adding the alias even though
> > > it
> > > was supposed to be commented out?
> > >
> > > // Martin
> >
> > Ah, it should not be there. My attempt was to cleanup comments without
> > any functional change. But all those parenthesis, semicolons and pluses
> > in diff make me hard to miss some accidental changes.
>
> Yep. I'd be fine with the patch with the commit message typo fixed (or with
> the more verbose version), and this typo here fixed as well.
>
> // Martin
Ok, do you need me to resend this patch? Or would you fix this one
missing semicolon during applying patch?
And... I do not know how to better write the commit message, so I would
be happy if give me example of how to rephrase it.
___
Mingw-w64-public mailing list
Mingw-w64-public@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mingw-w64-public
