Bug 195924

Summary: FTL: Emit code to validate AI's state when running the compiled code
Product: WebKit Reporter: Saam Barati <saam>
Component: JavaScriptCoreAssignee: Saam Barati <saam>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, commit-queue, ews-watchlist, fpizlo, ggaren, gskachkov, guijemont, keith_miller, mark.lam, msaboff, rmorisset, ticaiolima, tzagallo, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=195980
https://bugs.webkit.org/show_bug.cgi?id=195981
https://bugs.webkit.org/show_bug.cgi?id=196030
Attachments:
Description Flags
WIP
none
WIP
none
WIP
none
WIP
none
patch
none
WIP
none
WIP
none
WIP
none
patch
none
patch none

Description Saam Barati 2019-03-18 18:23:52 PDT
...
Comment 1 Radar WebKit Bug Importer 2019-03-18 18:40:56 PDT
<rdar://problem/49003422>
Comment 2 Saam Barati 2019-03-19 16:19:35 PDT
Created attachment 365252 [details]
WIP
Comment 3 Saam Barati 2019-03-19 16:24:05 PDT
Created attachment 365255 [details]
WIP
Comment 4 Saam Barati 2019-03-19 16:40:09 PDT
Created attachment 365261 [details]
WIP
Comment 5 Saam Barati 2019-03-19 16:51:19 PDT
seems possible it's already found some bugs...
Comment 6 Saam Barati 2019-03-19 17:10:28 PDT
(In reply to Saam Barati from comment #5)
> seems possible it's already found some bugs...

Perhaps not. It seems like using combined liveness may not work, since AI only tracks live in IR values.
Comment 7 Saam Barati 2019-03-19 17:14:47 PDT
Created attachment 365267 [details]
WIP
Comment 8 Saam Barati 2019-03-19 18:12:26 PDT
It found a bug:

```
    case ValueBitXor:
    case ValueBitAnd:
    case ValueBitOr:
        if (node->binaryUseKind() == BigIntUse)
            setTypeForNode(node, SpecBigInt);
        else {
            clobberWorld();
            setTypeForNode(node, SpecBoolInt32 | SpecBigInt);
        }
        break;

```

Should be:
```
    case ValueBitXor:
    case ValueBitAnd:
    case ValueBitOr:
        if (node->binaryUseKind() == BigIntUse)
            setTypeForNode(node, SpecBigInt);
        else {
            clobberWorld();
            setTypeForNode(node, SpecInt32Only | SpecBigInt);
        }
        break;
````
Comment 9 Saam Barati 2019-03-19 18:46:03 PDT
Created attachment 365284 [details]
patch

WIP
Comment 10 Saam Barati 2019-03-19 19:21:31 PDT
Created attachment 365288 [details]
WIP
Comment 11 Saam Barati 2019-03-19 19:38:56 PDT
Created attachment 365293 [details]
WIP
Comment 12 Saam Barati 2019-03-20 18:30:47 PDT
Created attachment 365457 [details]
WIP
Comment 13 Saam Barati 2019-03-25 15:44:38 PDT
Created attachment 365910 [details]
patch
Comment 14 Saam Barati 2019-03-25 16:01:17 PDT
Comment on attachment 365910 [details]
patch

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

> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:177
> +                        live.add(child.node());

I’ll also make this addVoid
Comment 15 EWS Watchlist 2019-03-25 16:02:56 PDT
Attachment 365910 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/ChangeLog:12:  Please consider whether the use of security-sensitive phrasing could help someone exploit WebKit: fuzzer  [changelog/unwantedsecurityterms] [3]
Total errors found: 1 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 16 Saam Barati 2019-03-25 16:57:27 PDT
Comment on attachment 365910 [details]
patch

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

> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:597
> +                input = boxDouble(lowDouble(Edge(node, DoubleRepUse)));

this logic is somewhat wrong for doubles. I think I need to validate them unboxed.
Comment 17 Saam Barati 2019-03-25 17:23:40 PDT
Created attachment 365926 [details]
patch
Comment 18 EWS Watchlist 2019-03-25 17:27:49 PDT
Attachment 365926 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/ChangeLog:12:  Please consider whether the use of security-sensitive phrasing could help someone exploit WebKit: fuzzer  [changelog/unwantedsecurityterms] [3]
Total errors found: 1 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 19 WebKit Commit Bot 2019-03-26 17:08:52 PDT
Comment on attachment 365926 [details]
patch

Clearing flags on attachment: 365926

Committed r243530: <https://trac.webkit.org/changeset/243530>
Comment 20 WebKit Commit Bot 2019-03-26 17:08:54 PDT
All reviewed patches have been landed.  Closing bug.