diff --git a/compiler-rt/lib/scudo/standalone/fuchsia.cpp b/compiler-rt/lib/scudo/standalone/fuchsia.cpp index 0788c4198e53c..2144f1b63f894 100644 --- a/compiler-rt/lib/scudo/standalone/fuchsia.cpp +++ b/compiler-rt/lib/scudo/standalone/fuchsia.cpp @@ -34,11 +34,10 @@ static_assert(ZX_HANDLE_INVALID == 0, ""); static void NORETURN dieOnError(zx_status_t Status, const char *FnName, uptr Size) { - char Error[128]; - formatString(Error, sizeof(Error), - "SCUDO ERROR: %s failed with size %zuKB (%s)", FnName, + ScopedString Error; + Error.append("SCUDO ERROR: %s failed with size %zuKB (%s)", FnName, Size >> 10, zx_status_get_string(Status)); - outputRaw(Error); + outputRaw(Error.data()); die(); } diff --git a/compiler-rt/lib/scudo/standalone/mem_map_fuchsia.cpp b/compiler-rt/lib/scudo/standalone/mem_map_fuchsia.cpp index 0566ab0655263..28e5a11a37f25 100644 --- a/compiler-rt/lib/scudo/standalone/mem_map_fuchsia.cpp +++ b/compiler-rt/lib/scudo/standalone/mem_map_fuchsia.cpp @@ -22,11 +22,10 @@ namespace scudo { static void NORETURN dieOnError(zx_status_t Status, const char *FnName, uptr Size) { - char Error[128]; - formatString(Error, sizeof(Error), - "SCUDO ERROR: %s failed with size %zuKB (%s)", FnName, + ScopedString Error; + Error.append("SCUDO ERROR: %s failed with size %zuKB (%s)", FnName, Size >> 10, _zx_status_get_string(Status)); - outputRaw(Error); + outputRaw(Error.data()); die(); } diff --git a/compiler-rt/lib/scudo/standalone/report_linux.cpp b/compiler-rt/lib/scudo/standalone/report_linux.cpp index 6a983036e6cd9..dfddef3324bd6 100644 --- a/compiler-rt/lib/scudo/standalone/report_linux.cpp +++ b/compiler-rt/lib/scudo/standalone/report_linux.cpp @@ -24,33 +24,30 @@ namespace scudo { // Fatal internal map() error (potentially OOM related). void NORETURN reportMapError(uptr SizeIfOOM) { - char Error[128] = "Scudo ERROR: internal map failure\n"; + ScopedString Error; + Error.append("Scudo ERROR: internal map failure"); if (SizeIfOOM) { - formatString( - Error, sizeof(Error), - "Scudo ERROR: internal map failure (NO MEMORY) requesting %zuKB\n", - SizeIfOOM >> 10); + Error.append(" (NO MEMORY) requesting %zuKB", SizeIfOOM >> 10); } - reportRawError(Error); + Error.append("\n"); + reportRawError(Error.data()); } void NORETURN reportUnmapError(uptr Addr, uptr Size) { - char Error[128]; - formatString(Error, sizeof(Error), - "Scudo ERROR: internal unmap failure (error desc=%s) Addr 0x%zx " + ScopedString Error; + Error.append("Scudo ERROR: internal unmap failure (error desc=%s) Addr 0x%zx " "Size %zu\n", strerror(errno), Addr, Size); - reportRawError(Error); + reportRawError(Error.data()); } void NORETURN reportProtectError(uptr Addr, uptr Size, int Prot) { - char Error[128]; - formatString( - Error, sizeof(Error), + ScopedString Error; + Error.append( "Scudo ERROR: internal protect failure (error desc=%s) Addr 0x%zx " "Size %zu Prot %x\n", strerror(errno), Addr, Size, Prot); - reportRawError(Error); + reportRawError(Error.data()); } } // namespace scudo diff --git a/compiler-rt/lib/scudo/standalone/string_utils.cpp b/compiler-rt/lib/scudo/standalone/string_utils.cpp index d4e4e3becd0e1..e584bd806e579 100644 --- a/compiler-rt/lib/scudo/standalone/string_utils.cpp +++ b/compiler-rt/lib/scudo/standalone/string_utils.cpp @@ -14,30 +14,21 @@ namespace scudo { -static int appendChar(char **Buffer, const char *BufferEnd, char C) { - if (*Buffer < BufferEnd) { - **Buffer = C; - (*Buffer)++; - } - return 1; -} - // Appends number in a given Base to buffer. If its length is less than // |MinNumberLength|, it is padded with leading zeroes or spaces, depending // on the value of |PadWithZero|. -static int appendNumber(char **Buffer, const char *BufferEnd, u64 AbsoluteValue, - u8 Base, u8 MinNumberLength, bool PadWithZero, - bool Negative, bool Upper) { +void ScopedString::appendNumber(u64 AbsoluteValue, u8 Base, u8 MinNumberLength, + bool PadWithZero, bool Negative, bool Upper) { constexpr uptr MaxLen = 30; RAW_CHECK(Base == 10 || Base == 16); RAW_CHECK(Base == 10 || !Negative); RAW_CHECK(AbsoluteValue || !Negative); RAW_CHECK(MinNumberLength < MaxLen); - int Res = 0; if (Negative && MinNumberLength) --MinNumberLength; - if (Negative && PadWithZero) - Res += appendChar(Buffer, BufferEnd, '-'); + if (Negative && PadWithZero) { + String.push_back('-'); + } uptr NumBuffer[MaxLen]; int Pos = 0; do { @@ -55,34 +46,32 @@ static int appendNumber(char **Buffer, const char *BufferEnd, u64 AbsoluteValue, Pos--; for (; Pos >= 0 && NumBuffer[Pos] == 0; Pos--) { char c = (PadWithZero || Pos == 0) ? '0' : ' '; - Res += appendChar(Buffer, BufferEnd, c); + String.push_back(c); } if (Negative && !PadWithZero) - Res += appendChar(Buffer, BufferEnd, '-'); + String.push_back('-'); for (; Pos >= 0; Pos--) { char Digit = static_cast(NumBuffer[Pos]); Digit = static_cast((Digit < 10) ? '0' + Digit : (Upper ? 'A' : 'a') + Digit - 10); - Res += appendChar(Buffer, BufferEnd, Digit); + String.push_back(Digit); } - return Res; } -static int appendUnsigned(char **Buffer, const char *BufferEnd, u64 Num, - u8 Base, u8 MinNumberLength, bool PadWithZero, - bool Upper) { - return appendNumber(Buffer, BufferEnd, Num, Base, MinNumberLength, - PadWithZero, /*Negative=*/false, Upper); +void ScopedString::appendUnsigned(u64 Num, u8 Base, u8 MinNumberLength, + bool PadWithZero, bool Upper) { + appendNumber(Num, Base, MinNumberLength, PadWithZero, /*Negative=*/false, + Upper); } -static int appendSignedDecimal(char **Buffer, const char *BufferEnd, s64 Num, - u8 MinNumberLength, bool PadWithZero) { +void ScopedString::appendSignedDecimal(s64 Num, u8 MinNumberLength, + bool PadWithZero) { const bool Negative = (Num < 0); const u64 UnsignedNum = (Num == INT64_MIN) ? static_cast(INT64_MAX) + 1 : static_cast(Negative ? -Num : Num); - return appendNumber(Buffer, BufferEnd, UnsignedNum, 10, MinNumberLength, - PadWithZero, Negative, /*Upper=*/false); + appendNumber(UnsignedNum, 10, MinNumberLength, PadWithZero, Negative, + /*Upper=*/false); } // Use the fact that explicitly requesting 0 Width (%0s) results in UB and @@ -90,44 +79,45 @@ static int appendSignedDecimal(char **Buffer, const char *BufferEnd, s64 Num, // Width == 0 - no Width requested // Width < 0 - left-justify S within and pad it to -Width chars, if necessary // Width > 0 - right-justify S, not implemented yet -static int appendString(char **Buffer, const char *BufferEnd, int Width, - int MaxChars, const char *S) { +void ScopedString::appendString(int Width, int MaxChars, const char *S) { if (!S) S = ""; - int Res = 0; + int NumChars = 0; for (; *S; S++) { - if (MaxChars >= 0 && Res >= MaxChars) + if (MaxChars >= 0 && NumChars >= MaxChars) break; - Res += appendChar(Buffer, BufferEnd, *S); + String.push_back(*S); + NumChars++; + } + if (Width < 0) { + // Only left justification supported. + Width = -Width - NumChars; + while (Width-- > 0) + String.push_back(' '); } - // Only the left justified strings are supported. - while (Width < -Res) - Res += appendChar(Buffer, BufferEnd, ' '); - return Res; } -static int appendPointer(char **Buffer, const char *BufferEnd, u64 ptr_value) { - int Res = 0; - Res += appendString(Buffer, BufferEnd, 0, -1, "0x"); - Res += appendUnsigned(Buffer, BufferEnd, ptr_value, 16, - SCUDO_POINTER_FORMAT_LENGTH, /*PadWithZero=*/true, - /*Upper=*/false); - return Res; +void ScopedString::appendPointer(u64 ptr_value) { + appendString(0, -1, "0x"); + appendUnsigned(ptr_value, 16, SCUDO_POINTER_FORMAT_LENGTH, + /*PadWithZero=*/true, + /*Upper=*/false); } -static int formatString(char *Buffer, uptr BufferLength, const char *Format, - va_list Args) { +void ScopedString::vappend(const char *Format, va_list &Args) { + // Since the string contains the '\0' terminator, put our size before it + // so that push_back calls work correctly. + DCHECK(String.size() > 0); + String.resize(String.size() - 1); + static const char *PrintfFormatsHelp = - "Supported formatString formats: %([0-9]*)?(z|ll)?{d,u,x,X}; %p; " + "Supported formats: %([0-9]*)?(z|ll)?{d,u,x,X}; %p; " "%[-]([0-9]*)?(\\.\\*)?s; %c\n"; RAW_CHECK(Format); - RAW_CHECK(BufferLength > 0); - const char *BufferEnd = &Buffer[BufferLength - 1]; const char *Cur = Format; - int Res = 0; for (; *Cur; Cur++) { if (*Cur != '%') { - Res += appendChar(&Buffer, BufferEnd, *Cur); + String.push_back(*Cur); continue; } Cur++; @@ -162,7 +152,7 @@ static int formatString(char *Buffer, uptr BufferLength, const char *Format, DVal = HaveLL ? va_arg(Args, s64) : HaveZ ? va_arg(Args, sptr) : va_arg(Args, int); - Res += appendSignedDecimal(&Buffer, BufferEnd, DVal, Width, PadWithZero); + appendSignedDecimal(DVal, Width, PadWithZero); break; } case 'u': @@ -172,27 +162,25 @@ static int formatString(char *Buffer, uptr BufferLength, const char *Format, : HaveZ ? va_arg(Args, uptr) : va_arg(Args, unsigned); const bool Upper = (*Cur == 'X'); - Res += appendUnsigned(&Buffer, BufferEnd, UVal, (*Cur == 'u') ? 10 : 16, - Width, PadWithZero, Upper); + appendUnsigned(UVal, (*Cur == 'u') ? 10 : 16, Width, PadWithZero, Upper); break; } case 'p': { RAW_CHECK_MSG(!HaveFlags, PrintfFormatsHelp); - Res += appendPointer(&Buffer, BufferEnd, va_arg(Args, uptr)); + appendPointer(va_arg(Args, uptr)); break; } case 's': { RAW_CHECK_MSG(!HaveLength, PrintfFormatsHelp); // Only left-justified Width is supported. CHECK(!HaveWidth || LeftJustified); - Res += appendString(&Buffer, BufferEnd, LeftJustified ? -Width : Width, - Precision, va_arg(Args, char *)); + appendString(LeftJustified ? -Width : Width, Precision, + va_arg(Args, char *)); break; } case 'c': { RAW_CHECK_MSG(!HaveFlags, PrintfFormatsHelp); - Res += - appendChar(&Buffer, BufferEnd, static_cast(va_arg(Args, int))); + String.push_back(static_cast(va_arg(Args, int))); break; } // In Scudo, `s64`/`u64` are supposed to use `lld` and `llu` respectively. @@ -207,19 +195,17 @@ static int formatString(char *Buffer, uptr BufferLength, const char *Format, if (*Cur == 'd') { DVal = va_arg(Args, s64); - Res += - appendSignedDecimal(&Buffer, BufferEnd, DVal, Width, PadWithZero); + appendSignedDecimal(DVal, Width, PadWithZero); } else { UVal = va_arg(Args, u64); - Res += appendUnsigned(&Buffer, BufferEnd, UVal, 10, Width, PadWithZero, - false); + appendUnsigned(UVal, 10, Width, PadWithZero, false); } break; } case '%': { RAW_CHECK_MSG(!HaveFlags, PrintfFormatsHelp); - Res += appendChar(&Buffer, BufferEnd, '%'); + String.push_back('%'); break; } default: { @@ -227,35 +213,13 @@ static int formatString(char *Buffer, uptr BufferLength, const char *Format, } } } - RAW_CHECK(Buffer <= BufferEnd); - appendChar(&Buffer, BufferEnd + 1, '\0'); - return Res; -} - -int formatString(char *Buffer, uptr BufferLength, const char *Format, ...) { - va_list Args; - va_start(Args, Format); - int Res = formatString(Buffer, BufferLength, Format, Args); - va_end(Args); - return Res; -} - -void ScopedString::vappend(const char *Format, va_list Args) { - va_list ArgsCopy; - va_copy(ArgsCopy, Args); - // formatString doesn't currently support a null buffer or zero buffer length, - // so in order to get the resulting formatted string length, we use a one-char - // buffer. - char C[1]; - const uptr AdditionalLength = - static_cast(formatString(C, sizeof(C), Format, Args)) + 1; - const uptr Length = length(); - String.resize(Length + AdditionalLength); - const uptr FormattedLength = static_cast(formatString( - String.data() + Length, String.size() - Length, Format, ArgsCopy)); - RAW_CHECK(data()[length()] == '\0'); - RAW_CHECK(FormattedLength + 1 == AdditionalLength); - va_end(ArgsCopy); + String.push_back('\0'); + if (String.back() != '\0') { + // String truncated, make sure the string is terminated properly. + // This can happen if there is no more memory when trying to resize + // the string. + String.back() = '\0'; + } } void ScopedString::append(const char *Format, ...) { diff --git a/compiler-rt/lib/scudo/standalone/string_utils.h b/compiler-rt/lib/scudo/standalone/string_utils.h index a4cab5268ede4..6e00b63779737 100644 --- a/compiler-rt/lib/scudo/standalone/string_utils.h +++ b/compiler-rt/lib/scudo/standalone/string_utils.h @@ -25,17 +25,24 @@ class ScopedString { String.clear(); String.push_back('\0'); } - void vappend(const char *Format, va_list Args); + void vappend(const char *Format, va_list &Args); void append(const char *Format, ...) FORMAT(2, 3); void output() const { outputRaw(String.data()); } void reserve(size_t Size) { String.reserve(Size + 1); } + uptr capacity() { return String.capacity() - 1; } private: + void appendNumber(u64 AbsoluteValue, u8 Base, u8 MinNumberLength, + bool PadWithZero, bool Negative, bool Upper); + void appendUnsigned(u64 Num, u8 Base, u8 MinNumberLength, bool PadWithZero, + bool Upper); + void appendSignedDecimal(s64 Num, u8 MinNumberLength, bool PadWithZero); + void appendString(int Width, int MaxChars, const char *S); + void appendPointer(u64 ptr_value); + Vector String; }; -int formatString(char *Buffer, uptr BufferLength, const char *Format, ...) - FORMAT(3, 4); void Printf(const char *Format, ...) FORMAT(1, 2); } // namespace scudo diff --git a/compiler-rt/lib/scudo/standalone/tests/strings_test.cpp b/compiler-rt/lib/scudo/standalone/tests/strings_test.cpp index 7a69ffd9762c5..e068c48fc97c9 100644 --- a/compiler-rt/lib/scudo/standalone/tests/strings_test.cpp +++ b/compiler-rt/lib/scudo/standalone/tests/strings_test.cpp @@ -66,6 +66,10 @@ TEST(ScudoStringsTest, Precision) { Str.append("%-6s", "12345"); EXPECT_EQ(Str.length(), strlen(Str.data())); EXPECT_STREQ("12345 ", Str.data()); + Str.clear(); + Str.append("%-8s", "12345"); + EXPECT_EQ(Str.length(), strlen(Str.data())); + EXPECT_STREQ("12345 ", Str.data()); } static void fillString(scudo::ScopedString &Str, scudo::uptr Size) { @@ -123,3 +127,31 @@ TEST(ScudoStringsTest, Padding) { testAgainstLibc("%03d - %03d", 12, 1234); testAgainstLibc("%03d - %03d", -12, -1234); } + +#if defined(__linux__) +#include + +TEST(ScudoStringsTest, CapacityIncreaseFails) { + scudo::ScopedString Str; + + rlimit Limit = {}; + EXPECT_EQ(0, getrlimit(RLIMIT_AS, &Limit)); + rlimit EmptyLimit = {.rlim_max = Limit.rlim_max}; + EXPECT_EQ(0, setrlimit(RLIMIT_AS, &EmptyLimit)); + + // Test requires that the default length is at least 6 characters. + scudo::uptr MaxSize = Str.capacity(); + EXPECT_LE(6, MaxSize); + + for (size_t i = 0; i < MaxSize - 5; i++) { + Str.append("B"); + } + + // Attempt to append past the end of the current capacity. + Str.append("%d", 12345678); + EXPECT_EQ(MaxSize, Str.capacity()); + EXPECT_STREQ("B12345", &Str.data()[MaxSize - 6]); + + EXPECT_EQ(0, setrlimit(RLIMIT_AS, &Limit)); +} +#endif diff --git a/compiler-rt/lib/scudo/standalone/tests/vector_test.cpp b/compiler-rt/lib/scudo/standalone/tests/vector_test.cpp index dc23c2a347131..b7678678d8a29 100644 --- a/compiler-rt/lib/scudo/standalone/tests/vector_test.cpp +++ b/compiler-rt/lib/scudo/standalone/tests/vector_test.cpp @@ -41,3 +41,38 @@ TEST(ScudoVectorTest, ResizeReduction) { V.resize(1); EXPECT_EQ(V.size(), 1U); } + +#if defined(__linux__) + +#include + +// Verify that if the reallocate fails, nothing new is added. +TEST(ScudoVectorTest, ReallocateFails) { + scudo::Vector V; + scudo::uptr capacity = V.capacity(); + + // Get the current address space size. + rlimit Limit = {}; + EXPECT_EQ(0, getrlimit(RLIMIT_AS, &Limit)); + + rlimit EmptyLimit = {.rlim_max = Limit.rlim_max}; + EXPECT_EQ(0, setrlimit(RLIMIT_AS, &EmptyLimit)); + + V.resize(capacity); + // Set the last element so we can check it later. + V.back() = '\0'; + + // The reallocate should fail, so the capacity should not change. + V.reserve(capacity + 1000); + EXPECT_EQ(capacity, V.capacity()); + + // Now try to do a push back and verify that the size does not change. + scudo::uptr Size = V.size(); + V.push_back('2'); + EXPECT_EQ(Size, V.size()); + // Verify that the last element in the vector did not change. + EXPECT_EQ('\0', V.back()); + + EXPECT_EQ(0, setrlimit(RLIMIT_AS, &Limit)); +} +#endif diff --git a/compiler-rt/lib/scudo/standalone/vector.h b/compiler-rt/lib/scudo/standalone/vector.h index c0f1ba0eddfa9..ca10cc281d770 100644 --- a/compiler-rt/lib/scudo/standalone/vector.h +++ b/compiler-rt/lib/scudo/standalone/vector.h @@ -35,7 +35,9 @@ template class VectorNoCtor { DCHECK_LE(Size, capacity()); if (Size == capacity()) { const uptr NewCapacity = roundUpPowerOfTwo(Size + 1); - reallocate(NewCapacity); + if (!reallocate(NewCapacity)) { + return; + } } memcpy(&Data[Size++], &Element, sizeof(T)); } @@ -51,14 +53,17 @@ template class VectorNoCtor { const T *data() const { return Data; } T *data() { return Data; } constexpr uptr capacity() const { return CapacityBytes / sizeof(T); } - void reserve(uptr NewSize) { + bool reserve(uptr NewSize) { // Never downsize internal buffer. if (NewSize > capacity()) - reallocate(NewSize); + return reallocate(NewSize); + return true; } void resize(uptr NewSize) { if (NewSize > Size) { - reserve(NewSize); + if (!reserve(NewSize)) { + return; + } memset(&Data[Size], 0, sizeof(T) * (NewSize - Size)); } Size = NewSize; @@ -86,13 +91,16 @@ template class VectorNoCtor { } private: - void reallocate(uptr NewCapacity) { + bool reallocate(uptr NewCapacity) { DCHECK_GT(NewCapacity, 0); DCHECK_LE(Size, NewCapacity); MemMapT NewExternalBuffer; NewCapacity = roundUp(NewCapacity * sizeof(T), getPageSizeCached()); - NewExternalBuffer.map(/*Addr=*/0U, NewCapacity, "scudo:vector"); + if (!NewExternalBuffer.map(/*Addr=*/0U, NewCapacity, "scudo:vector", + MAP_ALLOWNOMEM)) { + return false; + } T *NewExternalData = reinterpret_cast(NewExternalBuffer.getBase()); memcpy(NewExternalData, Data, Size * sizeof(T)); @@ -101,6 +109,7 @@ template class VectorNoCtor { Data = NewExternalData; CapacityBytes = NewCapacity; ExternalBuffer = NewExternalBuffer; + return true; } T *Data = nullptr;