Bug 168574

Summary:

functionTestWasmModuleFunctions() should use a MarkedArgumentBuffer for storing args instead of a Vector.

Product:

WebKit

Reporter:

Mark Lam <mark.lam>

Component:

JavaScriptCore

Assignee:

Mark Lam <mark.lam>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

fpizlo, ggaren, jfbastien, keith_miller, msaboff, saam

Priority:

Version:

WebKit Local Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
proposed patch.	fpizlo: review+

Description Mark Lam 2017-02-19 08:17:40 PST

Vectors don't tell GC about the need to scan the args.

Comment 1 JF Bastien 2017-02-19 13:44:23 PST

I think we're better off getting rid of the function entirely, now that we have the JS builder.

Comment 2 Mark Lam 2017-02-19 13:54:12 PST

(In reply to comment #1)
> I think we're better off getting rid of the function entirely, now that we
> have the JS builder.

Is functionTestWasmModuleFunctions() no longer needed?

Comment 3 JF Bastien 2017-02-19 13:56:07 PST

(In reply to comment #2)
> (In reply to comment #1)
> > I think we're better off getting rid of the function entirely, now that we
> > have the JS builder.
> 
> Is functionTestWasmModuleFunctions() no longer needed?

IIRC we need to update some tests to use the JS builder instead of that function. It's still needed until we update the tests.

Comment 4 Mark Lam 2017-02-19 19:53:45 PST

Created attachment 302114 [details]
proposed patch.

Comment 5 Mark Lam 2017-02-19 20:02:18 PST

Thanks for the review.  Landed in r212620: <http://trac.webkit.org/r212620>.

Comment 6 Keith Miller 2017-02-20 09:27:34 PST

Yeah, I'm pretty sure we don't have tests that use this function anymore. Also, as a note, we don't really need to use a MarkedArgumentBuffer since we can't put JSCells into that vector so the GC won't see anything of interest anyway. The only reason the contents of that vector were JSValues is to avoid casting when we did vmEntryToJavaScript.