Bug 223649 - Don't require VM_FLAGS_PERMANENT on the simulator builds
Summary: Don't require VM_FLAGS_PERMANENT on the simulator builds
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Saam Barati
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-03-23 11:48 PDT by Saam Barati
Modified: 2021-03-23 13:55 PDT (History)
5 users (show)

See Also:


Attachments
patch (2.22 KB, patch)
2021-03-23 12:14 PDT, Saam Barati
ap: review+
Details | Formatted Diff | Diff
patch for landing (2.31 KB, patch)
2021-03-23 13:12 PDT, Saam Barati
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Saam Barati 2021-03-23 11:48:14 PDT
...
Comment 1 Radar WebKit Bug Importer 2021-03-23 11:48:30 PDT
<rdar://problem/75747788>
Comment 2 Saam Barati 2021-03-23 12:14:57 PDT
Created attachment 424048 [details]
patch
Comment 3 Alexey Proskuryakov 2021-03-23 12:28:46 PDT
Comment on attachment 424048 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=424048&action=review

> Source/WTF/wtf/WTFConfig.cpp:80
> +#if HAVE(VM_FLAGS_PERMANENT) && PLATFORM(IOS_FAMILY_SIMULATOR)

I'd add FIXME: Remove the fallback when the oldest host OS that we support for Simulator has VM_FLAGS_PERMANENT.

> Source/WTF/wtf/WTFConfig.cpp:82
> +            flags = VM_FLAGS_FIXED | VM_FLAGS_OVERWRITE;

It should be safer against potential future code modifications to clear VM_FLAGS_PERMANENT here, not to double-guess what it was set to ten lines above.
Comment 4 Saam Barati 2021-03-23 13:08:13 PDT
Comment on attachment 424048 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=424048&action=review

>> Source/WTF/wtf/WTFConfig.cpp:80
>> +#if HAVE(VM_FLAGS_PERMANENT) && PLATFORM(IOS_FAMILY_SIMULATOR)
> 
> I'd add FIXME: Remove the fallback when the oldest host OS that we support for Simulator has VM_FLAGS_PERMANENT.

Sounds good

>> Source/WTF/wtf/WTFConfig.cpp:82
>> +            flags = VM_FLAGS_FIXED | VM_FLAGS_OVERWRITE;
> 
> It should be safer against potential future code modifications to clear VM_FLAGS_PERMANENT here, not to double-guess what it was set to ten lines above.

Sounds good
Comment 5 Saam Barati 2021-03-23 13:12:58 PDT
Created attachment 424054 [details]
patch for landing
Comment 6 EWS 2021-03-23 13:55:25 PDT
Committed r274898: <https://commits.webkit.org/r274898>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 424054 [details].