[scudo][standalone] Add string utility functions

[cryptoad]

Summary

Add some string utility functions, notably to format strings, get
lengths, convert a string to a number. Those functions will be
used in reports and flags (coming up next). They were mostly borrowed
from sanitizer_common.

Make use of the string length function in a couple places in the
platform code that was checked in with inlined version of it.

Add some tests.

[hctim]

lib/scudo/standalone/string_utils.cc

34

Why unsigned? Shouldn't this just be char?

57

Personally, input params before output params, EndPtr should be after Base.

64

const char *OldNPtr = NPtr

66

Sign is initialized to 1 in declaration, can remove this.

74

Why uptr? char or int should be sufficient. Also const not needed here.

79

Can remove brackets

80

*EndPtr = (HaveDigits) ? NPtr : OldNPtr;

82

Can remove brackets. Same with the else of this clause.

83

Is return Min(static_cast<u64>(INT64_MAX), Res); sufficient here?

97

Nit: Capitalize appends at start of sentence.

lib/scudo/standalone/string_utils.h

10

SCUDO_STRING_UTILS_H_

lib/scudo/standalone/tests/strings_test.cc

80

Could you add some UINT_MAX and INT_MAX tests here?

Also "+0", "-0", "-", "+", however I'm not sure how likely these are to be used in the reports/flags.

[vitalybuka]

lib/scudo/standalone/string_utils.cc

25

do you need internal implementation of these functions? Why can't you use libc?
I would expect that scudo has no the same restrictions as sanitizer_common

lib/scudo/standalone/string_utils.h

25

Could you please switch this to aggregation instead of inheritance?

[cryptoad]

Addressing Mitch's comments.

[cryptoad]

lib/scudo/standalone/string_utils.cc

57

This one was mostly to mirror the behavior & parameters of https://linux.die.net/man/3/strtol
I am open to changing it if it makes sense to everyone.

74

Changed to int. char errors with -Werror=conversion.
As for const, I'd rather keep for consistency.

[cryptoad]

lib/scudo/standalone/string_utils.cc

25

With those, I am slightly worried about localization in the libc, and how strings might end up being processed, with possibly at some point heap allocations.
Looking around it looks like strlen/strnlen/strcmp/strncmp are generally safe (glibc/bionic) but it's not an exhaustive assessment.
strtol appears to be far more complicated.
You likely know better about those intricacies, let me know what you think!

[cryptoad]

Changing ScopedString to aggregation instead of inheritance.

[vitalybuka]

lib/scudo/standalone/string_utils.cc

25

I would go with as little as possible of custom code.
Add them when you hit an issue?

[cryptoad]

Removing the custom strlen & strncmp in favor of the libc ones.

[hctim]

lib/scudo/standalone/string_utils.cc

57

Agreed, probably best to leave as-is then.

[hctim]

lib/scudo/standalone/string_utils.cc

25

Similar to this, is there any particular reason why we're not using the current sanitizer printf commons and basically copying the code? Are we trying to avoid bringing compiler-rt along with us wherever scuda goes? If this is the case, it doesn't look like the printf commons have much that we're not using, and doesn't have any large dependencies on other things in compiler-rt, could we use the commons instead?

[cryptoad]

lib/scudo/standalone/string_utils.cc

25

The reason for the rewrite as a standalone component is to not depend on sanitizer_common anymore.
One of the reasons being that Fuchsia wants Scudo in their libc, and we can't afford to pull in extraneous code.
Some benefits is that Scudo doesn't have the same requirements as a lot of sanitizer (wrt interception for example) and so we can use some standard function that they can't (like strlen as Vitaly pointed out).

[morehouse]

lib/scudo/standalone/string_utils.cc

18

Can we use isspace from libc?

23

Can we use isdigit from libc?

25

Can we use tolower from libc?

29

strtoll should be safe here. I can't think of a good reason for it to malloc anything.

lib/scudo/standalone/string_utils.h

33

