Bug 180306 - Try proxying all function arguments
Summary: Try proxying all function arguments
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: JF Bastien
URL:
Keywords: InRadar
Depends on: 180297
Blocks: 180375
  Show dependency treegraph
 
Reported: 2017-12-01 21:58 PST by JF Bastien
Modified: 2017-12-04 13:22 PST (History)
11 users (show)

See Also:


Attachments
patch (3.52 KB, patch)
2017-12-01 22:15 PST, JF Bastien
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description JF Bastien 2017-12-01 21:58:42 PST
In bug #180297 we found that some of the Math functions weren't doing their exception checks. Enumerate all functions and try calling all of them to see if there's anything else that fails.
Comment 1 JF Bastien 2017-12-01 22:15:22 PST
Created attachment 328212 [details]
patch

Everything passes!
Comment 2 WebKit Commit Bot 2017-12-01 22:48:31 PST
Comment on attachment 328212 [details]
patch

Clearing flags on attachment: 328212

Committed r225444: <https://trac.webkit.org/changeset/225444>
Comment 3 WebKit Commit Bot 2017-12-01 22:48:33 PST
All reviewed patches have been landed.  Closing bug.
Comment 4 Radar WebKit Bug Importer 2017-12-01 22:49:25 PST
<rdar://problem/35812613>
Comment 5 Michael Catanzaro 2017-12-04 12:07:41 PST
All the new tests are crashing for WPE and GTK:


** The following JSC stress test failures have been introduced:
	stress/proxy-all-the-parameters.js.default
	stress/proxy-all-the-parameters.js.dfg-eager
	stress/proxy-all-the-parameters.js.dfg-eager-no-cjit-validate
	stress/proxy-all-the-parameters.js.dfg-maximal-flush-validate-no-cjit
	stress/proxy-all-the-parameters.js.ftl-eager
	stress/proxy-all-the-parameters.js.ftl-eager-no-cjit
	stress/proxy-all-the-parameters.js.ftl-eager-no-cjit-b3o1
	stress/proxy-all-the-parameters.js.ftl-no-cjit-b3o1
	stress/proxy-all-the-parameters.js.ftl-no-cjit-no-inline-validate
	stress/proxy-all-the-parameters.js.ftl-no-cjit-no-put-stack-validate
	stress/proxy-all-the-parameters.js.ftl-no-cjit-small-pool
	stress/proxy-all-the-parameters.js.ftl-no-cjit-validate-sampling-profiler
	stress/proxy-all-the-parameters.js.no-cjit-collect-continuously
	stress/proxy-all-the-parameters.js.no-cjit-validate-phases
	stress/proxy-all-the-parameters.js.no-ftl
	stress/proxy-all-the-parameters.js.no-llint

Results for JSC stress tests:
    16 failures found.


Let's see if I can figure out how to run the tests and get a backtrace.
Comment 6 JF Bastien 2017-12-04 12:32:57 PST
(In reply to Michael Catanzaro from comment #5)
> All the new tests are crashing for WPE and GTK:
> 
> 
> ** The following JSC stress test failures have been introduced:
> 	stress/proxy-all-the-parameters.js.default
> 	stress/proxy-all-the-parameters.js.dfg-eager
> 	stress/proxy-all-the-parameters.js.dfg-eager-no-cjit-validate
> 	stress/proxy-all-the-parameters.js.dfg-maximal-flush-validate-no-cjit
> 	stress/proxy-all-the-parameters.js.ftl-eager
> 	stress/proxy-all-the-parameters.js.ftl-eager-no-cjit
> 	stress/proxy-all-the-parameters.js.ftl-eager-no-cjit-b3o1
> 	stress/proxy-all-the-parameters.js.ftl-no-cjit-b3o1
> 	stress/proxy-all-the-parameters.js.ftl-no-cjit-no-inline-validate
> 	stress/proxy-all-the-parameters.js.ftl-no-cjit-no-put-stack-validate
> 	stress/proxy-all-the-parameters.js.ftl-no-cjit-small-pool
> 	stress/proxy-all-the-parameters.js.ftl-no-cjit-validate-sampling-profiler
> 	stress/proxy-all-the-parameters.js.no-cjit-collect-continuously
> 	stress/proxy-all-the-parameters.js.no-cjit-validate-phases
> 	stress/proxy-all-the-parameters.js.no-ftl
> 	stress/proxy-all-the-parameters.js.no-llint
> 
> Results for JSC stress tests:
>     16 failures found.
> 
> 
> Let's see if I can figure out how to run the tests and get a backtrace.

When you run locally can you set verbose and check what function is run before the failure? You might be injecting a function that we shouldn’t call into the globally namespace. And this test will call it :)
Comment 7 Michael Catanzaro 2017-12-04 13:14:28 PST
(In reply to Michael Catanzaro from comment #5)
> Let's see if I can figure out how to run the tests and get a backtrace.

They all pass on my personal machine. Sigh.

Yuskue, is this something you might want to look into?
Comment 8 JF Bastien 2017-12-04 13:16:42 PST
(In reply to Michael Catanzaro from comment #7)
> (In reply to Michael Catanzaro from comment #5)
> > Let's see if I can figure out how to run the tests and get a backtrace.
> 
> They all pass on my personal machine. Sigh.
> 
> Yuskue, is this something you might want to look into?

Wild guess is that it's calling some internal function, and if I filter out objects whose name starts with "$" we'll be good. I'll upload a patch to do so.
Comment 9 Michael Catanzaro 2017-12-04 13:17:59 PST
(In reply to Michael Catanzaro from comment #7)
> (In reply to Michael Catanzaro from comment #5)
> > Let's see if I can figure out how to run the tests and get a backtrace.
> 
> They all pass on my personal machine. Sigh.

Um, actually I think I was not running the tests properly. They completed instantaneously. But now I've tried passing more arguments to run-jsc-tests, just like our bots do, and it's no longer completing immediately.
Comment 10 Michael Catanzaro 2017-12-04 13:18:16 PST
(In reply to JF Bastien from comment #8)
> Wild guess is that it's calling some internal function, and if I filter out
> objects whose name starts with "$" we'll be good. I'll upload a patch to do
> so.

Happy to test it.
Comment 11 JF Bastien 2017-12-04 13:22:37 PST
Will try to address this in https://bugs.webkit.org/show_bug.cgi?id=180375