scudo: Limit the number of bytes tested in a realloc test.

[pcc]

Summary

This test was previously effectively doing:
P = malloc(X); write X bytes to P; P = realloc(P, X - Y); P = realloc(P, X)
and expecting that all X bytes stored to P would still be identical after
the final realloc.

This happens to be true for the current scudo implementation of realloc,
but is not guaranteed to be true by the C standard ("Any bytes in the new
object beyond the size of the old object have indeterminate values.").
This implementation detail will change with the new memory tagging support,
which unconditionally zeros newly allocated granules when memory tagging
is enabled. Fix this by limiting the number of bytes that we test to the
minimum size that we realloc the allocation to.

[merge_guards_bot]

Build result: pass - 60330 tests passed, 0 failed and 732 were skipped.

Log files: console-log.txt, CMakeCache.txt

[cryptoad]

Hey,
I guess my test is just wrong.
Initially I wanted to cover the situation described at https://reviews.llvm.org/D67293, eg: reallocating slightly larger chunk keeps the same block to save on some dealloc/alloc combos.
But you are right in the sense of that there is no guarantee that the extra bytes are identical.
I still want the sizes to be increasing to cover that scenario, but I think what I should have done is keep a MinSize that tracks the smallest size so that we keep the minimal size all the way throughout the test.
Does that make sense to you?

[pcc]

compiler-rt/lib/scudo/standalone/tests/combined_test.cpp

114

We already have scudo::Min(DataSize, NewSize) here. The problem is that it increases from DataSize - 32 to DataSize during the test. Maybe we should replace this with DataSize - 32?

[pcc]

compiler-rt/lib/scudo/standalone/tests/combined_test.cpp

114

I see that you edited your message while I was writing my reply. Your new suggestion would also work for me, let me know which one you prefer.

[cryptoad]

compiler-rt/lib/scudo/standalone/tests/combined_test.cpp

114

Yeah, I apologize my initial response was also wrong.
Whichever you feel is better I am perfectly fine with either.

[pcc]

Address review comment

[merge_guards_bot]

Build result: pass - 60353 tests passed, 0 failed and 732 were skipped.

Log files: console-log.txt, CMakeCache.txt

compiler-rt/lib/scudo/standalone/tests/combined_test.cpp

template <class Config> static void testAllocator() {
   // the same block size, but that should be the case for 2048 with our default
   // class size maps.
   P = Allocator->allocate(DataSize, Origin);
   memset(P, Marker, DataSize);
   for (scudo::sptr Delta = -32; Delta < 32; Delta += 8) {
     const scudo::uptr NewSize = DataSize + Delta;
     void *NewP = Allocator->reallocate(P, NewSize);
     EXPECT_EQ(NewP, P);
-    for (scudo::uptr I = 0; I < scudo::Min(DataSize, NewSize); I++)
+    for (scudo::uptr I = 0; I < DataSize - 32; I++)
       EXPECT_EQ((reinterpret_cast<char *>(NewP))[I], Marker);
   }
   Allocator->deallocate(P, Origin);
 
   // Allocates a bunch of chunks, then iterate over all the chunks, ensuring
   // they are the ones we allocated. This requires the allocator to not have any
   // other allocated chunk at this point (eg: won't work with the Quarantine).
   if (!UseQuarantine) {