On Monday 24 January 2022 13:41:27 Martin Storsjö wrote:
> 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.
Hello! That is a good point!
> 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.
Well, if you think that it is OK to always copy first pointers from
stack to registers then I'm fine with 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. */
That is a good point. x86_64 asm code should be fixed for 16byte
aligning.
> 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
I'm OK with this change.
_______________________________________________
Mingw-w64-public mailing list
Mingw-w64-public@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mingw-w64-public
