vfprintf: Do not resize work buffer based on width/precision
Since commit 3e95f6602b226e0de06aaff686dc47b282d7cc16 (Remove
limitation on size of precision for integers), padding is no
longer written to the work buffer, only the number itself
(and its transformations), so a statically sized buffer is
sufficient.
This commit does not remove existing overflow checks which
lead to early failures, but it adjusts them not to use EXTSIZE.
This causes one of the EOVERFLOW tests in stdio-comment/bug22 to
pass. This commit adjust it so that it fails again.
2017-05-30 Florian Weimer <fweimer@redhat.com>
* stdio-common/vfprintf.c (EXTSIZ): Remove definition.
(WORK_BUFFER_SIZE): Expand comment.
(vfprintf): Remove workstart. Turn workend into a constant.
Remove work buffer resizing.
(printf_positional): Likewise.
* stdio-common/bug22.c: Convert to <support/test-driver.c>.
(do_test): Adjust SN3 test to trigger the overflow again.
@@ -1,6 +1,7 @@
/* BZ #5424 */
#include <stdio.h>
#include <errno.h>
+#include <support/check.h>
/* INT_MAX + 1 */
#define N 2147483648
@@ -32,25 +33,26 @@ do_test (void)
ret = fprintf (fp, "%" SN "d", 1);
printf ("ret = %d\n", ret);
- if (ret != -1 || errno != EOVERFLOW)
- return 1;
+ TEST_VERIFY (ret == -1);
+ TEST_VERIFY (errno == EOVERFLOW);
ret = fprintf (fp, "%." SN "d", 1);
printf ("ret = %d\n", ret);
- if (ret != -1 || errno != EOVERFLOW)
- return 1;
+ TEST_VERIFY (ret == -1);
+ TEST_VERIFY (errno == EOVERFLOW);
- ret = fprintf (fp, "%." SN3 "d", 1);
+ ret = fprintf (fp, "%." SN3 "d%4d", 1, 2);
printf ("ret = %d\n", ret);
- if (ret != -1 || errno != EOVERFLOW)
- return 1;
+ TEST_VERIFY (ret == -1);
+ TEST_VERIFY (errno == EOVERFLOW);
ret = fprintf (fp, "%" SN2 "d%" SN2 "d", 1, 1);
printf ("ret = %d\n", ret);
+ TEST_VERIFY (ret == -1);
+ TEST_VERIFY (errno == EOVERFLOW);
- return ret != -1 || errno != EOVERFLOW;
+ return 0;
}
#define TIMEOUT 60
-#define TEST_FUNCTION do_test ()
-#include "../test-skeleton.c"
+#include <support/test-driver.c>
@@ -45,10 +45,6 @@
#define va_list _IO_va_list
#undef BUFSIZ
#define BUFSIZ _IO_BUFSIZ
-/* In some cases we need extra space for all the output which is not
- counted in the width of the string. We assume 32 characters is
- enough. */
-#define EXTSIZ 32
#define ARGCHECK(S, Format) \
do \
{ \
@@ -204,7 +200,9 @@ typedef wchar_t THOUSANDS_SEP_T;
/* Global constants. */
static const CHAR_T null[] = L_("(null)");
-/* Size of the work_buffer variable (in characters, not bytes. */
+/* Size of the work_buffer variable (in characters, not bytes. This
+ must be large enough to store all formatted integers, including
+ grouping and alternative digits, but excluding padding. */
enum { WORK_BUFFER_SIZE = 1000 };
/* This table maps a character into a number representing a class. In
@@ -1258,8 +1256,7 @@ vfprintf (FILE *s, const CHAR_T *format, va_list ap)
/* Buffer intermediate results. */
CHAR_T work_buffer[WORK_BUFFER_SIZE];
- CHAR_T *workstart = NULL;
- CHAR_T *workend;
+ CHAR_T *const workend = work_buffer + WORK_BUFFER_SIZE;
/* We have to save the original argument pointer. */
va_list ap_save;
@@ -1367,9 +1364,6 @@ vfprintf (FILE *s, const CHAR_T *format, va_list ap)
UCHAR_T pad = L_(' ');/* Padding character. */
CHAR_T spec;
- workstart = NULL;
- workend = work_buffer + WORK_BUFFER_SIZE;
-
/* Get current character in format string. */
JUMP (*++f, step0_jumps);
@@ -1460,62 +1454,25 @@ vfprintf (FILE *s, const CHAR_T *format, va_list ap)
left = 1;
}
- if (__glibc_unlikely (width >= INT_MAX / sizeof (CHAR_T) - EXTSIZ))
+ if (__glibc_unlikely (width >= INT_MAX / sizeof (CHAR_T)))
{
__set_errno (EOVERFLOW);
done = -1;
goto all_done;
}
-
- if (width >= WORK_BUFFER_SIZE - EXTSIZ)
- {
- /* We have to use a special buffer. */
- size_t needed = ((size_t) width + EXTSIZ) * sizeof (CHAR_T);
- if (__libc_use_alloca (needed))
- workend = (CHAR_T *) alloca (needed) + width + EXTSIZ;
- else
- {
- workstart = (CHAR_T *) malloc (needed);
- if (workstart == NULL)
- {
- done = -1;
- goto all_done;
- }
- workend = workstart + width + EXTSIZ;
- }
- }
}
JUMP (*f, step1_jumps);
/* Given width in format string. */
LABEL (width):
width = read_int (&f);
-
- if (__glibc_unlikely (width == -1
- || width >= INT_MAX / sizeof (CHAR_T) - EXTSIZ))
+ if (__glibc_unlikely (width == -1 || width >= INT_MAX / sizeof (CHAR_T)))
{
__set_errno (EOVERFLOW);
done = -1;
goto all_done;
}
- if (width >= WORK_BUFFER_SIZE - EXTSIZ)
- {
- /* We have to use a special buffer. */
- size_t needed = ((size_t) width + EXTSIZ) * sizeof (CHAR_T);
- if (__libc_use_alloca (needed))
- workend = (CHAR_T *) alloca (needed) + width + EXTSIZ;
- else
- {
- workstart = (CHAR_T *) malloc (needed);
- if (workstart == NULL)
- {
- done = -1;
- goto all_done;
- }
- workend = workstart + width + EXTSIZ;
- }
- }
if (*f == L_('$'))
/* Oh, oh. The argument comes from a positional parameter. */
goto do_positional;
@@ -1564,34 +1521,6 @@ vfprintf (FILE *s, const CHAR_T *format, va_list ap)
}
else
prec = 0;
- if (prec > width && prec > WORK_BUFFER_SIZE - EXTSIZ)
- {
- /* Deallocate any previously allocated buffer because it is
- too small. */
- if (__glibc_unlikely (workstart != NULL))
- free (workstart);
- workstart = NULL;
- if (__glibc_unlikely (prec >= INT_MAX / sizeof (CHAR_T) - EXTSIZ))
- {
- __set_errno (EOVERFLOW);
- done = -1;
- goto all_done;
- }
- size_t needed = ((size_t) prec + EXTSIZ) * sizeof (CHAR_T);
-
- if (__libc_use_alloca (needed))
- workend = (CHAR_T *) alloca (needed) + prec + EXTSIZ;
- else
- {
- workstart = (CHAR_T *) malloc (needed);
- if (workstart == NULL)
- {
- done = -1;
- goto all_done;
- }
- workend = workstart + prec + EXTSIZ;
- }
- }
JUMP (*f, step2_jumps);
/* Process 'h' modifier. There might another 'h' following. */
@@ -1655,10 +1584,6 @@ vfprintf (FILE *s, const CHAR_T *format, va_list ap)
/* The format is correctly handled. */
++nspecs_done;
- if (__glibc_unlikely (workstart != NULL))
- free (workstart);
- workstart = NULL;
-
/* Look for next format specifier. */
#ifdef COMPILE_WPRINTF
f = __find_specwc ((end_of_spec = ++f));
@@ -1676,18 +1601,11 @@ vfprintf (FILE *s, const CHAR_T *format, va_list ap)
/* Hand off processing for positional parameters. */
do_positional:
- if (__glibc_unlikely (workstart != NULL))
- {
- free (workstart);
- workstart = NULL;
- }
done = printf_positional (s, format, readonly_format, ap, &ap_save,
done, nspecs_done, lead_str_end, work_buffer,
save_errno, grouping, thousands_sep);
all_done:
- if (__glibc_unlikely (workstart != NULL))
- free (workstart);
/* Unlock the stream. */
_IO_funlockfile (s);
_IO_cleanup_region_end (0);
@@ -1732,8 +1650,6 @@ printf_positional (_IO_FILE *s, const CHAR_T *format, int readonly_format,
/* Just a counter. */
size_t cnt;
- CHAR_T *workstart = NULL;
-
if (grouping == (const char *) -1)
{
#ifdef COMPILE_WPRINTF
@@ -1931,8 +1847,7 @@ printf_positional (_IO_FILE *s, const CHAR_T *format, int readonly_format,
char pad = specs[nspecs_done].info.pad;
CHAR_T spec = specs[nspecs_done].info.spec;
- workstart = NULL;
- CHAR_T *workend = work_buffer + WORK_BUFFER_SIZE;
+ CHAR_T *const workend = work_buffer + WORK_BUFFER_SIZE;
/* Fill in last information. */
if (specs[nspecs_done].width_arg != -1)
@@ -1965,27 +1880,6 @@ printf_positional (_IO_FILE *s, const CHAR_T *format, int readonly_format,
prec = specs[nspecs_done].info.prec;
}
- /* Maybe the buffer is too small. */
- if (MAX (prec, width) + EXTSIZ > WORK_BUFFER_SIZE)
- {
- if (__libc_use_alloca ((MAX (prec, width) + EXTSIZ)
- * sizeof (CHAR_T)))
- workend = ((CHAR_T *) alloca ((MAX (prec, width) + EXTSIZ)
- * sizeof (CHAR_T))
- + (MAX (prec, width) + EXTSIZ));
- else
- {
- workstart = (CHAR_T *) malloc ((MAX (prec, width) + EXTSIZ)
- * sizeof (CHAR_T));
- if (workstart == NULL)
- {
- done = -1;
- goto all_done;
- }
- workend = workstart + (MAX (prec, width) + EXTSIZ);
- }
- }
-
/* Process format specifiers. */
while (1)
{
@@ -2059,10 +1953,6 @@ printf_positional (_IO_FILE *s, const CHAR_T *format, int readonly_format,
break;
}
- if (__glibc_unlikely (workstart != NULL))
- free (workstart);
- workstart = NULL;
-
/* Write the following constant string. */
outstring (specs[nspecs_done].end_of_fmt,
specs[nspecs_done].next_fmt
@@ -2071,8 +1961,6 @@ printf_positional (_IO_FILE *s, const CHAR_T *format, int readonly_format,
all_done:
if (__glibc_unlikely (args_malloced != NULL))
free (args_malloced);
- if (__glibc_unlikely (workstart != NULL))
- free (workstart);
scratch_buffer_free (&specsbuf);
return done;
}