Bug 175366 - FTL's compileGetTypedArrayByteOffset needs to do caging
Summary: FTL's compileGetTypedArrayByteOffset needs to do caging
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Filip Pizlo
URL:
Keywords: InRadar
Depends on:
Blocks: 174917
  Show dependency treegraph
 
Reported: 2017-08-08 22:08 PDT by Filip Pizlo
Modified: 2017-08-12 18:59 PDT (History)
8 users (show)

See Also:


Attachments
maybe the patch (5.85 KB, patch)
2017-08-12 11:55 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
the patch (6.01 KB, patch)
2017-08-12 12:26 PDT, Filip Pizlo
sbarati: review+
Details | Formatted Diff | Diff

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