Bug 185218 - Strings should not be allocated in a gigacage
Summary: Strings should not be allocated in a gigacage
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: bmalloc (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Filip Pizlo
URL:
Keywords: InRadar
Depends on:
Blocks: 174917
  Show dependency tree / graph
 
Reported: 2018-05-02 14:48 PDT by Filip Pizlo
Modified: 2018-05-03 18:51 PDT (History)
18 users (show)

See Also:

Attachments
possible patch (85.74 KB, patch)
2018-05-02 14:49 PDT, Filip Pizlo 		no flags Details | Formatted Diff | Diff
the patch (97.26 KB, patch)
2018-05-03 12:08 PDT, Filip Pizlo 		no flags Details | Formatted Diff | Diff
the patch (97.89 KB, patch)
2018-05-03 14:23 PDT, Filip Pizlo 		saam: review+
Details | Formatted Diff | Diff
patch for landing (97.93 KB, patch)
2018-05-03 14:50 PDT, Filip Pizlo 		no flags Details | Formatted Diff | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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>