| Age | Commit message (Collapse) | Author | Lines |
|---|
|
in nearest rounding mode exact halfway cases were not following the
round to even rule if the rounding happened at a base 1000000000 digit
boundary of the internal representation and the previous digit was odd.
e.g. printf("%.0f", 1.5) printed 1 instead of 2.
|
|
this patch fixes a large number of missed internal signed-overflow
checks and errors in determining when the return value (output length)
would exceed INT_MAX, which should result in EOVERFLOW. some of the
issues fixed were reported by Alexander Cherepanov; others were found
in subsequent review of the code.
aside from the signed overflows being undefined behavior, the
following specific bugs were found to exist in practice:
- overflows computing length of floating point formats with huge
explicit precisions, integer formats with prefix characters and huge
explicit precisions, or string arguments or format strings longer
than INT_MAX, resulted in wrong return value and wrong %n results.
- literal width and precision values outside the range of int were
misinterpreted, yielding wrong behavior in at least one well-defined
case: string formats with precision greater than INT_MAX were
sometimes truncated.
- in cases where EOVERFLOW is produced, incorrect values could be
written for %n specifiers past the point of exceeding INT_MAX.
in addition to fixing these bugs, we now stop producing output
immediately when output length would exceed INT_MAX, rather than
continuing and returning an error only at the end.
|
|
if the requested precision is close to INT_MAX, adding
LDBL_MANT_DIG/3+8 overflows. in practice the resulting undefined
behavior manifests as a large negative result, which is then used to
compute the new end pointer (z) with a wildly out-of-bounds value
(more overflow, more undefined behavior). the end result is at least
incorrect output and character count (return value); worse things do
not seem to happen, but detailed analysis has not been done.
this patch fixes the overflow by performing the intermediate
computation as unsigned; after division by 9, the final result
necessarily fits in int.
|
|
commit b91cdbe2bc8b626aa04dc6e3e84345accf34e4b1, in fixing another
issue, changed the logic for how alt-form octal adds the leading zero
to adjust the precision rather than using a prefix character. this
wrongly suppressed the zero flag by mimicing an explicit precision
given by the format string. switch back to using a prefix character.
based on bug report and patch by Dmitry V. Levin, but simplified.
|
|
formally, it seems a sign is only required when the '+' modifier
appears in the format specifier, in which case either '+' or '-' must
be present in the output. but the specification is written such that
an optional negative sign is part of the output format anyway, and the
simplest approach to fixing the problem is removing the code that was
suppressing the sign.
|
|
previously, write errors neither stopped further output attempts nor
caused the function to return an error to the caller. this could
result in silent loss of output, possibly in the middle of output in
the event of a non-permanent error.
the simplest solution is temporarily clearing the error flag for the
target stream, then suppressing further output when the error flag is
set and checking/restoring it at the end of the operation to determine
the correct return value.
since the wide version of the code internally calls the narrow fprintf
to perform some of its underlying operations, initial clearing of the
error flag is suppressed when performing a narrow vfprintf on a
wide-oriented stream. this is not a problem since the behavior of
narrow operations on wide-oriented streams is undefined.
|
|
in this case there are two conflicting rules in play: that an explicit
precision of zero with the value zero produces no output, and that the
'#' modifier for octal increases the precision sufficiently to yield a
leading zero. ISO C (7.19.6.1 paragraph 6 in C99+TC3) includes a
parenthetical remark to clarify that the precision-increasing behavior
takes precedence, but the corresponding text in POSIX off of which I
based the implementation is missing this remark.
this issue was covered in WG14 DR#151.
|
|
CONCAT(0x1p,LDBL_MANT_DIG) is not safe outside of libc,
use 2/LDBL_EPSILON instead.
fix was proposed by Morten Welinder.
|
|
the subsequent rounding code assumes the end pointer (z) accurately
reflects the end of significance in the decimal expansion, but for
certain large integers, spurious trailing zero slots were left behind
when applying the binary exponent.
issue reported by Morten Welinder; the analysis of the cause was
performed by nsz, who also proposed this change.
|
|
the code to strip trailing zeros was only looking in the last slot for
up to 9 zeros, assuming that the rounding code had already removed
fully-zero slots from the end. however, this ignored cases where the
rounding code did not run at all, which occur when the value being
printed is exactly representable in the requested precision.
the simplest solution is to move the code that strips trailing zero
slots to run unconditionally, immediately after rounding, rather than
as the last step of rounding.
|
|
in cases where rounding caused a carry, the slot into which the carry
was taking place was unconditionally treated as valid, despite the
possibility that it could be a new slot prior to the beginning of the
existing non-rounded number. in theory this could lead to unbounded
runaway carry, but in order for that to happen, the whole
uninitialized buffer would need to have been pre-filled with 32-bit
integer values greater than or equal to 999999999.
patch based on proposed fix by Morten Welinder, who also discovered
and reported the bug.
|
|
the printf floating point formatting code contains an optimization to
avoid computing digits that will be thrown away by rounding at the
specified (or default) precision. while it was correctly retaining all
places up to the last decimal place to be printed, it was not
retaining enough precision to see the next nonzero decimal place in
all cases. this could cause incorrect rounding down in round-to-even
(default) rounding mode, for example, when printing 0.5+DBL_EPSILON
with "%.0f".
in the fix, LDBL_MANT_DIG/3 is a lazy (non-sharp) upper bound on the
number of zeros between any two nonzero decimal digits.
|
|
empirically the overflow was an off-by-one, and it did not seem to be
overwriting meaningful data. rather than simply increasing the buffer
size by one, however, I have attempted to make the size obviously
correct in terms of bounds on the number of iterations for the loops
that fill the buffer. this still results in no more than a negligible
size increase of the buffer on the stack (6-7 32-bit slots) and is a
"safer" fix unless/until somebody wants to do the proof that a smaller
buffer would suffice.
|
|
add missing va_end and remove some unnecessary code.
|
|
check in configure to be polite (failing early if we're going to fail)
and in vfprintf.c since that is the point at which a mismatching type
would be extremely dangerous.
|
|
this header evolved to facilitate the extremely lazy practice of
omitting explicit includes of the necessary headers in individual
stdio source files; not only was this sloppy, but it also increased
build time.
now, stdio_impl.h is only including the headers it needs for its own
use; any further headers needed by source files are included directly
where needed.
|
|
|
|
to deal with the fact that the public headers may be used with pre-c99
compilers, __restrict is used in place of restrict, and defined
appropriately for any supported compiler. we also avoid the form
[restrict] since older versions of gcc rejected it due to a bug in the
original c99 standard, and instead use the form *restrict.
|
|
the strspn call was made for every format specifier and end-of-string,
even though the expected return value was 1-2 for normal usage.
replace with simple loop.
|
|
this one could never cause any problems unless the compiler/machine
goes to extra trouble to break oob pointer arithmetic, but it's best
to fix it anyway.
|
|
large precision values could cause out-of-bounds pointer arithmetic in
computing the precision cutoff (used to avoid expensive long-precision
arithmetic when the result will be discarded). per the C standard,
this is undefined behavior. one would expect that it works anyway, and
in fact it did in most real-world cases, but it was randomly
(depending on aslr) crashing in i386 binaries running on x86_64
kernels. this is because linux puts the userspace stack near 4GB
(instead of near 3GB) when the kernel is 64-bit, leading to the
out-of-bounds pointer arithmetic overflowing past the end of address
space and giving a very low pointer value, which then compared lower
than a pointer it should have been higher than.
the new code rearranges the arithmetic so that no overflow can occur.
while this bug could crash printf with memory corruption, it's
unlikely to have security impact in real-world applications since the
ability to provide an extremely large field precision value under
attacker-control is required to trigger the bug.
|
|
signedness issue kept %ls with no precision from working at all
|
|
printf was not printing too many characters, but it was reading one
too many wchar_t elements from the input. this could lead to crashes
if running off the page, or spurious failure if the conversion of the
extra wchar_t resulted in EILSEQ.
|
|
vfprintf temporarily swaps in a local buffer (for the duration of the
operation) when the target stream is unbuffered; this both simplifies
the implementation of functions like dprintf (they don't need their
own buffers) and eliminates the pathologically bad performance of
writing the formatted output with one or more write syscalls per
formatting field.
in cases like dprintf where we are dealing with a virgin FILE
structure, everything worked correctly. however for long-lived files
(like stderr), it's possible that the buffer bounds were already set
for the internal zero-size buffer. on the next write, __stdio_write
would pick up and use the new buffer provided by vfprintf, but the
bound (wend) field was still pointing at the internal zero-size
buffer's end. this in turn allowed unbounded writes to the temporary
buffer.
|
|
the l prefix is redundant/no-op with printf, since default promotions
always promote floats to double; however, it is valid, and printf was
wrongly rejecting it.
|
|
passing null pointer for %s is UB but lots of broken programs do it anyway
|
|
|
|
|
|
|
|
the observed symptom was that the code was incorrectly rounding up
1.0625 to 1.063 despite the rounding mode being round-to-nearest with
ties broken by rounding to even last place. however, the code was just
not right in many respects, and i'm surprised it worked as well as it
did. this time i tested the values that end up in the variables round,
small, and the expression round+small, and all look good.
|
|
|
|
|
|
|
|
this change makes it so most calls to fprintf(stderr, ...) will result
in a single writev syscall, as opposed to roughly 2*N syscalls (and
possibly more) where N is the number of format specifiers. in
principle we could use a much larger buffer, but it's best not to
increase the stack requirements too much. most messages are under 80
chars.
|
|
the biggest change in this commit is that stdio now uses readv to fill
the caller's buffer and the FILE buffer with a single syscall, and
likewise writev to flush the FILE buffer and write out the caller's
buffer in a single syscall.
making this change required fundamental architectural changes to
stdio, so i also made a number of other improvements in the process:
- the implementation no longer assumes that further io will fail
following errors, and no longer blocks io when the error flag is set
(though the latter could easily be changed back if desired)
- unbuffered mode is no longer implemented as a one-byte buffer. as a
consequence, scanf unreading has to use ungetc, to the unget buffer
has been enlarged to hold at least 2 wide characters.
- the FILE structure has been rearranged to maintain the locations of
the fields that might be used in glibc getc/putc type macros, while
shrinking the structure to save some space.
- error cases for fflush, fseek, etc. should be more correct.
- library-internal macros are used for getc_unlocked and putc_unlocked
now, eliminating some ugly code duplication. __uflow and __overflow
are no longer used anywhere but these macros. switch to read or
write mode is also separated so the code can be better shared, e.g.
with ungetc.
- lots of other small things.
|
|
sadly the C language does not specify any such implicit conversion, so
this is not a matter of just fixing warnings (as gcc treats it) but
actual errors. i would like to revisit a number of these changes and
possibly revise the types used to reduce the number of casts required.
|
|
|
|
|
|
|
