Bug 84513 - Add an ability to find the extent of a callframe
Summary: Add an ability to find the extent of a callframe
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Oliver Hunt
URL:
Keywords:
Depends on:
Blocks: 79668
  Show dependency treegraph
 
Reported: 2012-04-20 16:50 PDT by Oliver Hunt
Modified: 2012-04-24 21:06 PDT (History)
11 users (show)

See Also:


Attachments
Patch (3.33 KB, patch)
2012-04-20 16:57 PDT, Oliver Hunt
fpizlo: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Oliver Hunt 2012-04-20 16:50:29 PDT
Add an ability to find the extent of a callframe
Comment 1 Oliver Hunt 2012-04-20 16:57:07 PDT
Created attachment 138202 [details]
Patch
Comment 2 Oliver Hunt 2012-04-20 17:12:30 PDT
Committed r114803: <http://trac.webkit.org/changeset/114803>
Comment 3 Csaba Osztrogonác 2012-04-21 00:56:29 PDT
Reopen, because it broke zillion jsc tests and layout tests in debug mode on the GTK and on the Qt x86_64 bots. Maybe they are broken on Mac too, but unfortunately Mac debug bots aren't working properly now.


- http://build.webkit.org/builders/GTK%20Linux%2064-bit%20Debug/builds/32645
- http://build.webkit.sed.hu/builders/x86-64%20Linux%20Qt%20Debug/builds/22391

Could you check and fix this regression please?
Comment 4 Thiago Marcos P. Santos 2012-04-21 04:03:03 PDT
Hi guys, looks like all debug build (EFL, GTK, Win, etc) bots are sick after this change. Release bots are passing on these tests.

http://build.webkit.org/builders/EFL%20Linux%20Debug/builds/379

** Danger, Will Robinson! Danger! The following failures have been introduced:
	ecma/Expressions/11.2.2-1-n.js
	ecma/Expressions/11.2.2-10-n.js
	ecma/Expressions/11.2.2-7-n.js
	ecma/Expressions/11.2.2-8-n.js
	ecma/Expressions/11.2.2-9-n.js
	ecma/GlobalObject/15.1-1-n.js
	ecma/GlobalObject/15.1-2-n.js
	ecma/Math/15.8-2-n.js
	ecma/Math/15.8-3-n.js
	ecma_2/Exceptions/exception-001.js
	ecma_2/Exceptions/exception-002.js
	ecma_2/Exceptions/expression-005.js
	ecma_2/Exceptions/expression-006.js
	ecma_2/Exceptions/expression-012.js
	ecma_2/Exceptions/expression-013.js
	ecma_2/Exceptions/expression-014.js
	ecma_2/Exceptions/expression-019.js
	ecma_2/Exceptions/global-001.js
	ecma_2/Exceptions/global-002.js
	js1_2/function/regexparg-2-n.js
	js1_4/Regress/function-004-n.js
Comment 5 Philippe Normand 2012-04-23 08:03:46 PDT
Hi Oliver, any idea about this issue?
Comment 6 Oliver Hunt 2012-04-23 08:53:32 PDT
I'm looking at this today -- the assertion that's firing is *very* bad.  It's actually showing up on a number of other sites in mac (just not on the test suite :-/ ).  This is currently my #1 priority.
Comment 7 Dominik Röttsches (drott) 2012-04-23 08:57:55 PDT
Thanks for getting back on this - if it's taking longer than a day - do you think we could roll this out until the sites are fixed that hit this assertion?

Reason is, this hits the "20 crashes to bailout" threshold on our (EFL Linux Debug) buildbot. Similar for GTK Debug Buildbot. So we can't use the results.
Comment 8 Oliver Hunt 2012-04-23 10:59:45 PDT
Ah ha, if LLINT isn't enabled, i get crashes running webkit tests on mac.  I assume effected builds have llint disabled.  Investigating some more.
Comment 9 Oliver Hunt 2012-04-23 11:30:20 PDT
It looks like there are a few places in the baseline jit that mix-and-match the current callframe and the caller's callframe.  Fixing.
Comment 10 Oliver Hunt 2012-04-23 11:41:52 PDT
Running tests on my patch, then some tidying up and should be good.  Then we'll just be stuck with the horrible bug that i originally added these assertions to catch.
Comment 11 Thiago Marcos P. Santos 2012-04-23 12:12:19 PDT
(In reply to comment #10)
> Running tests on my patch, then some tidying up and should be good.  Then we'll just be stuck with the horrible bug that i originally added these assertions to catch.

I'm still online for a couple of hours if you need a second opinion (i.e. run your patch/fix on other port).
Comment 12 Dominik Röttsches (drott) 2012-04-24 00:20:26 PDT
Debug Buildbots are back to work after Oliver removed one assertion:
http://trac.webkit.org/changeset/114953
Comment 13 Philippe Normand 2012-04-24 00:32:54 PDT
(In reply to comment #10)
> Running tests on my patch, then some tidying up and should be good.  Then we'll just be stuck with the horrible bug that i originally added these assertions to catch.

Thanks Oliver! Bots are bleeding less now :) This bug can be closed now, maybe?