diff --git a/lang/LangSource/GC.cpp b/lang/LangSource/GC.cpp index bf2bcfa82d1..cd0a7d9659a 100644 --- a/lang/LangSource/GC.cpp +++ b/lang/LangSource/GC.cpp @@ -664,6 +664,10 @@ HOT void PyrGC::Collect() { if (mNumToScan > 0) { // post("->Collect ns %d ng %d s %d\n", mNumToScan, mNumGrey, mScans); // DumpInfo(); + + // This operation can easily overflow if mNumToScan is large causing undefined behaviour. + // Right shifting a negative is also implementation defined and has led to bugs that only appear on certain + // operating systems and hardware. mNumToScan += mNumToScan >> 3; // post("->Collect2 ns %d ng %d s %d\n", mNumToScan, mNumGrey, mScans); diff --git a/lang/LangSource/GC.h b/lang/LangSource/GC.h index 512917963d8..3842182b5b9 100644 --- a/lang/LangSource/GC.h +++ b/lang/LangSource/GC.h @@ -303,9 +303,20 @@ inline void PyrGC::ToGrey2(PyrObjectHdr* obj) { } inline PyrObject* PyrGC::Allocate(size_t inNumBytes, int32 sizeclass, bool inRunCollection) { - if (inRunCollection && mNumToScan >= kScanThreshold) + // When allocating a large number of objects, mNumToScan (int32) can overflow. + // This usually happens when creating temporary frames, which might not be collected until later. + // This is designed to force collection when there might be a large number of temporary objects. + // The threshold is somewhat arbitrary and could be made larger, but it is better to be safe than fast, + // and sclang is not designed nor optimized to process large amounts of data. + const bool aboveTemporaryObjectLimit = mNumToScan > 5'000'000; + + // All allocations should set inRunCollection to true, but some types of methods do not as a part of an + // optimization. Don't scan an insignificant amount of memory, wait until above kScanThreshold. + const bool shouldCollect = inRunCollection && mNumToScan >= kScanThreshold; + + if (aboveTemporaryObjectLimit || shouldCollect) { Collect(); - else { + } else { if (inRunCollection) mUncollectedAllocations = 0; else