Bug 185218

Summary: Strings should not be allocated in a gigacage
Product: WebKit Reporter: Filip Pizlo <fpizlo>
Component: bmallocAssignee: Filip Pizlo <fpizlo>
Status: RESOLVED FIXED    
Severity: Normal CC: alecflett, beidson, benjamin, cdumez, cmarcelo, dbates, esprehn+autocc, ews-watchlist, ggaren, gyuyoung.kim, jsbell, keith_miller, mark.lam, msaboff, saam, simon.fraser, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
See Also: https://bugs.webkit.org/show_bug.cgi?id=174924
Bug Depends on:    
Bug Blocks: 174917    
Attachments:
Description Flags
possible patch
none
the patch
none
the patch
saam: review+
patch for landing none

Description Filip Pizlo 2018-05-02 14:48:03 PDT 
Caging strings only protects us from string-based read gadgets.  It's also a slow-down on some benchmarks and a memory use regression.  The benefit/cost isn't big enough to keep this around so we should go back to allocating strings the normal way.
Comment 1 Filip Pizlo 2018-05-02 14:49:12 PDT 
Created attachment 339347 [details]
possible patch
Comment 2 Filip Pizlo 2018-05-03 12:08:34 PDT 
Created attachment 339437 [details]
the patch
Comment 3 Filip Pizlo 2018-05-03 14:23:10 PDT 
Created attachment 339465 [details]
the patch
Comment 4 Saam Barati 2018-05-03 14:44:45 PDT 
Comment on attachment 339465 [details]
the patch

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

> Source/bmalloc/bmalloc/Gigacage.h:38
>  #define PRIMITIVE_GIGACAGE_SIZE 0x80000000llu
>  #define JSVALUE_GIGACAGE_SIZE 0x40000000llu

Might be worth expanding one (or both) of these
Comment 5 Filip Pizlo 2018-05-03 14:50:01 PDT 
Created attachment 339470 [details]
patch for landing
Comment 6 Filip Pizlo 2018-05-03 17:38:52 PDT 
(In reply to Saam Barati from comment #4)
> Comment on attachment 339465 [details]
> the patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=339465&action=review
> 
> > Source/bmalloc/bmalloc/Gigacage.h:38
> >  #define PRIMITIVE_GIGACAGE_SIZE 0x80000000llu
> >  #define JSVALUE_GIGACAGE_SIZE 0x40000000llu
> 
> Might be worth expanding one (or both) of these

Yeah.  Or give them separate runways.
Comment 7 Filip Pizlo 2018-05-03 17:40:41 PDT 
Landed in https://trac.webkit.org/changeset/231337/webkit
Comment 8 Yusuke Suzuki 2018-05-03 18:50:52 PDT 
Committed r231344: <https://trac.webkit.org/changeset/231344>
Comment 9 Radar WebKit Bug Importer 2018-05-03 18:51:23 PDT 
<rdar://problem/39963637>