Bug 39327

Summary: Enforce size constraints on various data structures in JavaScriptCore/wtf.
Product: WebKit Reporter: David Levin <levin>
Component: New BugsAssignee: David Levin <levin>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, darin, eric, hamaji, simon.fraser, webkit.review.bot, zoltan
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Attachments:
Description Flags
Patch
none
Proposed fix (same as last time, just updated to not have conflicts with things that have landed). darin: review+

David Levin
Reported 2010-05-18 16:00:05 PDT
Enforce size constraints on various data structures in JavaScriptCore/wtf.
Attachments
Patch (9.42 KB, patch)
2010-05-18 16:05 PDT, David Levin 		no flags
DetailsFormatted DiffDiff
Proposed fix (same as last time, just updated to not have conflicts with things that have landed). (9.18 KB, patch)
2010-05-21 14:07 PDT, David Levin 		darin: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
David Levin
Comment 1 2010-05-18 16:05:55 PDT
Created attachment 56418 [details] Patch
David Levin
Comment 2 2010-05-18 16:10:23 PDT
This is a start of helping to automatically enforce memory constraints on key data structures. The idea for doing this comes from https://bugs.webkit.org/show_bug.cgi?id=38906#c5 and that it would be good to detect issues like this automatically at build time. This patch only addresses wtf items. (A future one could address some WebCore items.)
David Levin
Comment 3 2010-05-21 14:07:07 PDT
Created attachment 56744 [details] Proposed fix (same as last time, just updated to not have conflicts with things that have landed).
WebKit Review Bot
Comment 4 2010-05-21 14:08:45 PDT
Attachment 56744 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1 JavaScriptCore/wtf/WTFCompileAsserts.cpp:33: Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] Total errors found: 1 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style.
David Levin
Comment 5 2010-05-21 14:14:34 PDT
(In reply to comment #4) > Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1 > JavaScriptCore/wtf/WTFCompileAsserts.cpp:33: Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] > Total errors found: 1 in 6 files > > If any of these errors are false positives, please file a bug against check-webkit-style. Done: https://bugs.webkit.org/show_bug.cgi?id=39514
Darin Adler
Comment 6 2010-05-21 15:26:48 PDT
This seems like a great idea. Anders Carlsson and I have talked about going even further.
Darin Adler
Comment 7 2010-05-21 15:28:05 PDT
Comment on attachment 56744 [details] Proposed fix (same as last time, just updated to not have conflicts with things that have landed). > +++ b/JavaScriptCore/wtf/WTFCompileAsserts.cpp It seems strange to have WTF in the filename here. Also, I think they are assertions, not "asserts". If the intent here is to have these be size assertions only, then I think the file name is not quite right.
David Levin
Comment 8 2010-05-21 15:45:04 PDT
(In reply to comment #7) > (From update of attachment 56744 [details]) > > +++ b/JavaScriptCore/wtf/WTFCompileAsserts.cpp > > It seems strange to have WTF in the filename here. Also, I think they are assertions, not "asserts". > > If the intent here is to have these be size assertions only, then I think the file name is not quite right. How about SizeLimits.cpp ? This is a cpp file which contains size constraints/limits for data structures in WTF (that don't have their own cpp file).
Darin Adler
Comment 9 2010-05-21 15:45:39 PDT
(In reply to comment #8) > How about SizeLimits.cpp ? Sounds pretty good to me.
WebKit Review Bot
Comment 10 2010-05-21 16:11:33 PDT
http://trac.webkit.org/changeset/59969 might have broken Chromium Linux Release
David Levin
Comment 11 2010-05-21 16:20:51 PDT
(In reply to comment #10) > http://trac.webkit.org/changeset/59969 might have broken Chromium Linux Release Fix this and Snow Leopard, here http://trac.webkit.org/changeset/59970 (by removing one compile assert for the moment for cross thread ref counted -- I'll look to add this one back in the future).
Eric Seidel (no email)
Comment 12 2010-09-02 13:48:53 PDT
Looks like this can be closed.
Note You need to log in before you can comment on or make changes to this bug.