[msan] Dynamically grow kNumStackOriginDescrs

[thurstond]

StackOriginDescr/StackOriginPC are currently fixed size arrays
and have a FIXME of resizability. This patch dynamically doubles
the size of these arrays as needed (for runtime that is amortized linear
in the size of the array).

Since NumStackOriginDescrs (the number of items currently stored in the array)
is monotonically increasing, there is no need to ever decrease the
allocated size of the array.

Testing notes: the old fixed limit was 1 million variables; this would be an
extremely large test case. By manually capping the maximum size, I found
that param_tls_limit.cpp and compress_stack_depot.cpp are the tests that are most sensitive to
kNumStackOriginDescrs.

[llvmbot]

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Thurston Dang (thurstond)

Changes

StackOriginDescr/StackOriginPC are currently fixed size arrays
and have a FIXME of resizability. This patch dynamically doubles
the size of these arrays as needed (for runtime that is amortized linear
in the size of the array).

Since NumStackOriginDescrs (the number of items currently stored in the array)
is monotonically increasing, there is no need to ever decrease the
allocated size of the array.

Testing notes: the old fixed limit was 1 million variables; this would be an
extremely large test case. By manually capping the maximum size, I found
that param_tls_limit.cpp is the test that is most sensitive to
kNumStackOriginDescrs.

---

Full diff: https://github.com/llvm/llvm-project/pull/92826.diff

1 Files Affected:

• (modified) compiler-rt/lib/msan/msan.cpp (+25-4)

diff --git a/compiler-rt/lib/msan/msan.cpp b/compiler-rt/lib/msan/msan.cpp
index a2fc27de1901b..e66b5145f83de 100644
--- a/compiler-rt/lib/msan/msan.cpp
+++ b/compiler-rt/lib/msan/msan.cpp
@@ -99,10 +99,10 @@ bool msan_init_is_running;
 int msan_report_count = 0;
 
 // Array of stack origins.
-// FIXME: make it resizable.
-static const uptr kNumStackOriginDescrs = 1024 * 1024;
-static const char *StackOriginDescr[kNumStackOriginDescrs];
-static uptr StackOriginPC[kNumStackOriginDescrs];
+// This is resized dynamically by ResizeStackOriginMetadata
+static uptr kNumStackOriginDescrs = 0;
+static char **StackOriginDescr = nullptr;
+static uptr *StackOriginPC = nullptr;
 static atomic_uint32_t NumStackOriginDescrs;
 
 void Flags::SetDefaults() {
@@ -282,7 +282,27 @@ void ScopedThreadLocalStateBackup::Restore() {
 void UnpoisonThreadLocalState() {
 }
 
+// Grow stack origin metadata structures exponentially if out of space
+static void ResizeStackOriginMetadata(u32 id) {
+  if (id >= kNumStackOriginDescrs) {
+    if (kNumStackOriginDescrs == 0)
+      kNumStackOriginDescrs = 1;
+    else
+      kNumStackOriginDescrs = kNumStackOriginDescrs * 2;
+
+    VReport(2, "Resized stack origin metadata to %lu entries\n",
+            kNumStackOriginDescrs);
+  }
+
+  StackOriginDescr = (char **)InternalRealloc(
+      StackOriginDescr, kNumStackOriginDescrs * sizeof(char *));
+  StackOriginPC = (uptr *)InternalRealloc(StackOriginPC,
+                                          kNumStackOriginDescrs * sizeof(uptr));
+  // No checks needed - InternalRealloc() will die if out of memory
+}
+
 const char *GetStackOriginDescr(u32 id, uptr *pc) {
+  ResizeStackOriginMetadata(id);
   CHECK_LT(id, kNumStackOriginDescrs);
   if (pc) *pc = StackOriginPC[id];
   return StackOriginDescr[id];
@@ -312,6 +332,7 @@ static inline void SetAllocaOrigin(void *a, uptr size, u32 *id_ptr, char *descr,
   u32 id = *id_ptr;
   if (id == 0 || id == first_timer) {
     u32 idx = atomic_fetch_add(&NumStackOriginDescrs, 1, memory_order_relaxed);
+    ResizeStackOriginMetadata(idx);
     CHECK_LT(idx, kNumStackOriginDescrs);
     StackOriginDescr[idx] = descr;
     StackOriginPC[idx] = pc;

[eugenis]

This is not thread safe. I do not think you can do realloc() if you want the mutation to be lock free.

We could simply increase the constant size of the buffer (not perfect, but it is just .bss, not real memory).

We could go with an array of pointers to exponentially increasing allocations (i.e. same as your case, but never deallocate, instead just allocate more and consider the container a logical concatenation of multiple buffers).

[thurstond]

This is not thread safe. I do not think you can do realloc() if you want the mutation to be lock free.

Thanks for the review, I had overlooked thread safety.

We could simply increase the constant size of the buffer (not perfect, but it is just .bss, not real memory).

This is implemented in #92838

We could go with an array of pointers to exponentially increasing allocations (i.e. same as your case, but never deallocate, instead just allocate more and consider the container a logical concatenation of multiple buffers).

This is still tricky to implement in a thread-safe, lock-free manner, because there are two arrays to be grown (StackOriginDescrs, StackOriginPC).

[eugenis]

We could go with an array of pointers to exponentially increasing allocations (i.e. same as your case, but never deallocate, instead just allocate more and consider the container a logical concatenation of multiple buffers).

This is still tricky to implement in a thread-safe, lock-free manner, because there are two arrays to be grown (StackOriginDescrs, StackOriginPC).

I think it will actually work pretty well.

If subarrays are power of 2 sized (1, 2, 4, 8, 16, ...), then the subarray for index i is basically log2(i+1). Both arrays can be handled independently; once the index is acquired, the subarray pointer is initialized with a double-checked locking pattern.

[eugenis]

But then, it seems easier to just MAP_NORESERVE a large enough array in msan_init. It can be really large then, no concerns about relocation offset.