Summary: | MarkedBlock should have a footer instead of a header | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Filip Pizlo <fpizlo> | ||||||||||||
Component: | JavaScriptCore | Assignee: | Filip Pizlo <fpizlo> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | ggaren, jfbastien, keith_miller, mark.lam, msaboff, rmorisset, saam, webkit-bug-importer, ysuzuki | ||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||
Hardware: | All | ||||||||||||||
OS: | All | ||||||||||||||
Bug Depends on: | |||||||||||||||
Bug Blocks: | 181636 | ||||||||||||||
Attachments: |
|
Description
Filip Pizlo
2018-01-27 10:59:34 PST
Created attachment 332475 [details]
it's a start
Created attachment 332476 [details]
maybe this is right
Created attachment 332482 [details]
the patch
Created attachment 332483 [details]
the patch
Created attachment 332484 [details]
the patch
Now I like the changelog.
Comment on attachment 332484 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=332484&action=review r=me > Source/JavaScriptCore/heap/MarkedBlock.cpp:92 > + dataLog(RawPointer(this), ": Allocated.\n"); Drop logging or add verbose flag. > Source/JavaScriptCore/heap/MarkedBlock.h:269 > + CountingLock m_lock; This is 4 bytes, I'd put it next to HeapVersion (also 4) instead of before the 2x 2 byte mark counts. > Source/JavaScriptCore/heap/MarkedBlock.h:615 > return true; Seems like all uses of isAtom() should be succeeded by a size mask (when accessing the atoms) to Spectre-protect the accesses? (In reply to JF Bastien from comment #6) > Comment on attachment 332484 [details] > the patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=332484&action=review > > r=me > > > Source/JavaScriptCore/heap/MarkedBlock.cpp:92 > > + dataLog(RawPointer(this), ": Allocated.\n"); > > Drop logging or add verbose flag. That code was already there. The patch makes it seem like it was added. I'm just moving it. > > > Source/JavaScriptCore/heap/MarkedBlock.h:269 > > + CountingLock m_lock; > > This is 4 bytes, I'd put it next to HeapVersion (also 4) instead of before > the 2x 2 byte mark counts. But the goal is more padding! ;-) > > > Source/JavaScriptCore/heap/MarkedBlock.h:615 > > return true; > > Seems like all uses of isAtom() should be succeeded by a size mask (when > accessing the atoms) to Spectre-protect the accesses? I don't think so, because the index is not under user control. |