Bug 141802

Summary: bmalloc should implement malloc introspection (to stop false-positive leaks when MallocStackLogging is off)
Product: WebKit Reporter: Geoffrey Garen <ggaren>
Component: New BugsAssignee: Geoffrey Garen <ggaren>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, ddkilzer, kling, ossy, slewis, webkit-bug-importer, zwarich
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 141814    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch kling: review+

Description Geoffrey Garen 2015-02-19 11:12:35 PST
bmalloc should implement malloc introspection (to stop false-positive leaks when MallocStackLogging is off)
Comment 1 Geoffrey Garen 2015-02-19 11:22:11 PST
Created attachment 246900 [details]
Patch
Comment 2 WebKit Commit Bot 2015-02-19 11:24:54 PST
Attachment 246900 [details] did not pass style-queue:


ERROR: Source/bmalloc/bmalloc/Zone.cpp:26:  Found other header before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/bmalloc/bmalloc/Zone.cpp:27:  Found header this file implements after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/bmalloc/bmalloc/Zone.cpp:46:  type_mask is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/bmalloc/bmalloc/Zone.cpp:46:  zone_address is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 4 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Geoffrey Garen 2015-02-19 11:26:55 PST
> ERROR: Source/bmalloc/bmalloc/Zone.cpp:46:  type_mask is incorrectly named.
> Don't use underscores in your identifier names. 
> [readability/naming/underscores] [4]
> ERROR: Source/bmalloc/bmalloc/Zone.cpp:46:  zone_address is incorrectly
> named. Don't use underscores in your identifier names. 
> [readability/naming/underscores] [4]

I did this on purpose. We're implementing a delegation API, and I want our signature to match the signature declared in the header exactly.
Comment 4 Andreas Kling 2015-02-19 11:41:01 PST
Comment on attachment 246900 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=246900&action=review

r=me

> Source/bmalloc/bmalloc/Zone.cpp:54
> +    auto begin = remoteZone.superChunks().begin();
> +    auto end = remoteZone.superChunks().end();
> +    
> +    for (auto it = begin; it != end; ++it) {

Could we use range for here?

> Source/bmalloc/bmalloc/Zone.h:39
> +    // Enough capacity to track a 16GB heap, so probably enough for anything.
> +    static const size_t capacity = 512;

Let's go a bit bigger here. 64GB?
Comment 5 Geoffrey Garen 2015-02-19 12:43:02 PST
Committed r180359: <http://trac.webkit.org/changeset/180359>
Comment 6 Geoffrey Garen 2015-02-19 13:34:09 PST
Reopening to attach new patch.
Comment 7 Geoffrey Garen 2015-02-19 13:34:10 PST
Created attachment 246914 [details]
Patch
Comment 8 Andreas Kling 2015-02-19 13:34:35 PST
Comment on attachment 246914 [details]
Patch

rs=me
Comment 9 Geoffrey Garen 2015-02-19 13:38:19 PST
Committed r180363: <http://trac.webkit.org/changeset/180363>
Comment 10 Csaba Osztrogonác 2015-02-19 14:10:10 PST
(In reply to comment #9)
> Committed r180363: <http://trac.webkit.org/changeset/180363>

It caused serious regression (at least) on Apple Yosemite bot:
https://build.webkit.org/builders/Apple%20Yosemite%20Release%20WK2%20%28Tests%29/builds/2973
Comment 11 WebKit Commit Bot 2015-02-19 16:37:52 PST
Re-opened since this is blocked by bug 141814
Comment 12 David Kilzer (:ddkilzer) 2015-02-20 10:47:45 PST
<rdar://problem/19799874>
Comment 13 Geoffrey Garen 2015-02-20 11:19:20 PST
Created attachment 246977 [details]
Patch
Comment 14 Andreas Kling 2015-02-20 11:21:56 PST
Comment on attachment 246977 [details]
Patch

A-ha!
Comment 15 Geoffrey Garen 2015-02-20 11:28:51 PST
Committed r180430: <http://trac.webkit.org/changeset/180430>
Comment 16 Stephanie Lewis 2015-02-24 19:04:01 PST
Rolled out in http://trac.webkit.org/changeset/180604 until we can figure out why the PLT is crashing.

rdar://problem/19948015
Comment 17 Geoffrey Garen 2015-03-03 13:58:23 PST
Committed r180954: <http://trac.webkit.org/changeset/180954>
Comment 18 Geoffrey Garen 2015-03-03 14:49:59 PST
Reopening to attach new patch.
Comment 19 Geoffrey Garen 2015-03-03 14:50:02 PST
Created attachment 247797 [details]
Patch
Comment 20 Geoffrey Garen 2015-03-03 15:28:07 PST
Committed r180960: <http://trac.webkit.org/changeset/180960>