Bug 175366

Summary: FTL's compileGetTypedArrayByteOffset needs to do caging
Product: WebKit Reporter: Filip Pizlo <fpizlo>
Component: JavaScriptCoreAssignee: Filip Pizlo <fpizlo>
Status: RESOLVED FIXED    
Severity: Normal CC: ggaren, jfbastien, keith_miller, mark.lam, msaboff, saam, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 174917    
Attachments:
Description Flags
maybe the patch
none
the patch saam: review+

Description Filip Pizlo 2017-08-08 22:08:17 PDT
I think that for primitive gigacage, it has to do it.  The tricky thing about it is that it needs to handle null vectors.
Comment 1 Filip Pizlo 2017-08-12 11:55:29 PDT
Created attachment 317998 [details]
maybe the patch
Comment 2 Filip Pizlo 2017-08-12 12:26:45 PDT
Created attachment 317999 [details]
the patch
Comment 3 Saam Barati 2017-08-12 15:37:53 PDT
Comment on attachment 317999 [details]
the patch

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

> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:11653
> +        LBasicBlock lastNext = m_out.insertNewBlocksBefore(notNull);

Why this instead of relying on the result of appendTo below?
Comment 4 Filip Pizlo 2017-08-12 18:56:03 PDT
(In reply to Saam Barati from comment #3)
> Comment on attachment 317999 [details]
> the patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=317999&action=review
> 
> > Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:11653
> > +        LBasicBlock lastNext = m_out.insertNewBlocksBefore(notNull);
> 
> Why this instead of relying on the result of appendTo below?

That way, the code that we insert at the end of the current block will do the right thing if it also contains a control flow diamond.

Using the result of appendTo() is only safe if you are absolutely sure that none of the things you did before newBlock and appendTo added any other blocks.  Therefore, although I used to use the result of appendTo() a lot, I don't think it's actually safe.
Comment 5 Filip Pizlo 2017-08-12 18:59:32 PDT
Landed in https://trac.webkit.org/changeset/220628/webkit
Comment 6 Radar WebKit Bug Importer 2017-08-12 18:59:58 PDT
<rdar://problem/33865694>