Why do we need this append with va_list?

lib/scudo/standalone/tests/strings_test.cc

14

Can we add a test that the ScopedString buffer won't be overflowed by append?

[cryptoad]

Following Matt's comments, I had a look at various libc implementations
and the ctype function isspace, isdigit, tolower all appear to be
safe, so use them instead of our own.
strtol & co. should also do (Windows might be using some multibyte to
widechar conversion function which would use dynamic allocation, but we
don't support Windows so we should be good).
Add a test that attempts to overflow a ScopedString with different
variations of append.

[cryptoad]

Correct some spacing.

[morehouse]

Still want to know if we need append with a va_list.

[cryptoad]

lib/scudo/standalone/string_utils.h

33

Printf for example uses a va_list for append. The variadic version is a wrapper to this one as well.

[morehouse]

lib/scudo/standalone/string_utils.h

33

It looks like we only use void append(const char *Format, ...).

Are there cases where void append(const char *Format, va_list Args) is used?

[morehouse]

lib/scudo/standalone/string_utils.h

33

Never mind, I see what you were saying about Printf now.

[vitalybuka]

lib/scudo/standalone/string_utils.cc

130

I expect that returning the position where it stops is more convenient.
E.g.

static int appendChar(char *Buffer, const char *BufferEnd, char C) {
  if (Buffer < BufferEnd)
    *Buffer = C;
  return Buffer  + 1;
}

remove Res and return (Buffer - BufferIn)

But up to you if you prefer the current approach.

lib/scudo/standalone/string_utils.h

34

If you use LLVM style, members go before methods

[cryptoad]

lib/scudo/standalone/string_utils.h

34

Ack. I will revisit that later on.
I'd like to land what is there for now, as Phabricator is quickly making this unreadable, if that's ok with you.

[vitalybuka]

lib/scudo/standalone/string_utils.h

34

lgtm

lib/scudo/standalone/CMakeLists.txt

# android-changes-for-ndk-developers.md#changes-to-library-search-order
   append_list_if(COMPILER_RT_HAS_Z_GLOBAL -Wl,-z,global SCUDO_LINK_FLAGS)
 endif()
 
 set(SCUDO_SOURCES
   checksum.cc
   crc32_hw.cc
   common.cc
   fuchsia.cc
-  linux.cc)
+  linux.cc
+  string_utils.cc)
 
 # Enable the SSE 4.2 instruction set for crc32_hw.cc, if available.
 if (COMPILER_RT_HAS_MSSE4_2_FLAG)
   set_source_files_properties(crc32_hw.cc PROPERTIES COMPILE_FLAGS -msse4.2)
 endif()
 
 # Enable the AArch64 CRC32 feature for crc32_hw.cc, if available.
 # Note that it is enabled by default starting with armv8.1-a.
 if (COMPILER_RT_HAS_MCRC_FLAG)
   set_source_files_properties(crc32_hw.cc PROPERTIES COMPILE_FLAGS -mcrc)
 endif()
 
 set(SCUDO_HEADERS
   atomic_helpers.h
   bytemap.h
   checksum.h
   internal_defs.h
   linux.h
   list.h
   mutex.h
   platform.h
   stats.h
+  string_utils.h
   vector.h)
 
 if(COMPILER_RT_HAS_SCUDO_STANDALONE)
   add_compiler_rt_object_libraries(RTScudoStandalone
     ARCHS ${SCUDO_STANDALONE_SUPPORTED_ARCH}
     SOURCES ${SCUDO_SOURCES}
     ADDITIONAL_HEADERS ${SCUDO_HEADERS}
     CFLAGS ${SCUDO_CFLAGS})

lib/scudo/standalone/fuchsia.cc

 //===-- fuchsia.cc ----------------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "platform.h"
 
 #if SCUDO_FUCHSIA
 
 #include "common.h"
 #include "mutex.h"
