Bug 193423

Summary: JSFunction::canUseAllocationProfile() should account for builtin functions with no own prototypes.
Product: WebKit Reporter: Mark Lam <mark.lam>
Component: JavaScriptCoreAssignee: Mark Lam <mark.lam>
Status: RESOLVED FIXED    
Severity: Normal CC: ews-watchlist, fpizlo, keith_miller, msaboff, rmorisset, saam, tzagallo, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
proposed patch.
saam: review-
proposed patch.
saam: review+
patch for landing. none

Mark Lam
Reported 2019-01-14 18:25:54 PST
Attachments
proposed patch. (5.21 KB, patch)
2019-01-14 18:49 PST, Mark Lam
saam: review-
proposed patch. (6.81 KB, patch)
2019-01-14 22:24 PST, Mark Lam
saam: review+
patch for landing. (8.74 KB, patch)
2019-01-15 18:44 PST, Mark Lam
no flags
Mark Lam
Comment 1 2019-01-14 18:49:03 PST
Created attachment 359116 [details] proposed patch.
Saam Barati
Comment 2 2019-01-14 18:50:40 PST
Comment on attachment 359116 [details] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=359116&action=review > Source/JavaScriptCore/runtime/JSFunctionInlines.h:125 > + if (prototype.isCell()) { > + JSCell* cell = prototype.asCell(); > + if (cell->isGetterSetter() || cell->isCustomGetterSetter() || cell->isProxy()) > + return false; > + } Not a fan of this. Can you do the structure lookup along w/ and inspect the bits that say if this is a getter/setter?
Mark Lam
Comment 3 2019-01-14 22:24:17 PST
Created attachment 359137 [details] proposed patch.
Saam Barati
Comment 4 2019-01-15 12:19:54 PST
Comment on attachment 359137 [details] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=359137&action=review r=me > Source/JavaScriptCore/ChangeLog:10 > + because the majority of them has no prototype property. The only exception to s/has/have > Source/JavaScriptCore/runtime/JSFunctionInlines.h:120 > + if (!prototype || (attributes & PropertyAttribute::AccessorOrCustomAccessorOrValue)) Can we add a test case for this? Like define a getter as the prototype of a random builtin function.
Mark Lam
Comment 5 2019-01-15 18:44:10 PST
Created attachment 359239 [details] patch for landing. Thanks for the review.
Mark Lam
Comment 6 2019-01-16 10:12:02 PST
Note You need to log in before you can comment on or make changes to this bug.