Bug 139572 – Need a regression test for bug 139533

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Michael Saboff]

The fix for https://bugs.webkit.org/show_bug.cgi?id=139533 needs a regression test.

[Michael Saboff]

Created attachment 243168 [details]
New Test

[Mark Lam]

Comment on attachment 243168 [details]
New Test

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

r=me with fixes.

> LayoutTests/js/script-tests/regress-139533.js:9
> +	return arg + obj.addend;

Bad indentation here.

> LayoutTests/js/script-tests/regress-139533.js:32
> +    // After tiering up into the DFG, change the "addend" of obj.  This will do two things:
> +    // 1) We should OSR exit with a BadType (addend is no longer an integer)
> +    // 2) In the next call to inner, we will call jsAddSlowCase which will make a 
> +    //    native call to get the default value of obj.addend.
> +    // The OSR exit handler will not restore the ScopeChain slot in the header and the inlining
> +    // should have overwritten inner's ScopeChain slot with something else.

Down the road, this could be misread as how things work then.  I think you should add a comment here indicating how the fix for 139533 makes this not crash anymore.

[Michael Saboff]

(In reply to comment #2)
> Comment on attachment 243168 [details]
> New Test
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=243168&action=review
> 
> r=me with fixes.
> 
> > LayoutTests/js/script-tests/regress-139533.js:9
> > +	return arg + obj.addend;
> 
> Bad indentation here.

Changed the tab to spaces.

> > LayoutTests/js/script-tests/regress-139533.js:32
> > +    // After tiering up into the DFG, change the "addend" of obj.  This will do two things:
> > +    // 1) We should OSR exit with a BadType (addend is no longer an integer)
> > +    // 2) In the next call to inner, we will call jsAddSlowCase which will make a 
> > +    //    native call to get the default value of obj.addend.
> > +    // The OSR exit handler will not restore the ScopeChain slot in the header and the inlining
> > +    // should have overwritten inner's ScopeChain slot with something else.
> 
> Down the road, this could be misread as how things work then.  I think you
> should add a comment here indicating how the fix for 139533 makes this not
> crash anymore.

I added a little more details.

[Michael Saboff]

Committed r177203: <http://trac.webkit.org/changeset/177203>