+#include "string_utils.h"
 
 #include <limits.h> // for PAGE_SIZE
 #include <stdlib.h> // for abort()
 #include <zircon/process.h>
 #include <zircon/sanitizer.h>
 #include <zircon/syscalls.h>
 
 namespace scudo {
void *map(void *Addr, uptr Size, const char *Name, uptr Flags, u64 *Extra) {
   } else {
     // Otherwise, create a Vmo and set its name.
     Status = _zx_vmo_create(Size, 0, &Vmo);
     if (Status != ZX_OK) {
       if (Status != ZX_ERR_NO_MEMORY || !AllowNoMem)
         dieOnMapUnmapError(Status == ZX_ERR_NO_MEMORY);
       return nullptr;
     }
-    uptr N = 0;
-    while (Name[N])
-      N++;
-    _zx_object_set_property(Vmo, ZX_PROP_NAME, Name, N);
+    _zx_object_set_property(Vmo, ZX_PROP_NAME, Name, strlen(Name));
   }
 
   uintptr_t P;
   zx_vm_option_t MapFlags = ZX_VM_PERM_READ | ZX_VM_PERM_WRITE;
   const uint64_t Offset = Addr ? getOffsetInVmar(Vmar, Addr, Size) : 0;
   if (Offset)
     MapFlags |= ZX_VM_SPECIFIC;
   Status = _zx_vmar_map(Vmar, MapFlags, Offset, Vmo, VmoSize, Size, &P);
bool getRandom(void *Buffer, uptr Length, bool Blocking) {
   COMPILER_CHECK(MaxRandomLength <= ZX_CPRNG_DRAW_MAX_LEN);
   if (!Buffer || !Length || Length > MaxRandomLength)
     return false;
   _zx_cprng_draw(Buffer, Length);
   return true;
 }
 
 void outputRaw(const char *Buffer) {
-  uptr N = 0;
-  while (Buffer[N])
-    N++;
-  __sanitizer_log_write(Buffer, N);
+  __sanitizer_log_write(Buffer, strlen(Buffer));
 }
 
 void setAbortMessage(const char *Message) {}
 
 } // namespace scudo
 
 #endif // SCUDO_FUCHSIA

lib/scudo/standalone/linux.cc

 //===-- linux.cc ------------------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "platform.h"
 
 #if SCUDO_LINUX
 
 #include "common.h"
 #include "linux.h"
 #include "mutex.h"
+#include "string_utils.h"
 
 #include <errno.h>
 #include <fcntl.h>
 #include <linux/futex.h>
 #include <sched.h>
 #include <stdlib.h>
 #include <string.h>
 #include <sys/mman.h>
#endif // defined(SYS_getrandom)
   ReadBytes = read(FileDesc, Buffer, Length);
   close(FileDesc);
   return (ReadBytes == static_cast<ssize_t>(Length));
 }
 
 void outputRaw(const char *Buffer) {
   static StaticSpinMutex Mutex;
   SpinMutexLock L(&Mutex);
-  uptr N = 0;
-  while (Buffer[N])
-    N++;
-  write(2, Buffer, N);
+  write(2, Buffer, strlen(Buffer));
 }
 
 extern "C" WEAK void android_set_abort_message(const char *);
 
 void setAbortMessage(const char *Message) {
   if (&android_set_abort_message)
     android_set_abort_message(Message);
 }
 
 } // namespace scudo
 
 #endif // SCUDO_LINUX

lib/scudo/standalone/string_utils.h

+//===-- string_utils.h ------------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef SCUDO_STRING_UTILS_H_
+#define SCUDO_STRING_UTILS_H_
+
+#include "internal_defs.h"
+#include "vector.h"
+
+#include <stdarg.h>
+
+namespace scudo {
+
+class ScopedString {
+public:
+  explicit ScopedString(uptr MaxLength) : String(MaxLength), Length(0) {
+    String[0] = '\0';
+  }
+  uptr length() { return Length; }
+  const char *data() { return String.data(); }
+  void clear() {
+    String[0] = '\0';
+    Length = 0;
+  }
+  void append(const char *Format, va_list Args);
+  void append(const char *Format, ...);
+
+private:
+  Vector<char> String;
+  uptr Length;
+};
+
+void Printf(const char *Format, ...);
+
+} // namespace scudo
+
+#endif // SCUDO_STRING_UTILS_H_

