Bug 213524

Summary: Handle string overflow in DFG graph dump while validating AI.
Product: WebKit Reporter: Mark Lam <mark.lam>
Component: JavaScriptCoreAssignee: Mark Lam <mark.lam>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, cdumez, cmarcelo, darin, ews-watchlist, keith_miller, msaboff, saam, tzagallo, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
proposed patch. saam: review+

Description Mark Lam 2020-06-23 10:49:42 PDT
<rdar://problem/64635620>
Comment 1 Mark Lam 2020-06-23 10:56:49 PDT
Created attachment 402569 [details]
proposed patch.
Comment 2 Mark Lam 2020-06-23 11:16:26 PDT
Thanks for the review.  Landed in r263405: <http://trac.webkit.org/r263405>.
Comment 3 Darin Adler 2020-06-23 11:22:41 PDT
Comment on attachment 402569 [details]
proposed patch.

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

> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:563
> +            auto expectedString = out.tryToString();
> +            m_graphDump = expectedString ? expectedString.value() : String("<out of memory while dumping graph>"_s);

I would have written this using valueOr. I think it possibly be this one-liner:

    m_graphDump = out.tryToString().valueOr("<out of memory while dumping graph>"_s);
Comment 4 Mark Lam 2020-06-23 11:44:07 PDT
(In reply to Darin Adler from comment #3)
> Comment on attachment 402569 [details]
> proposed patch.
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=402569&action=review
> 
> > Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:563
> > +            auto expectedString = out.tryToString();
> > +            m_graphDump = expectedString ? expectedString.value() : String("<out of memory while dumping graph>"_s);
> 
> I would have written this using valueOr. I think it possibly be this
> one-liner:
> 
>     m_graphDump = out.tryToString().valueOr("<out of memory while dumping
> graph>"_s);

Nice.  Will apply this (except with Expected::value_or(); there's no valueOr() method).
Comment 5 Mark Lam 2020-06-23 11:49:53 PDT
Landed follow up in r263408: <http://trac.webkit.org/r263408>.