Commit c234aa77 authored by Mat Hostetter's avatar Mat Hostetter Committed by Facebook GitHub Bot

Fix harmless math errors in folly::Format stack buffer manipulation.

Summary:
The math that figures out the biggest possible formatted
number, for a stack buffer size, computed 65 bytes for "binary" format:

```
    // #b: 0b prefix + 64 bytes = 65 bytes
```

But with that 2-byte "0b" prefix this of course sums to 66 bytes.

This turns out to be harmless, rather than causing a buffer overrun,
because the buffer size additionally reserves two more bytes for
"prefix shenanigans", which are redundant with the two prefix bytes
explicitly reserved for "0b" (but in this case were covering up the
off-by-one error).

I cleaned up a few things while I was there:

- I fixed the size computation for binary buffers, including
  making it use kMaxBinaryLength, which is based on `sizeof(uintmax_t)`,
  rather than hardcoding `64`.
- I removed the redundant prefix shenanigans reservation.
- I fixed the places that were reserving two bytes for a prefix
  (like "0x") right after asserting that no prefix is allowed,
  e.g. for chars and decimal numbers.
- I stopped reserving a byte for NUL, because these days this code
  uses folly::StringPiece and never NUL-terminates.

Reviewed By: yfeldblum

Differential Revision: D20694395

fbshipit-source-id: 6658675d9993d2fe223675db55c8040ddee143a8
parent 6276e84e
......@@ -481,14 +481,18 @@ class FormatValue<
"sign specifications not allowed for unsigned values");
}
// max of:
// #x: 0x prefix + 16 bytes = 18 bytes
// #o: 0 prefix + 22 bytes = 23 bytes
// #b: 0b prefix + 64 bytes = 65 bytes
// 1 byte for sign, plus max of:
// #x: two byte "0x" prefix + kMaxHexLength
// #o: one byte "0" prefix + kMaxOctalLength
// #b: two byte "0b" prefix + kMaxBinaryLength
// n: 19 bytes + 1 NUL
// ,d: 26 bytes (including thousands separators!)
// + nul terminator
// + 3 for sign and prefix shenanigans (see below)
constexpr size_t valBufSize = 69;
//
// Binary format must take the most space, so we use that.
//
// Note that we produce a StringPiece rather than NUL-terminating,
// so we don't need an extra byte for a NUL.
constexpr size_t valBufSize = 1 + 2 + detail::kMaxBinaryLength;
char valBuf[valBufSize];
char* valBufBegin = nullptr;
char* valBufEnd = nullptr;
......@@ -508,7 +512,7 @@ class FormatValue<
presentation,
"' specifier");
valBufBegin = valBuf + 3; // room for sign and base prefix
valBufBegin = valBuf + 1; // room for sign
#if defined(__ANDROID__)
int len = snprintf(
valBufBegin,
......@@ -534,7 +538,7 @@ class FormatValue<
"base prefix not allowed with '",
presentation,
"' specifier");
valBufBegin = valBuf + 3; // room for sign and base prefix
valBufBegin = valBuf + 1; // room for sign
// Use uintToBuffer, faster than sprintf
valBufEnd = valBufBegin + uint64ToBufferUnsafe(uval, valBufBegin);
......@@ -553,7 +557,7 @@ class FormatValue<
"thousands separator (',') not allowed with '",
presentation,
"' specifier");
valBufBegin = valBuf + 3;
valBufBegin = valBuf + 1; // room for sign
*valBufBegin = static_cast<char>(uval);
valBufEnd = valBufBegin + 1;
break;
......@@ -564,9 +568,8 @@ class FormatValue<
"thousands separator (',') not allowed with '",
presentation,
"' specifier");
valBufEnd = valBuf + valBufSize - 1;
valBufBegin =
valBuf + detail::uintToOctal(valBuf, valBufSize - 1, uval);
valBufEnd = valBuf + valBufSize;
valBufBegin = valBuf + detail::uintToOctal(valBuf, valBufSize, uval);
if (arg.basePrefix) {
*--valBufBegin = '0';
prefixLen = 1;
......@@ -578,9 +581,8 @@ class FormatValue<
"thousands separator (',') not allowed with '",
presentation,
"' specifier");
valBufEnd = valBuf + valBufSize - 1;
valBufBegin =
valBuf + detail::uintToHexLower(valBuf, valBufSize - 1, uval);
valBufEnd = valBuf + valBufSize;
valBufBegin = valBuf + detail::uintToHexLower(valBuf, valBufSize, uval);
if (arg.basePrefix) {
*--valBufBegin = 'x';
*--valBufBegin = '0';
......@@ -593,9 +595,8 @@ class FormatValue<
"thousands separator (',') not allowed with '",
presentation,
"' specifier");
valBufEnd = valBuf + valBufSize - 1;
valBufBegin =
valBuf + detail::uintToHexUpper(valBuf, valBufSize - 1, uval);
valBufEnd = valBuf + valBufSize;
valBufBegin = valBuf + detail::uintToHexUpper(valBuf, valBufSize, uval);
if (arg.basePrefix) {
*--valBufBegin = 'X';
*--valBufBegin = '0';
......@@ -609,9 +610,8 @@ class FormatValue<
"thousands separator (',') not allowed with '",
presentation,
"' specifier");
valBufEnd = valBuf + valBufSize - 1;
valBufBegin =
valBuf + detail::uintToBinary(valBuf, valBufSize - 1, uval);
valBufEnd = valBuf + valBufSize;
valBufBegin = valBuf + detail::uintToBinary(valBuf, valBufSize, uval);
if (arg.basePrefix) {
*--valBufBegin = presentation; // 0b or 0B
*--valBufBegin = '0';
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment