On Sun, 16 Jan 2022, Pali Rohár wrote:
Currently v*scanf functions are broken and crash when are called with more
than 30 arguments in va_list. This is because va_list v*scanf functions are
redirected to variadic *scanf functions and this redirect implemented in
scanf.S file has fixed limit for 30 arguments.
Number of arguments for msvcrt *scanf function can be determined from
format string by counting number of '%' characters which are not followed
by another '%' or '*'. Every scanf parameter is pointer and therefore has
fixed size which means that required stack size can be exactly calculated.
Fix this scanf.S redirect implementation by dynamically allocating stack
for exact number of pointer parameters.
---
I have tested this patch for i686 and x86_64. Both ARM (arm32 and aarch64)
changes are untested, so please test it if vsscanf() on these platforms
still works.
With this patch following code works fine without any crashing.
Compile for msvcrt with -std=c89 or -D__USE_MINGW_ANSI_STDIO=0
#include <stdio.h>
#include <stdarg.h>
int call_vsscanf(const char *str, const char *format, ...)
{
int ret;
va_list ap;
va_start(ap, format);
ret = vsscanf(str, format, ap);
va_end(ap);
return ret;
}
int main()
{
char b[53];
call_vsscanf(
"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ",
"%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c"
"%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c",
&b[0],&b[1],&b[2],&b[3],&b[4],&b[5],&b[6],&b[7],&b[8],&b[9],&b[10],&b[11],
&b[12],&b[13],&b[14],&b[15],&b[16],&b[17],&b[18],&b[19],&b[20],&b[21],
&b[22],&b[23],&b[24],&b[25],&b[26],&b[27],&b[28],&b[29],&b[30],&b[31],
&b[32],&b[33],&b[34],&b[35],&b[36],&b[37],&b[38],&b[39],&b[40],&b[41],
&b[42],&b[43],&b[44],&b[45],&b[46],&b[47],&b[48],&b[49],&b[50],&b[51]
);
printf("b=%s\n", b);
return 0;
}
For the testcase, it might be good to test with an odd number of
arguments.
I didn't read the x86 assembly changes in very close detail, but assuming
it mirrors the arm assembly change attempts, it probably looks reasonable.
I see that the code now tries to avoid loading the first couple arguments
(that are moved to register arguments) if the number of parameters is
lower than that. I think that it might be overkill (the arg pointer
probably can't be null anyway, and there should be enough space on the
stack to load a couple extra arguments), but I don't mind keeping it if
you feel strongly about it.
But it seems like the it doesn't try to align the stack as it's supposed
to (it's losing a comment that looks like this):
- /* The max number of pointers we support. Must be an even number
- to keep the 64bit stack 16byte aligned. Must not be less than 4. */
To make the code work for arm, I had to make some adjustments - see the
attached patch. (Some adjustments, like doing 'subs' before 'str', is to
reduce the latency between the 'str' and 'ldr', and likewise between
'subs' and the conditional branch - I guess I could skip that change too,
to keep this diff simpler.)
// Martin
From 172df7002c76d7d490b6bf6d6ea6cc985369fecc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Martin=20Storsj=C3=B6?= <mar...@martin.st>
Date: Mon, 24 Jan 2022 13:21:39 +0200
Subject: [PATCH] fixup: Fix stdio/scanf.S for arm/aarch64
---
mingw-w64-crt/stdio/scanf.S | 64 +++++++++++++++++++++----------------
1 file changed, 37 insertions(+), 27 deletions(-)
diff --git a/mingw-w64-crt/stdio/scanf.S b/mingw-w64-crt/stdio/scanf.S
index ae8090e4c..d9584b206 100644
--- a/mingw-w64-crt/stdio/scanf.S
+++ b/mingw-w64-crt/stdio/scanf.S
@@ -169,22 +169,27 @@ __argtos:
__argtos:
push {r4-r8, lr}
- ldr r12, [sp, #0]
+ ldr r12, [sp, #24]
cmp r3, #0
- ldrne r5, [r2], #4
- subsne r3, r3, #1
- ldrne r6, [r2], #4
- subsne r3, r3, #1
- moveq r8, #0
- beq 2b
+ beq 2f
+ subs r3, r3, #1
+ ldr r5, [r2], #4
+ beq 2f
+ subs r3, r3, #1
+ ldr r6, [r2], #4
+ beq 2f
+ /* Round the number of entries to an even number, to maintain
+ * 8 byte stack alignment. */
mov r8, r3
- sub sp, sp, r8
+ add r8, r8, #1
+ bic r8, r8, #1
+ sub sp, sp, r8, lsl #2
mov r4, sp
1: ldr r7, [r2], #4
- str r7, [r4], #4
subs r3, r3, #1
+ str r7, [r4], #4
bne 1b
2:
@@ -192,7 +197,7 @@ __argtos:
mov r3, r6
blx r12
- add sp, sp, r8
+ add sp, sp, r8, lsl #2
pop {r4-r8, pc}
#elif defined (__aarch64__)
@@ -208,38 +213,43 @@ __argtos:
mov x11, x3
mov x12, x4
- cmp r11, #0
- b.eq 2b
+ cmp x11, #0
+ b.eq 2f
- ldr x2, [x10], #8
subs x11, x11, #1
- b.eq 2b
+ ldr x2, [x10], #8
+ b.eq 2f
- ldr x3, [x10], #8
subs x11, x11, #1
- b.eq 2b
+ ldr x3, [x10], #8
+ b.eq 2f
- ldr x4, [x10], #8
subs x11, x11, #1
- b.eq 2b
+ ldr x4, [x10], #8
+ b.eq 2f
- ldr x5, [x10], #8
subs x11, x11, #1
- b.eq 2b
+ ldr x5, [x10], #8
+ b.eq 2f
- ldr x6, [x10], #8
subs x11, x11, #1
- b.eq 2b
+ ldr x6, [x10], #8
+ b.eq 2f
- ldr x7, [x10], #8
subs x11, x11, #1
- b.eq 2b
-
- sub sp, sp, x11
+ ldr x7, [x10], #8
+ b.eq 2f
+
+ /* Round the number of entries to an even number, to maintain
+ * 16 byte stack alignment. */
+ mov x13, x11
+ add x13, x13, #1
+ bic x13, x13, #1
+ sub sp, sp, x13, lsl #3
mov x9, sp
1: ldr x13, [x10], #8
- str x13, [x9], #8
subs x11, x11, #1
+ str x13, [x9], #8
b.ne 1b
2:
--
2.25.1
_______________________________________________
Mingw-w64-public mailing list
Mingw-w64-public@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mingw-w64-public
