Last Comment Bug 572232 - js_InvokeConstructor with clampReturn==JS_TRUE and user-controlled ctor is a gc hazard
: js_InvokeConstructor with clampReturn==JS_TRUE and user-controlled ctor is a ...
: assertion, regression, testcase
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All All
-- critical (vote)
: ---
Assigned To: Jeff Walden [:Waldo] (remove +bmo to email)
: Jason Orendorff [:jorendorff]
Depends on: 567577
Blocks: jsfunfuzz 516827
  Show dependency treegraph
Reported: 2010-06-15 15:56 PDT by Jeff Walden [:Waldo] (remove +bmo to email)
Modified: 2013-03-20 08:36 PDT (History)
15 users (show)
choller: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Branch patch (615 bytes, patch)
2010-07-27 17:18 PDT, Jeff Walden [:Waldo] (remove +bmo to email)
dvander: review+
christian: approval1.9.2.9+
christian: approval1.9.1.12+
Details | Diff | Splinter Review
Patch for 1.9.0 (1.51 KB, patch)
2010-09-03 09:45 PDT, Mike Hommey [:glandium]
dvander: review+
Details | Diff | Splinter Review

Description User image Jeff Walden [:Waldo] (remove +bmo to email) 2010-06-15 15:56:34 PDT
See bug 567577 comment 9 and bug 567577 comment 10.

I *think* we either need to add a tvr for the noted default-return object in js_InvokeConstructor or figure out whether __noSuchMethod__ functionality can ever be invoked in a constructing situation.  The latter is probably hard, and the former is probably easy, which suggests probably the former is what we should do.

This also shows up in JS_New on trunk (but conservative stack scanning saves us) and in fun_applyConstructor (Narcissus-only, so I think we don't care), but neither of these cases needs a change.
Comment 1 User image Brendan Eich [:brendan] 2010-06-15 16:44:38 PDT
Cc'ing mrbkap who was involved in bug 453462, and gal who took over at some point.

Comment 2 User image Robert Sayre 2010-06-22 13:23:29 PDT
this is still biting on branches, even if we get a conservative scanner on trunk, right?
Comment 3 User image Jeff Walden [:Waldo] (remove +bmo to email) 2010-06-22 13:28:23 PDT
Comment 4 User image David Anderson [:dvander] 2010-06-29 13:02:15 PDT
The patch in bug 567577 fixes this, I think.
Comment 5 User image Robert Sayre 2010-06-29 13:12:02 PDT
(In reply to comment #4)
> The patch in bug 567577 fixes this, I think.

"Can we get a little more confidence than 'I Think'?" -- Damon Sicore
Comment 6 User image Andreas Gal :gal 2010-06-29 13:17:15 PDT
Waldo filed it. Waldo should decide. As far as I understand the bug report scanning the stack will find this root, so this is safe now. Waldo?
Comment 7 User image Jeff Walden [:Waldo] (remove +bmo to email) 2010-06-29 15:12:51 PDT
Comment 4 is correct with respect to trunk, even with conservative stack scanning turned off.  We may not want to apply the patch there to branches exactly as is, however, as it does force clamping, which would be a web-visible change.  Just adding the AutoObjectRooter (JSAutoTempValueRooter on branches) should be enough for branches, without compatibility concerns.
Comment 8 User image Robert Sayre 2010-07-13 13:38:43 PDT
Waldo, could you please get a branch patch going?
Comment 9 User image Jeff Walden [:Waldo] (remove +bmo to email) 2010-07-14 11:46:37 PDT
Comment 10 User image Damon Sicore (:damons) 2010-07-27 13:29:20 PDT
Any news on the branch patch?
Comment 11 User image Jeff Walden [:Waldo] (remove +bmo to email) 2010-07-27 17:18:12 PDT
Created attachment 460709 [details] [diff] [review]
Branch patch
Comment 12 User image christian 2010-08-02 15:44:36 PDT
Comment on attachment 460709 [details] [diff] [review]
Branch patch

a=LegNeato for and Please land on mozilla-1.9.2 default and mozilla-1.9.1 default.
Comment 14 User image Mike Hommey [:glandium] 2010-09-03 09:45:03 PDT
Created attachment 471885 [details] [diff] [review]
Patch for 1.9.0
Comment 15 User image Ed Morley [:emorley] 2013-03-20 04:48:57 PDT

Note You need to log in before you can comment on or make changes to this bug.