lib/scudo/standalone/string_utils.cc

+//===-- string_utils.cc -----------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "string_utils.h"
+#include "common.h"
+
+#include <ctype.h>
+#include <stdarg.h>
+#include <string.h>
+
+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) {
+  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, '-');
+  uptr NumBuffer[MaxLen];
+  int Pos = 0;
+  do {
+    RAW_CHECK_MSG(static_cast<uptr>(Pos) < MaxLen,
+                  "appendNumber buffer overflow");
+    NumBuffer[Pos++] = AbsoluteValue % Base;
+    AbsoluteValue /= Base;
+  } while (AbsoluteValue > 0);
+  if (Pos < MinNumberLength) {
+    memset(&NumBuffer[Pos], 0,
+           sizeof(NumBuffer[0]) * static_cast<uptr>(MinNumberLength - Pos));
+    Pos = MinNumberLength;
+  }
+  RAW_CHECK(Pos > 0);
+  Pos--;
+  for (; Pos >= 0 && NumBuffer[Pos] == 0; Pos--) {
+    char c = (PadWithZero || Pos == 0) ? '0' : ' ';
+    Res += appendChar(Buffer, BufferEnd, c);
+  }
+  if (Negative && !PadWithZero)
+    Res += appendChar(Buffer, BufferEnd, '-');
+  for (; Pos >= 0; Pos--) {
+    char Digit = static_cast<char>(NumBuffer[Pos]);
+    Digit = static_cast<char>((Digit < 10) ? '0' + Digit
+                                           : (Upper ? 'A' : 'a') + Digit - 10);
+    Res += appendChar(Buffer, BufferEnd, 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);
+}
+
+static int appendSignedDecimal(char **Buffer, const char *BufferEnd, s64 Num,
+                               u8 MinNumberLength, bool PadWithZero) {
+  const bool Negative = (Num < 0);
+  return appendNumber(Buffer, BufferEnd,
+                      static_cast<u64>(Negative ? -Num : Num), 10,
+                      MinNumberLength, PadWithZero, Negative,
+                      /*Upper=*/false);
+}
+
+// Use the fact that explicitly requesting 0 Width (%0s) results in UB and
+// interpret Width == 0 as "no Width requested":
+// 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) {
+  if (!S)
+    S = "<null>";
+  int Res = 0;
+  for (; *S; S++) {
+    if (MaxChars >= 0 && Res >= MaxChars)
+      break;
+    Res += appendChar(Buffer, BufferEnd, *S);
+  }
+  // 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;
+}
+
+int formatString(char *Buffer, uptr BufferLength, const char *Format,
+                 va_list Args) {
+  UNUSED static const char *PrintfFormatsHelp =
+      "Supported formatString 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);
+      continue;
+    }
+    Cur++;
+    const bool LeftJustified = *Cur == '-';
+    if (LeftJustified)
+      Cur++;
+    bool HaveWidth = (*Cur >= '0' && *Cur <= '9');
+    const bool PadWithZero = (*Cur == '0');
+    u8 Width = 0;
+    if (HaveWidth) {
+      while (*Cur >= '0' && *Cur <= '9')
+        Width = static_cast<u8>(Width * 10 + *Cur++ - '0');
+    }
+    const bool HavePrecision = (Cur[0] == '.' && Cur[1] == '*');
+    int Precision = -1;
+    if (HavePrecision) {
+      Cur += 2;
+      Precision = va_arg(Args, int);
+    }
+    const bool HaveZ = (*Cur == 'z');
+    Cur += HaveZ;
+    const bool HaveLL = !HaveZ && (Cur[0] == 'l' && Cur[1] == 'l');
+    Cur += HaveLL * 2;
+    s64 DVal;
+    u64 UVal;
+    const bool HaveLength = HaveZ || HaveLL;
+    const bool HaveFlags = HaveWidth || HaveLength;
+    // At the moment only %s supports precision and left-justification.
+    CHECK(!((Precision >= 0 || LeftJustified) && *Cur != 's'));
+    switch (*Cur) {
+    case 'd': {
+      DVal = HaveLL ? va_arg(Args, s64)
+                    : HaveZ ? va_arg(Args, sptr) : va_arg(Args, int);
+      Res += appendSignedDecimal(&Buffer, BufferEnd, DVal, Width, PadWithZero);
+      break;
+    }
+    case 'u':
+    case 'x':
+    case 'X': {
+      UVal = HaveLL ? va_arg(Args, u64)
+                    : 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);
+      break;
+    }
+    case 'p': {
+      RAW_CHECK_MSG(!HaveFlags, PrintfFormatsHelp);
+      Res += appendPointer(&Buffer, BufferEnd, 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 *));
+      break;
+    }
+    case 'c': {
+      RAW_CHECK_MSG(!HaveFlags, PrintfFormatsHelp);
+      Res +=
+          appendChar(&Buffer, BufferEnd, static_cast<char>(va_arg(Args, int)));
+      break;
+    }
+    case '%': {
+      RAW_CHECK_MSG(!HaveFlags, PrintfFormatsHelp);
+      Res += appendChar(&Buffer, BufferEnd, '%');
+      break;
+    }
+    default: {
+      RAW_CHECK_MSG(false, PrintfFormatsHelp);
+    }
+    }
+  }
+  RAW_CHECK(Buffer <= BufferEnd);
+  appendChar(&Buffer, BufferEnd + 1, '\0');
+  return Res;
+}
+
+void ScopedString::append(const char *Format, va_list Args) {
+  CHECK_LT(Length, String.size());
+  formatString(String.data() + Length, String.size() - Length, Format, Args);
+  Length += strlen(String.data() + Length);
+  CHECK_LT(Length, String.size());
+}
+
+FORMAT(2, 3)
+void ScopedString::append(const char *Format, ...) {
+  va_list Args;
+  va_start(Args, Format);
+  append(Format, Args);
+  va_end(Args);
+}
+
+FORMAT(1, 2)
+void Printf(const char *Format, ...) {
+  va_list Args;
+  va_start(Args, Format);
+  ScopedString Msg(512);
+  Msg.append(Format, Args);
+  outputRaw(Msg.data());
+  va_end(Args);
+}
+
+} // namespace scudo

