Summary: | Revert OSAllocator behavior to pre-Structure-Allocator change one | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Yusuke Suzuki <ysuzuki> | ||||
Component: | New Bugs | Assignee: | Yusuke Suzuki <ysuzuki> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | benjamin, cdumez, cmarcelo, ews-watchlist, keith_miller, mark.lam, msaboff, saam, tzagallo, webkit-bug-importer | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | WebKit Nightly Build | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Yusuke Suzuki
2022-01-31 23:43:02 PST
Created attachment 450504 [details]
Patch
Comment on attachment 450504 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=450504&action=review r=me > Source/WTF/ChangeLog:42 > + We also fix that tryReserveUncommitted is not correctly handling MAP_FAILED. We should not return MAP_FAILED from OSAllocator, instead, I suggest "fix that tryReserveUncommitted is not correctly handling MAP_FAILED" => "fix a bug in tryReserveUncommitted where it handles MAP_FAILED incorrectly". > Source/WTF/wtf/posix/OSAllocatorPOSIX.cpp:116 > +void* OSAllocator::tryReserveUncommitted(size_t bytes, Usage usage, bool writable, bool executable, bool jitCageEnabled, bool includesGuardPages) Why remove the "OSAllocator::" qualifier for Usage? You kept to for tryReserveAndCommit above. I see that it is not used for functions below. Maybe remove it also for tryReserveAndCommit above? Comment on attachment 450504 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=450504&action=review >> Source/WTF/ChangeLog:42 >> + We also fix that tryReserveUncommitted is not correctly handling MAP_FAILED. We should not return MAP_FAILED from OSAllocator, instead, > > I suggest "fix that tryReserveUncommitted is not correctly handling MAP_FAILED" => "fix a bug in tryReserveUncommitted where it handles MAP_FAILED incorrectly". Changed. >> Source/WTF/wtf/posix/OSAllocatorPOSIX.cpp:116 >> +void* OSAllocator::tryReserveUncommitted(size_t bytes, Usage usage, bool writable, bool executable, bool jitCageEnabled, bool includesGuardPages) > > Why remove the "OSAllocator::" qualifier for Usage? You kept to for tryReserveAndCommit above. I see that it is not used for functions below. Maybe remove it also for tryReserveAndCommit above? Because now it is static function of OSAllocator::. So, we don't need OSAllocator:: prefix to Usage. Removed that of tryReserveAndCommit too. Committed r288870 (246622@trunk): <https://commits.webkit.org/246622@trunk> |