Bug 164431

Summary: Implement MallocScribble for bmalloc
Product: WebKit Reporter: David Kilzer (:ddkilzer) <ddkilzer>
Component: bmallocAssignee: David Kilzer (:ddkilzer) <ddkilzer>
Status: RESOLVED WONTFIX    
Severity: Normal CC: beidson, ggaren, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Safari 10   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=128249
Attachments:
Description Flags
Patch v1 beidson: review-

David Kilzer (:ddkilzer)
Reported 2016-11-04 13:59:38 PDT
We should add support for MallocScribble to bmalloc.
Attachments
Patch v1 (9.35 KB, patch)
2016-11-16 13:14 PST, David Kilzer (:ddkilzer) 		beidson: review-
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2016-11-04 14:00:00 PDT
<rdar://problem/29118340>
David Kilzer (:ddkilzer)
Comment 2 2016-11-16 13:14:27 PST
Created attachment 294968 [details] Patch v1
Geoffrey Garen
Comment 3 2016-11-16 13:32:39 PST
Comment on attachment 294968 [details] Patch v1 Why do we want to implement scribbling in bmalloc instead of deferring to system malloc? Do we also plan to duplicate the 14 other Malloc* options, or is there something special about scribbling? In general, I think it's best to defer debugging options to a separate implementation -- system malloc or our own -- so that debugging cruft doesn't sneak into high-performance code. For example, I don't think we want to do the m_isMallocScribbleEnabled check in deallocateFastCase. Even if we mark it unlikely, we still increase code size and harm register allocation. I went out of my way to avoid even checking m_isBmallocEnabled in deallocateFastCase. I'd like to keep that code as tight as possible.
Brady Eidson
Comment 4 2018-02-14 10:35:53 PST
Comment on attachment 294968 [details] Patch v1 Patches that have been up for review since 2016 are almost certainly too stale to be relevant to trunk in their current form. If this patch is still important please rebase it and post it for review again.
Note You need to log in before you can comment on or make changes to this bug.