lib/scudo/standalone/tests/CMakeLists.txt

 set(SCUDO_UNIT_TEST_SOURCES
   atomic_test.cc
   bytemap_test.cc
   checksum_test.cc
   list_test.cc
   map_test.cc
   mutex_test.cc
   stats_test.cc
+  strings_test.cc
   vector_test.cc
   scudo_unit_test_main.cc)
 
 add_scudo_unittest(ScudoUnitTest
   SOURCES ${SCUDO_UNIT_TEST_SOURCES})

lib/scudo/standalone/tests/strings_test.cc

+//===-- strings_test.cc -----------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "scudo/standalone/string_utils.h"
+#include "gtest/gtest.h"
+
+#include <limits.h>
+
+TEST(ScudoStringsTest, Basic) {
+  scudo::ScopedString Str(128);
+  Str.append("a%db%zdc%ue%zuf%xh%zxq%pe%sr", static_cast<int>(-1),
+             static_cast<scudo::uptr>(-2), static_cast<unsigned>(-4),
+             static_cast<scudo::uptr>(5), static_cast<unsigned>(10),
+             static_cast<scudo::uptr>(11), reinterpret_cast<void *>(0x123),
+             "_string_");
+  EXPECT_EQ(Str.length(), strlen(Str.data()));
+
+  std::string expectedString = "a-1b-2c4294967292e5fahbq0x";
+  expectedString += std::string(SCUDO_POINTER_FORMAT_LENGTH - 3, '0');
+  expectedString += "123e_string_r";
+  EXPECT_EQ(Str.length(), strlen(Str.data()));
+  EXPECT_STREQ(expectedString.c_str(), Str.data());
+}
+
+TEST(ScudoStringsTest, Precision) {
+  scudo::ScopedString Str(128);
+  Str.append("%.*s", 3, "12345");
+  EXPECT_EQ(Str.length(), strlen(Str.data()));
+  EXPECT_STREQ("123", Str.data());
+  Str.clear();
+  Str.append("%.*s", 6, "12345");
+  EXPECT_EQ(Str.length(), strlen(Str.data()));
+  EXPECT_STREQ("12345", Str.data());
+  Str.clear();
+  Str.append("%-6s", "12345");
+  EXPECT_EQ(Str.length(), strlen(Str.data()));
+  EXPECT_STREQ("12345 ", Str.data());
+}
+
+static void fillString(scudo::ScopedString &Str, scudo::uptr Size) {
+  for (scudo::uptr I = 0; I < Size; I++)
+    Str.append("A");
+}
+
+TEST(ScudoStringTest, PotentialOverflows) {
+  // Use a ScopedString that spans a page, and attempt to write past the end
+  // of it with variations of append. The expectation is for nothing to crash.
+  const scudo::uptr PageSize = scudo::getPageSizeCached();
+  scudo::ScopedString Str(PageSize);
+  Str.clear();
+  fillString(Str, 2 * PageSize);
+  Str.clear();
+  fillString(Str, PageSize - 64);
+  Str.append("%-128s", "12345");
+  Str.clear();
+  fillString(Str, PageSize - 16);
+  Str.append("%024x", 12345);
+  Str.clear();
+  fillString(Str, PageSize - 16);
+  Str.append("EEEEEEEEEEEEEEEEEEEEEEEE");
+}
+
+template <typename T>
+static void testAgainstLibc(const char *Format, T Arg1, T Arg2) {
+  scudo::ScopedString Str(128);
+  Str.append(Format, Arg1, Arg2);
+  char Buffer[128];
+  snprintf(Buffer, sizeof(Buffer), Format, Arg1, Arg2);
+  EXPECT_EQ(Str.length(), strlen(Str.data()));
+  EXPECT_STREQ(Buffer, Str.data());
+}
+
+TEST(ScudoStringsTest, MinMax) {
+  testAgainstLibc<int>("%d-%d", INT_MIN, INT_MAX);
+  testAgainstLibc<unsigned>("%u-%u", 0, UINT_MAX);
+  testAgainstLibc<unsigned>("%x-%x", 0, UINT_MAX);
+  testAgainstLibc<long>("%zd-%zd", LONG_MIN, LONG_MAX);
+  testAgainstLibc<unsigned long>("%zu-%zu", 0, ULONG_MAX);
+  testAgainstLibc<unsigned long>("%zx-%zx", 0, ULONG_MAX);
+}
+
+TEST(ScudoStringsTest, Padding) {
+  testAgainstLibc<int>("%3d - %3d", 1, 0);
+  testAgainstLibc<int>("%3d - %3d", -1, 123);
+  testAgainstLibc<int>("%3d - %3d", -1, -123);
+  testAgainstLibc<int>("%3d - %3d", 12, 1234);
+  testAgainstLibc<int>("%3d - %3d", -12, -1234);
+  testAgainstLibc<int>("%03d - %03d", 1, 0);
+  testAgainstLibc<int>("%03d - %03d", -1, 123);
+  testAgainstLibc<int>("%03d - %03d", -1, -123);
+  testAgainstLibc<int>("%03d - %03d", 12, 1234);
+  testAgainstLibc<int>("%03d - %03d", -12, -1234);
+}