Make malloc validation useful
Created attachment 87606 [details] Patch
Attachment 87606 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1 Source/JavaScriptCore/wtf/FastMalloc.h:193: Should have a space between // and comment [whitespace/comments] [4] Source/JavaScriptCore/wtf/FastMalloc.cpp:169: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] Source/JavaScriptCore/wtf/FastMalloc.cpp:227: One space before end of line comments [whitespace/comments] [5] Source/JavaScriptCore/wtf/FastMalloc.cpp:340: One space before end of line comments [whitespace/comments] [5] Source/JavaScriptCore/wtf/FastMalloc.cpp:1304: pageheap_lock is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/wtf/FastMalloc.cpp:2480: Should have a space between // and comment [whitespace/comments] [4] Source/JavaScriptCore/wtf/FastMalloc.cpp:3827: One space before end of line comments [whitespace/comments] [5] Source/JavaScriptCore/wtf/FastMalloc.cpp:3992: One space before end of line comments [whitespace/comments] [5] Total errors found: 8 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style.
Attachment 87606 [details] did not build on win: Build output: http://queues.webkit.org/results/8280969
2>..\..\wtf\FastMalloc.cpp(236) : error C2220: warning treated as error - no 'object' file generated 2>..\..\wtf\FastMalloc.cpp(236) : warning C4245: '=' : conversion from '' to 'WTF::Internal::ValidationTag', signed/unsigned mismatch
+ memset(p, 0xCC, header->m_size); I think 0xbbadbeef would be better, since that's that standard dereference we look for to indicate assertion failure. + //if (Internal::fastMallocMatchValidationType(p) != allocType) + // Internal::fastMallocMatchFailed(p); Boo on commented-out code. + //ASSERT(IsHeld(pageheap_lock)); Boo on commented-out code. - ASSERT(IsHeld(pageheap_lock)); + ASSERT(pageheap_lock.IsHeld()); Why all of these changes? r- because of the build failure.
(In reply to comment #5) > + memset(p, 0xCC, header->m_size); > > I think 0xbbadbeef would be better, since that's that standard dereference we look for to indicate assertion failure. yeah you'd think memset would be able to plan an entire int given that it takes an int argument, but it converts to unsigned char first. Hence 0xCC. > > + //if (Internal::fastMallocMatchValidationType(p) != allocType) > + // Internal::fastMallocMatchFailed(p); > > Boo on commented-out code. Removed -- it would be nice to add this back, but currently new/free mismatches exist all over the place > > + //ASSERT(IsHeld(pageheap_lock)); > > Boo on commented-out code. Fixed > > - ASSERT(IsHeld(pageheap_lock)); > + ASSERT(pageheap_lock.IsHeld()); > > Why all of these changes? Apparently no one has done a debug build of fast malloc for a long time. > > r- because of the build failure.
Created attachment 87622 [details] Patch
Attachment 87622 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1 Source/JavaScriptCore/wtf/FastMalloc.cpp:169: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] Source/JavaScriptCore/wtf/FastMalloc.cpp:227: One space before end of line comments [whitespace/comments] [5] Source/JavaScriptCore/wtf/FastMalloc.cpp:340: One space before end of line comments [whitespace/comments] [5] Source/JavaScriptCore/wtf/FastMalloc.cpp:1304: pageheap_lock is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/wtf/FastMalloc.cpp:3827: One space before end of line comments [whitespace/comments] [5] Source/JavaScriptCore/wtf/FastMalloc.cpp:3992: One space before end of line comments [whitespace/comments] [5] Total errors found: 6 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style.
Attachment 87622 [details] did not build on win: Build output: http://queues.webkit.org/results/8311020
Comment on attachment 87622 [details] Patch Still failing to build.
Created attachment 88089 [details] Patch
Attachment 88089 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style']" exit_code: 1 Source/JavaScriptCore/wtf/FastMalloc.cpp:169: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] Source/JavaScriptCore/wtf/FastMalloc.cpp:227: One space before end of line comments [whitespace/comments] [5] Source/JavaScriptCore/wtf/FastMalloc.cpp:340: One space before end of line comments [whitespace/comments] [5] Source/JavaScriptCore/wtf/FastMalloc.cpp:1304: pageheap_lock is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/wtf/FastMalloc.cpp:3827: One space before end of line comments [whitespace/comments] [5] Source/JavaScriptCore/wtf/FastMalloc.cpp:3992: One space before end of line comments [whitespace/comments] [5] Total errors found: 6 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 88103 [details] Patch
Attachment 88103 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style']" exit_code: 1 Source/JavaScriptCore/wtf/FastMalloc.cpp:169: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] Source/JavaScriptCore/wtf/FastMalloc.cpp:227: One space before end of line comments [whitespace/comments] [5] Source/JavaScriptCore/wtf/FastMalloc.cpp:340: One space before end of line comments [whitespace/comments] [5] Source/JavaScriptCore/wtf/FastMalloc.cpp:1306: pageheap_lock is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/wtf/FastMalloc.cpp:3829: One space before end of line comments [whitespace/comments] [5] Source/JavaScriptCore/wtf/FastMalloc.cpp:3994: One space before end of line comments [whitespace/comments] [5] Total errors found: 6 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 88103 [details] Patch r=me, now that the build works and debug builds do not ASSERT.
Committed r82876: <http://trac.webkit.org/changeset/82876>
http://trac.webkit.org/changeset/82876 might have broken GTK Linux 32-bit Release
(In reply to comment #16) > Committed r82876: <http://trac.webkit.org/changeset/82876> I reverted it because of test crashes on the SnowLeopard bot. (Release-only?)