[JSC] Ensure PackedCellPtr only takes non-large-allocation pointers
Created attachment 374910 [details] Patch
Created attachment 374911 [details] Patch
Comment on attachment 374910 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=374910&action=review > Source/JavaScriptCore/ChangeLog:9 > + But this fact only meets when the JSCell is not large allocation. Currently, we are using PackedCellPtr really? We don't round large allocations?
Created attachment 374912 [details] Patch
Comment on attachment 374910 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=374910&action=review >> Source/JavaScriptCore/ChangeLog:9 >> + But this fact only meets when the JSCell is not large allocation. Currently, we are using PackedCellPtr > > really? We don't round large allocations? grammar: only meets => only holds.
Comment on attachment 374910 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=374910&action=review >>> Source/JavaScriptCore/ChangeLog:9 >>> + But this fact only meets when the JSCell is not large allocation. Currently, we are using PackedCellPtr >> >> really? We don't round large allocations? > > grammar: only meets => only holds. Fixed the grammar part, thanks. Let's see LargeAllocation::isLargeAllocation implementation. https://github.com/WebKit/webkit/blob/master/Source/JavaScriptCore/heap/LargeAllocation.h#L58-L61 https://github.com/WebKit/webkit/blob/master/Source/JavaScriptCore/heap/LargeAllocation.h#L148-L149 LargeAllocation's HeapCell is intentionally not 16-byte aligned to encode this information into the cell pointer.
Created attachment 374915 [details] Patch
Comment on attachment 374915 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=374915&action=review r=me > Source/JavaScriptCore/ChangeLog:9 > + But this fact only meets when the JSCell is not large allocation. Currently, we are using PackedCellPtr still not fixed: /meet/holds/ > Source/JavaScriptCore/ChangeLog:10 > + only for the cell types which holds the above requirement. But we would like to ensure that statically. this one should be 'meet' instead of 'holds'.
Comment on attachment 374915 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=374915&action=review > Source/JavaScriptCore/heap/PackedCellPtr.h:43 > + ASSERT(!pointer || !pointer->isLargeAllocation()); Can you just assert that it's 16 byte aligned? Seems more straight forward
Comment on attachment 374915 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=374915&action=review Thanks, fixed. >> Source/JavaScriptCore/ChangeLog:9 >> + But this fact only meets when the JSCell is not large allocation. Currently, we are using PackedCellPtr > > still not fixed: /meet/holds/ Oops, fixed. >> Source/JavaScriptCore/ChangeLog:10 >> + only for the cell types which holds the above requirement. But we would like to ensure that statically. > > this one should be 'meet' instead of 'holds'. Fixed.
Comment on attachment 374915 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=374915&action=review >> Source/JavaScriptCore/heap/PackedCellPtr.h:43 >> + ASSERT(!pointer || !pointer->isLargeAllocation()); > > Can you just assert that it's 16 byte aligned? Seems more straight forward Sounds nice. Fixed.
Committed r247843: <https://trac.webkit.org/changeset/247843>
<rdar://problem/53564083>
It seems that std::invoke_result_t is missing in some clang environments. I think it is C++17 features, and possibly it is not implemented yet. Use std::result_of instead, while it is deprecated in C++17 and removed in C++20, because we are now using std::result_of in the other places.
Committed r247844: <https://trac.webkit.org/changeset/247844>