[scudo] Rework Vector/String
Some platforms (eg: Trusty) are extremelly memory constrained, which
doesn't necessarily work well with some of Scudo's current assumptions.
`Vector` by default (and as such `String` and `ScopedString`) maps a
page, which is a bit of a waste. This CL changes `Vector` to use a
buffer local to the class first, then potentially map more memory if
needed (`ScopedString` currently are all stack based so it would be
stack data). We also want to allow a platform to prevent any dynamic
resizing, so I added a `CanGrow` templated parameter that for now is
always `true` but would be set to `false` on Trusty.
Differential Revision: https://reviews.llvm.org/D103641
GitOrigin-RevId: 868317b3fd765830c07ecf16cbfcf6c7708adc3c
Change-Id: Ia16abe9b3f18c6a719f1c202a943b1381ea5ecbb
diff --git a/standalone/combined.h b/standalone/combined.h
index 8caffcc..f58eaa9 100644
--- a/standalone/combined.h
+++ b/standalone/combined.h
@@ -694,7 +694,7 @@
// function. This can be called with a null buffer or zero size for buffer
// sizing purposes.
uptr getStats(char *Buffer, uptr Size) {
- ScopedString Str(1024);
+ ScopedString Str;
disable();
const uptr Length = getStats(&Str) + 1;
enable();
@@ -708,7 +708,7 @@
}
void printStats() {
- ScopedString Str(1024);
+ ScopedString Str;
disable();
getStats(&Str);
enable();
diff --git a/standalone/primary64.h b/standalone/primary64.h
index 0a43f46..a3456b6 100644
--- a/standalone/primary64.h
+++ b/standalone/primary64.h
@@ -341,7 +341,7 @@
if (UNLIKELY(RegionBase + MappedUser + MapSize > RegionSize)) {
if (!Region->Exhausted) {
Region->Exhausted = true;
- ScopedString Str(1024);
+ ScopedString Str;
getStats(&Str);
Str.append(
"Scudo OOM: The process has exhausted %zuM for size class %zu.\n",
diff --git a/standalone/report.cpp b/standalone/report.cpp
index 292c299..561c7c5 100644
--- a/standalone/report.cpp
+++ b/standalone/report.cpp
@@ -17,7 +17,7 @@
class ScopedErrorReport {
public:
- ScopedErrorReport() : Message(512) { Message.append("Scudo ERROR: "); }
+ ScopedErrorReport() : Message() { Message.append("Scudo ERROR: "); }
void append(const char *Format, ...) {
va_list Args;
va_start(Args, Format);
diff --git a/standalone/size_class_map.h b/standalone/size_class_map.h
index 2736e99..e1c2eda 100644
--- a/standalone/size_class_map.h
+++ b/standalone/size_class_map.h
@@ -309,7 +309,7 @@
typedef FixedSizeClassMap<SvelteSizeClassConfig> SvelteSizeClassMap;
template <typename SCMap> inline void printMap() {
- ScopedString Buffer(1024);
+ ScopedString Buffer;
uptr PrevS = 0;
uptr TotalCached = 0;
for (uptr I = 0; I < SCMap::NumClasses; I++) {
diff --git a/standalone/string_utils.cpp b/standalone/string_utils.cpp
index 25bddbc..ca80834 100644
--- a/standalone/string_utils.cpp
+++ b/standalone/string_utils.cpp
@@ -219,7 +219,7 @@
}
void ScopedString::append(const char *Format, va_list Args) {
- DCHECK_LT(Length, String.size());
+ RAW_CHECK(Length <= String.size());
va_list ArgsCopy;
va_copy(ArgsCopy, Args);
// formatString doesn't currently support a null buffer or zero buffer length,
@@ -232,7 +232,7 @@
formatString(String.data() + Length, AdditionalLength, Format, ArgsCopy);
va_end(ArgsCopy);
Length = strlen(String.data());
- CHECK_LT(Length, String.size());
+ RAW_CHECK(Length < String.size());
}
FORMAT(2, 3)
@@ -247,7 +247,7 @@
void Printf(const char *Format, ...) {
va_list Args;
va_start(Args, Format);
- ScopedString Msg(1024);
+ ScopedString Msg;
Msg.append(Format, Args);
outputRaw(Msg.data());
va_end(Args);
diff --git a/standalone/string_utils.h b/standalone/string_utils.h
index 4880fa1..7a57da3 100644
--- a/standalone/string_utils.h
+++ b/standalone/string_utils.h
@@ -18,8 +18,9 @@
class ScopedString {
public:
- explicit ScopedString(uptr MaxLength) : String(MaxLength), Length(0) {
- String[0] = '\0';
+ explicit ScopedString() : String() {
+ if (String.capacity() > 0)
+ String[0] = '\0';
}
uptr length() { return Length; }
const char *data() { return String.data(); }
@@ -33,7 +34,7 @@
private:
Vector<char> String;
- uptr Length;
+ uptr Length = 0;
};
int formatString(char *Buffer, uptr BufferLength, const char *Format, ...);
diff --git a/standalone/tests/primary_test.cpp b/standalone/tests/primary_test.cpp
index 3574c28..93157f9 100644
--- a/standalone/tests/primary_test.cpp
+++ b/standalone/tests/primary_test.cpp
@@ -132,7 +132,7 @@
}
Cache.destroy(nullptr);
Allocator->releaseToOS();
- scudo::ScopedString Str(1024);
+ scudo::ScopedString Str;
Allocator->getStats(&Str);
Str.output();
}
@@ -178,7 +178,7 @@
}
Cache.destroy(nullptr);
Allocator.releaseToOS();
- scudo::ScopedString Str(1024);
+ scudo::ScopedString Str;
Allocator.getStats(&Str);
Str.output();
EXPECT_EQ(AllocationFailed, true);
@@ -216,7 +216,7 @@
}
Cache.destroy(nullptr);
Allocator->releaseToOS();
- scudo::ScopedString Str(1024);
+ scudo::ScopedString Str;
Allocator->getStats(&Str);
Str.output();
}
@@ -263,7 +263,7 @@
for (auto &T : Threads)
T.join();
Allocator->releaseToOS();
- scudo::ScopedString Str(1024);
+ scudo::ScopedString Str;
Allocator->getStats(&Str);
Str.output();
}
diff --git a/standalone/tests/quarantine_test.cpp b/standalone/tests/quarantine_test.cpp
index 91de56a..972c98d 100644
--- a/standalone/tests/quarantine_test.cpp
+++ b/standalone/tests/quarantine_test.cpp
@@ -214,7 +214,7 @@
Quarantine.drainAndRecycle(&Cache, Cb);
EXPECT_EQ(Cache.getSize(), 0UL);
- scudo::ScopedString Str(1024);
+ scudo::ScopedString Str;
Quarantine.getStats(&Str);
Str.output();
}
@@ -246,7 +246,7 @@
for (scudo::uptr I = 0; I < NumberOfThreads; I++)
pthread_join(T[I].Thread, 0);
- scudo::ScopedString Str(1024);
+ scudo::ScopedString Str;
Quarantine.getStats(&Str);
Str.output();
diff --git a/standalone/tests/secondary_test.cpp b/standalone/tests/secondary_test.cpp
index 2320a4d..2dc041b 100644
--- a/standalone/tests/secondary_test.cpp
+++ b/standalone/tests/secondary_test.cpp
@@ -64,7 +64,7 @@
L->deallocate(scudo::Options{}, V.back());
V.pop_back();
}
- scudo::ScopedString Str(1024);
+ scudo::ScopedString Str;
L->getStats(&Str);
Str.output();
L->unmapTestOnly();
@@ -122,7 +122,7 @@
}
}
}
- scudo::ScopedString Str(1024);
+ scudo::ScopedString Str;
L->getStats(&Str);
Str.output();
L->unmapTestOnly();
@@ -146,7 +146,7 @@
L->deallocate(scudo::Options{}, V.back());
V.pop_back();
}
- scudo::ScopedString Str(1024);
+ scudo::ScopedString Str;
L->getStats(&Str);
Str.output();
L->unmapTestOnly();
@@ -217,7 +217,7 @@
}
for (auto &T : Threads)
T.join();
- scudo::ScopedString Str(1024);
+ scudo::ScopedString Str;
L->getStats(&Str);
Str.output();
L->unmapTestOnly();
diff --git a/standalone/tests/strings_test.cpp b/standalone/tests/strings_test.cpp
index eed174d..332bac2 100644
--- a/standalone/tests/strings_test.cpp
+++ b/standalone/tests/strings_test.cpp
@@ -13,7 +13,7 @@
#include <limits.h>
TEST(ScudoStringsTest, Basic) {
- scudo::ScopedString Str(128);
+ scudo::ScopedString Str;
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),
@@ -29,7 +29,7 @@
}
TEST(ScudoStringsTest, Precision) {
- scudo::ScopedString Str(128);
+ scudo::ScopedString Str;
Str.append("%.*s", 3, "12345");
EXPECT_EQ(Str.length(), strlen(Str.data()));
EXPECT_STREQ("123", Str.data());
@@ -52,7 +52,7 @@
// 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);
+ scudo::ScopedString Str;
Str.clear();
fillString(Str, 2 * PageSize);
Str.clear();
@@ -68,7 +68,7 @@
template <typename T>
static void testAgainstLibc(const char *Format, T Arg1, T Arg2) {
- scudo::ScopedString Str(128);
+ scudo::ScopedString Str;
Str.append(Format, Arg1, Arg2);
char Buffer[128];
snprintf(Buffer, sizeof(Buffer), Format, Arg1, Arg2);
diff --git a/standalone/tests/vector_test.cpp b/standalone/tests/vector_test.cpp
index d2c6a9b..dc23c2a 100644
--- a/standalone/tests/vector_test.cpp
+++ b/standalone/tests/vector_test.cpp
@@ -23,14 +23,14 @@
}
TEST(ScudoVectorTest, Stride) {
- scudo::Vector<int> V;
- for (int i = 0; i < 1000; i++) {
- V.push_back(i);
- EXPECT_EQ(V.size(), i + 1U);
- EXPECT_EQ(V[i], i);
+ scudo::Vector<scudo::uptr> V;
+ for (scudo::uptr I = 0; I < 1000; I++) {
+ V.push_back(I);
+ EXPECT_EQ(V.size(), I + 1U);
+ EXPECT_EQ(V[I], I);
}
- for (int i = 0; i < 1000; i++)
- EXPECT_EQ(V[i], i);
+ for (scudo::uptr I = 0; I < 1000; I++)
+ EXPECT_EQ(V[I], I);
}
TEST(ScudoVectorTest, ResizeReduction) {
diff --git a/standalone/vector.h b/standalone/vector.h
index 6ca350a..2c9a6e2 100644
--- a/standalone/vector.h
+++ b/standalone/vector.h
@@ -19,14 +19,13 @@
// small vectors. The current implementation supports only POD types.
template <typename T> class VectorNoCtor {
public:
- void init(uptr InitialCapacity) {
- CapacityBytes = 0;
- Size = 0;
- Data = nullptr;
+ void init(uptr InitialCapacity = 0) {
+ Data = reinterpret_cast<T *>(&LocalData[0]);
+ CapacityBytes = sizeof(LocalData);
reserve(InitialCapacity);
}
void destroy() {
- if (Data)
+ if (Data != reinterpret_cast<T *>(&LocalData[0]))
unmap(Data, CapacityBytes);
}
T &operator[](uptr I) {
@@ -82,26 +81,24 @@
void reallocate(uptr NewCapacity) {
DCHECK_GT(NewCapacity, 0);
DCHECK_LE(Size, NewCapacity);
- const uptr NewCapacityBytes =
- roundUpTo(NewCapacity * sizeof(T), getPageSizeCached());
+ NewCapacity = roundUpTo(NewCapacity * sizeof(T), getPageSizeCached());
T *NewData =
- reinterpret_cast<T *>(map(nullptr, NewCapacityBytes, "scudo:vector"));
- if (Data) {
- memcpy(NewData, Data, Size * sizeof(T));
- unmap(Data, CapacityBytes);
- }
+ reinterpret_cast<T *>(map(nullptr, NewCapacity, "scudo:vector"));
+ memcpy(NewData, Data, Size * sizeof(T));
+ destroy();
Data = NewData;
- CapacityBytes = NewCapacityBytes;
+ CapacityBytes = NewCapacity;
}
- T *Data;
- uptr CapacityBytes;
- uptr Size;
+ T *Data = nullptr;
+ u8 LocalData[256] = {};
+ uptr CapacityBytes = 0;
+ uptr Size = 0;
};
template <typename T> class Vector : public VectorNoCtor<T> {
public:
- Vector() { VectorNoCtor<T>::init(1); }
+ Vector() { VectorNoCtor<T>::init(); }
explicit Vector(uptr Count) {
VectorNoCtor<T>::init(Count);
this->resize(Count);