Bug 141802 – bmalloc should implement malloc introspection (to stop false-positive leaks when MallocStackLogging is off)

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Geoffrey Garen]

bmalloc should implement malloc introspection (to stop false-positive leaks when MallocStackLogging is off)

[Geoffrey Garen]

Created attachment 246900 [details]
Patch

[WebKit Commit Bot]

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.

[Geoffrey Garen]

> 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.

[Andreas Kling]

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?

[Geoffrey Garen]

Committed r180359: <http://trac.webkit.org/changeset/180359>

[Geoffrey Garen]

Reopening to attach new patch.

[Geoffrey Garen]

Created attachment 246914 [details]
Patch

[Andreas Kling]

Comment on attachment 246914 [details]
Patch

rs=me

[Geoffrey Garen]

Committed r180363: <http://trac.webkit.org/changeset/180363>

[Csaba Osztrogonác]

(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

[WebKit Commit Bot]

Re-opened since this is blocked by bug 141814

[David Kilzer (:ddkilzer)]

<rdar://problem/19799874>

[Geoffrey Garen]

Created attachment 246977 [details]
Patch

[Andreas Kling]

Comment on attachment 246977 [details]
Patch

A-ha!

[Geoffrey Garen]

Committed r180430: <http://trac.webkit.org/changeset/180430>

[Stephanie Lewis]

Rolled out in http://trac.webkit.org/changeset/180604 until we can figure out why the PLT is crashing.

rdar://problem/19948015

[Geoffrey Garen]

Committed r180954: <http://trac.webkit.org/changeset/180954>

[Geoffrey Garen]

Reopening to attach new patch.

[Geoffrey Garen]

Created attachment 247797 [details]
Patch

[Geoffrey Garen]

Committed r180960: <http://trac.webkit.org/changeset/180960>