Bug 171888

Summary: Heap::heap() should behave gracefully for null pointers
Product: WebKit Reporter: Filip Pizlo <fpizlo>
Component: JavaScriptCoreAssignee: Filip Pizlo <fpizlo>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, keith_miller, mark.lam, msaboff, saam
Priority: P2    
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
the patch mark.lam: review+

Filip Pizlo
Reported 2017-05-09 15:17:04 PDT
Some callers of Heap::heap() can pass a null cell and they will behave gracefully if we return a null Heap. So, let's do that.
Attachments
the patch (1.51 KB, patch)
2017-05-09 15:19 PDT, Filip Pizlo 		mark.lam: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Filip Pizlo
Comment 1 2017-05-09 15:19:11 PDT
Created attachment 309549 [details] the patch
Mark Lam
Comment 2 2017-05-09 15:23:15 PDT
Comment on attachment 309549 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=309549&action=review r=me > Source/JavaScriptCore/heap/HeapInlines.h:49 > + if (!cell) nit: make this "if (UNLIKELY(!cell))" > Source/JavaScriptCore/heap/HeapInlines.h:56 > if (!v.isCell()) might as well make this one UNLIKELY too.
Filip Pizlo
Comment 3 2017-05-09 15:25:56 PDT
(In reply to Mark Lam from comment #2) > Comment on attachment 309549 [details] > the patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=309549&action=review > > r=me > > > Source/JavaScriptCore/heap/HeapInlines.h:49 > > + if (!cell) > > nit: make this "if (UNLIKELY(!cell))" > > > Source/JavaScriptCore/heap/HeapInlines.h:56 > > if (!v.isCell()) > > might as well make this one UNLIKELY too. UNIKELY is unlikely to help if it's for an early return. UNLIKELY is most useful when it lets the compiler know that some code is unlikely to be reached, but here the fall-through path is likely to be reached either way. Also, I already benchmarked it without the UNLIKELY.
Filip Pizlo
Comment 4 2017-05-09 15:27:21 PDT
Landed in https://trac.webkit.org/changeset/216547/webkit
Note You need to log in before you can comment on or make changes to this bug.