Closed
Bug 946243
Opened 10 years ago
Closed 10 years ago
Crash with SIGTRAP and prototype
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
People
(Reporter: decoder, Assigned: h4writer)
References
Details
(Keywords: crash, regression, testcase, Whiteboard: [jsbugmon:update][qa-])
Attachments
(1 file)
1.13 KB,
patch
|
jandem
:
review+
lsblakk
:
approval-mozilla-aurora+
lsblakk
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
The following testcase crashes on mozilla-central revision 9688476c1544 (run with --fuzzing-safe --ion-eager): function $ERROR(message) {} function Foo() { var x = this.property; glob = x; } Foo.prototype.property = 10; for (var i = 0; i < 10; i++) { new Foo(); Foo ($ERROR, 10); }
Reporter | ||
Updated•10 years ago
|
Whiteboard: [jsbugmon:update,bisect]
Reporter | ||
Updated•10 years ago
|
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]
Reporter | ||
Comment 1•10 years ago
|
||
JSBugMon: Bisection requested, failed due to error (try manually).
Comment 2•10 years ago
|
||
autoBisect shows this is probably related to the following changeset: The first bad revision is: changeset: http://hg.mozilla.org/mozilla-central/rev/39bfcadd6492 user: Hannes Verschore date: Tue Nov 26 23:21:18 2013 +0100 summary: Bug 942105 - IonMonkey: Remove the inlineUseCountRatio option, r=jandem Hannes, is bug 942105 a likely regressor?
Blocks: 942105
Flags: needinfo?(hv1989)
Updated•10 years ago
|
Keywords: regression
Assignee | ||
Comment 3•10 years ago
|
||
(In reply to Gary Kwong [:gkw] [:nth10sd] from comment #2) > Hannes, is bug 942105 a likely regressor? There is a big chance it isn't the regressor. We are inlining more cases by removing that. Could be an edgecase that already existed from before. Gonna have a quick look.
Flags: needinfo?(hv1989)
Assignee | ||
Comment 4•10 years ago
|
||
Using bug 947188, I see: jit/BaselineIC.cpp:8287 Failed to return object in constructing call. So I'll add djvj since this breakpoint is in baseline.
Updated•10 years ago
|
Flags: needinfo?(kvijayan)
Assignee | ||
Comment 6•10 years ago
|
||
During "EliminateDeadResumePointOperands" we eliminate the value of "this", if the value is MComputeThis. In other cases "this" could only be MParameter or MTypeBarrier (I think). That is covered. I now did the easy fix, by just adding MComputeThis to the ignore list of "EliminateDeadResumePointOperands". Not sure if we want more extensive and less error prone code. I'm ok with that. Let's discuss! (But plz in other bug. As I want to backport this 'easy' fix)
Assignee | ||
Comment 7•10 years ago
|
||
Assignee: general → hv1989
Attachment #8344412 -
Flags: review?(jdemooij)
Comment 8•10 years ago
|
||
Comment on attachment 8344412 [details] [diff] [review] Patch Review of attachment 8344412 [details] [diff] [review]: ----------------------------------------------------------------- Ugh, thanks for fixing this.
Attachment #8344412 -
Flags: review?(jdemooij) → review+
Assignee | ||
Updated•10 years ago
|
Assignee | ||
Comment 9•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/6683ed2feb75
Comment 10•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/6683ed2feb75 Given that this doesn't have a sec rating and affects multiple branches, it shouldn't have landed without security approval.
Status: NEW → RESOLVED
Closed: 10 years ago
status-b2g-v1.2:
--- → affected
status-firefox29:
--- → fixed
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
Comment 11•10 years ago
|
||
This landed prior to the merge (from m-c to aurora), but at landing, there was not enough time for a inbound to m-c merge to make the aurora merge. (It seemed to only affect version 28, 29 didn't "exist" upon landing to inbound, if this makes more sense)
Comment 12•10 years ago
|
||
(In reply to Gary Kwong [:gkw] [:nth10sd] from comment #11) > (It seemed to only affect version 28, 29 didn't "exist" upon landing to > inbound, if this makes more sense) According to Hannes, this affects 26 and 27 too. See the status flags as set after comment 8.
Comment 13•10 years ago
|
||
Oh, you're right. I thought the regressor was bug 942105 as opposed to bug 913424.
Reporter | ||
Updated•10 years ago
|
Status: RESOLVED → VERIFIED
Reporter | ||
Comment 14•10 years ago
|
||
JSBugMon: This bug has been automatically verified fixed.
Reporter | ||
Comment 15•10 years ago
|
||
According to Hannes, this will always cause an int3, nothing else, so this is not a security bug.
Group: core-security
Assignee | ||
Comment 16•10 years ago
|
||
(In reply to Christian Holler (:decoder) from comment #15) > According to Hannes, this will always cause an int3, nothing else, so this > is not a security bug. Oh, just for completeness, in optimized builds this won't cause an int3. It will only give undefined as return value to "new Foo();" in some edge-cases that should be hard to reproduce. But also in that case not a security bug.
Assignee | ||
Comment 17•10 years ago
|
||
Comment on attachment 8344412 [details] [diff] [review] Patch [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 913424 User impact if declined: In some edge edge edge cases returning "undefined" instead of "object" with "new Foo();". Testing completed (on m-c, etc.): m-i for a day Risk to taking this patch (and alternatives if risky): Very low risk. The original code did some optimizations. We now don't do that optimization and take the regular path. String or IDL/UUID changes made by this patch: /
Attachment #8344412 -
Flags: approval-mozilla-beta?
Attachment #8344412 -
Flags: approval-mozilla-aurora?
Updated•10 years ago
|
Attachment #8344412 -
Flags: approval-mozilla-beta?
Attachment #8344412 -
Flags: approval-mozilla-beta+
Attachment #8344412 -
Flags: approval-mozilla-aurora?
Attachment #8344412 -
Flags: approval-mozilla-aurora+
Comment 18•10 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/a77b28e73f60 https://hg.mozilla.org/releases/mozilla-beta/rev/3d2dbc8282b2 Is this something we need on b2g26 (v1.2)?
Assignee | ||
Comment 19•10 years ago
|
||
I assume b2g26 and b2g28 right? If still possible that would be great to get fixed there too.
Flags: needinfo?(hv1989)
Comment 20•10 years ago
|
||
(In reply to Hannes Verschore [:h4writer] from comment #19) > I assume b2g26 and b2g28 right? If still possible that would be great to get > fixed there too. b2g28 = Aurora for this cycle, so that's already taken care of. Can you nominate the patch for b2g26?
Assignee | ||
Comment 21•10 years ago
|
||
Comment on attachment 8344412 [details] [diff] [review] Patch [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 913424 User impact if declined: In some edge edge edge cases returning "undefined" instead of "object" with "new Foo();". Testing completed (on m-c, etc.): m-i for a day Risk to taking this patch (and alternatives if risky): Very low risk. The original code did some optimizations. We now don't do that optimization and take the regular path. String or IDL/UUID changes made by this patch: /
Attachment #8344412 -
Flags: approval-mozilla-b2g26?
Comment 22•10 years ago
|
||
I don't think this needs QA verification. If anyone thinks that's a mistake please remove the [qa-] whiteboard tag and add the verifyme keyword.
Whiteboard: [jsbugmon:update] → [jsbugmon:update][qa-]
Assignee | ||
Comment 23•10 years ago
|
||
Who do I need to poke for approval-mozila-b2g26 ??
Comment 24•10 years ago
|
||
Trying nominating for koi? - some B2G folk might look at this.
blocking-b2g: --- → koi?
Comment 25•10 years ago
|
||
(In reply to Gary Kwong [:gkw] [:nth10sd] from comment #24) > Trying nominating for koi? - some B2G folk might look at this. What's the critical need to block on this? I am seeing edge of edge case, which implies a non-blocker.
Comment 26•10 years ago
|
||
> What's the critical need to block on this? I am seeing edge of edge case, > which implies a non-blocker. (In reply to Hannes Verschore [:h4writer] from comment #19) > I assume b2g26 and b2g28 right? If still possible that would be great to get > fixed there too. Hannes would be the right person for this.
Flags: needinfo?(hv1989)
Assignee | ||
Comment 27•10 years ago
|
||
(In reply to Jason Smith [:jsmith] from comment #25) > What's the critical need to block on this? I am seeing edge of edge case, > which implies a non-blocker. I'm not sure what the policy is for b2g26. I know it is an edgecase, but than again, b2g is only executing JS. Making it use it extensively. So could potentially cause strange behaviour for b2g. Now the fix is very straightforward and definitely without side-effects or potential regressions. It's a one-line patch too. So if we still take correction patches for b2g26, this would be a easy fix for potential very strange and hard to debug issues.
Flags: needinfo?(hv1989)
Comment 28•10 years ago
|
||
Historically for branches (even ESR) we've taken small fixes for fuzzing bugs no matter how edge-case they are, because should they pop out in real life eventually (which they have), it takes up an immense amount of developer resources to triage, reduce and debug the testcase only to find that they're the same issue. (which is a waste of time) Not sure if this applies to b2g26 (or any other b2g branch), but I really don't see a difference.
Updated•10 years ago
|
Attachment #8344412 -
Flags: approval-mozilla-b2g26?
Comment 30•10 years ago
|
||
https://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/7b521834610a
status-b2g-v1.3:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•