Bug 218733

Summary: REGRESSION (r269525): Many layout tests crash when run under ASan
Product: WebKit Reporter: Wenson Hsieh <wenson_hsieh>
Component: Layout and RenderingAssignee: Wenson Hsieh <wenson_hsieh>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, sabouhallawa, simon.fraser, thorton, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
roundUpToMultipleOf + remove some reinterpret_casts
thorton: review+
Patch for landing none

Wenson Hsieh
Reported 2020-11-09 18:38:17 PST
Attachments
Patch (32.23 KB, patch)
2020-11-09 20:17 PST, Wenson Hsieh
no flags
roundUpToMultipleOf + remove some reinterpret_casts (31.03 KB, patch)
2020-11-10 10:59 PST, Wenson Hsieh
thorton: review+
Patch for landing (31.00 KB, patch)
2020-11-10 11:59 PST, Wenson Hsieh
no flags
Wenson Hsieh
Comment 1 2020-11-09 20:17:52 PST Comment hidden (obsolete)
Wenson Hsieh
Comment 2 2020-11-10 10:59:07 PST
Created attachment 413715 [details] roundUpToMultipleOf + remove some reinterpret_casts
Tim Horton
Comment 3 2020-11-10 11:10:34 PST
Comment on attachment 413715 [details] roundUpToMultipleOf + remove some reinterpret_casts View in context: https://bugs.webkit.org/attachment.cgi?id=413715&action=review > Source/WebCore/platform/graphics/displaylists/DisplayListItemBuffer.cpp:558 > + auto itemLocation = destination.data + sizeof(uint64_t); offset? > Source/WebCore/platform/graphics/displaylists/DisplayListItemBuffer.h:80 > - return *reinterpret_cast<T*>(&data[sizeof(ItemType)]); > + return *reinterpret_cast<T*>(&data[sizeof(uint64_t)]); Didn't want to just inflate ItemType? I guess that's OK
Wenson Hsieh
Comment 4 2020-11-10 11:25:10 PST
Comment on attachment 413715 [details] roundUpToMultipleOf + remove some reinterpret_casts View in context: https://bugs.webkit.org/attachment.cgi?id=413715&action=review >> Source/WebCore/platform/graphics/displaylists/DisplayListItemBuffer.cpp:558 >> + auto itemLocation = destination.data + sizeof(uint64_t); > > offset? Renamed! >> Source/WebCore/platform/graphics/displaylists/DisplayListItemBuffer.h:80 >> + return *reinterpret_cast<T*>(&data[sizeof(uint64_t)]); > > Didn't want to just inflate ItemType? I guess that's OK Interesting. I suppose I could! I wasn't sure if it made sense to make ItemType itself larger for the sake of aligning item data to 8 bytes…(for instance, if another data structure wants to keep just a list of item types, it would end up being much larger than necessary. But perhaps that's not really an issue, seeing as there's already padding after each type in the item buffer itself).
Wenson Hsieh
Comment 5 2020-11-10 11:59:38 PST
Created attachment 413720 [details] Patch for landing
EWS
Comment 6 2020-11-10 12:31:09 PST
Committed r269640: <https://trac.webkit.org/changeset/269640> All reviewed patches have been landed. Closing bug and clearing flags on attachment 413720 [details].
Note You need to log in before you can comment on or make changes to this bug.