Leak of uint32_t arrays in testFastForwardCopy32() due to `continue` statements not releasing memory allocated by `new`. void testFastForwardCopy32() { #if CPU(X86_64) for (const bool aligned : { true, false }) { for (const bool overlap : { false, true }) { for (size_t arrsize : { 1, 4, 5, 6, 8, 10, 12, 16, 20, 40, 100, 1000}) { size_t overlapAmount = 5; uint32_t* arr1, *arr2; if (overlap) { arr1 = new uint32_t[arrsize * 2]; arr2 = arr1 + (arrsize - overlapAmount); } else { arr1 = new uint32_t[arrsize]; arr2 = new uint32_t[arrsize]; } if (!aligned && arrsize < 3) continue; // LEAKS! if (overlap && arrsize <= overlapAmount + 3) continue; // LEAKS! [...] if (!overlap) { delete[] arr1; delete[] arr2; } else delete[] arr1; } } } #endif } <https://trac.webkit.org/browser/webkit/trunk/Source/JavaScriptCore/b3/testb3_8.cpp>
Leak introduced by: Bug 200181: Identify memcpy loops in b3 <https://bugs.webkit.org/show_bug.cgi?id=200181> <https://trac.webkit.org/changeset/248938>
Created attachment 378811 [details] Patch v1
Created attachment 378812 [details] Patch v2
(In reply to David Kilzer (:ddkilzer) from comment #3) > Created attachment 378812 [details] > Patch v2 ERROR: Source/JavaScriptCore/b3/testb3_8.cpp:881: Use 'WTF::makeUnique<>' instead of 'std::make_unique<>'. [runtime/wtf_make_unique] [4] ERROR: Source/JavaScriptCore/b3/testb3_8.cpp:885: Use 'WTF::makeUnique<>' instead of 'std::make_unique<>'. [runtime/wtf_make_unique] [4] ERROR: Source/JavaScriptCore/b3/testb3_8.cpp:886: Use 'WTF::makeUnique<>' instead of 'std::make_unique<>'. [runtime/wtf_make_unique] [4] Total errors found: 3 in 2 files This is a false positive. WTF::makeUnique<> doesn't handle POD types, so std::make_unique<> must still be used. (See build failures of "Patch v1".)
<rdar://problem/55380614>
Comment on attachment 378812 [details] Patch v2 View in context: https://bugs.webkit.org/attachment.cgi?id=378812&action=review > Source/JavaScriptCore/b3/testb3_8.cpp:885 > + array1 = std::make_unique<uint32_t[]>(arrsize); Should we be using WTF::makeUnique?
(In reply to Saam Barati from comment #6) > Comment on attachment 378812 [details] > Patch v2 > > View in context: > https://bugs.webkit.org/attachment.cgi?id=378812&action=review > > > Source/JavaScriptCore/b3/testb3_8.cpp:885 > > + array1 = std::make_unique<uint32_t[]>(arrsize); > > Should we be using WTF::makeUnique? No. It doesn't support POD types. See Comment #4.
Comment on attachment 378812 [details] Patch v2 View in context: https://bugs.webkit.org/attachment.cgi?id=378812&action=review >>> Source/JavaScriptCore/b3/testb3_8.cpp:885 >>> + array1 = std::make_unique<uint32_t[]>(arrsize); >> >> Should we be using WTF::makeUnique? > > No. It doesn't support POD types. See Comment #4. Or is the patch missing a #include of <wtf/UniqueArray.h>?
Comment on attachment 378812 [details] Patch v2 View in context: https://bugs.webkit.org/attachment.cgi?id=378812&action=review >>>> Source/JavaScriptCore/b3/testb3_8.cpp:885 >>>> + array1 = std::make_unique<uint32_t[]>(arrsize); >>> >>> Should we be using WTF::makeUnique? >> >> No. It doesn't support POD types. See Comment #4. > > Or is the patch missing a #include of <wtf/UniqueArray.h>? Use `makeUniqueArray<uint32_t>(arrsize)` instead.
(In reply to Yusuke Suzuki from comment #9) > Comment on attachment 378812 [details] > Patch v2 > > View in context: > https://bugs.webkit.org/attachment.cgi?id=378812&action=review > > >>>> Source/JavaScriptCore/b3/testb3_8.cpp:885 > >>>> + array1 = std::make_unique<uint32_t[]>(arrsize); > >>> > >>> Should we be using WTF::makeUnique? > >> > >> No. It doesn't support POD types. See Comment #4. > > > > Or is the patch missing a #include of <wtf/UniqueArray.h>? > > Use `makeUniqueArray<uint32_t>(arrsize)` instead. Thanks! Just noticed that.
Created attachment 378856 [details] Patch v3
Created attachment 378858 [details] Patch v4
(In reply to David Kilzer (:ddkilzer) from comment #4) > (In reply to David Kilzer (:ddkilzer) from comment #3) > > Created attachment 378812 [details] > > Patch v2 > > ERROR: Source/JavaScriptCore/b3/testb3_8.cpp:881: Use 'WTF::makeUnique<>' > instead of 'std::make_unique<>'. [runtime/wtf_make_unique] [4] > ERROR: Source/JavaScriptCore/b3/testb3_8.cpp:885: Use 'WTF::makeUnique<>' > instead of 'std::make_unique<>'. [runtime/wtf_make_unique] [4] > ERROR: Source/JavaScriptCore/b3/testb3_8.cpp:886: Use 'WTF::makeUnique<>' > instead of 'std::make_unique<>'. [runtime/wtf_make_unique] [4] > Total errors found: 3 in 2 files > > This is a false positive. WTF::makeUnique<> doesn't handle POD types, so > std::make_unique<> must still be used. (See build failures of "Patch v1".) Filed Bug 201818 to fix this error message when using arrays: Bug 201818: check-webkit-style: Fix warning message for std::make_unique<typename[]> <https://bugs.webkit.org/show_bug.cgi?id=201818>
Comment on attachment 378858 [details] Patch v4 Clearing flags on attachment: 378858 Committed r249914: <https://trac.webkit.org/changeset/249914>
All reviewed patches have been landed. Closing bug.