Closed Bug 893684 Opened 10 years ago Closed 10 years ago

OdinMonkey: Assertion failure: (size_t) (dst - src) >= len, at jsutil.h


(Core :: JavaScript Engine, defect)

Not set



Tracking Status
firefox22 --- wontfix
firefox23 + fixed
firefox24 + fixed
firefox25 + fixed
firefox-esr17 --- unaffected
b2g18 --- unaffected


(Reporter: gkw, Assigned: luke)



(5 keywords, Whiteboard: [jsbugmon:][adv-main23+] fixed on trunk in bug 880538)


(2 files)

Attached file stack
    valueOf = (function() {\
        \"use asm\";\
        function f() {}\
        return f\
    this + ''\
", {
    fileName: '',
this + ''

asserts js debug shell on m-c changeset 3433a021847b without any CLI arguments intermittently (but fairly reliably) at Assertion failure: (size_t) (dst - src) >= len, at jsutil.h

Locking s-s because previously-found similar assertion bug 855442 was also s-s.

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
user:        Brian Hackett
date:        Wed Jul 10 09:29:52 2013 -0600
summary:     Bug 885758 - Add ExclusiveContext for use by threads with exclusive access to their compartment, r=billm.

Brian, is bug 885758 a likely regressor?
Flags: needinfo?(bhackett1024)
Whiteboard: [jsbugmon:update] → [jsbugmon:]
JSBugMon: Cannot process bug: Unable to automatically reproduce, please track manually.
Nope, silly pre-existing Odin bug.
Flags: needinfo?(bhackett1024)
The problem here is PostFailureLinkInfo stores a copy of the CompileOptions which stores a (dangling) pointer to filename (I should have caught this in review).  The patches in bug 880538 stop doing this and reconstruct the CompileOptions using the filename stored in ScriptSource.  That should be getting landed soon, so I'll wait until that and we can verify this problem is fixed.
Attached patch fixSplinter Review
Actually the fix is super-trivial, no need to wait for the bigger patch.  I'd like to wait to land the testcase until the fix is in release, though, since this is pretty exploitable.
Assignee: general → luke
Attachment #776120 - Flags: review?
Attachment #776120 - Flags: review? → review?(bbouvier)
Comment on attachment 776120 [details] [diff] [review]

Review of attachment 776120 [details] [diff] [review]:

Love it when patches are simple like that.
Attachment #776120 - Flags: review?(bbouvier) → review+
Bug 880538 just landed on inbound - is this patch still needed?
Flags: needinfo?(luke)
Heh, you beat me to commenting: no.
Flags: needinfo?(luke)
What does that mean for 23/24?  will we want this as a smaller, lower risk fix on those branches?
We could, if you think it is a good idea.  (It having not been discovered externally yet, I was thinking landing the smaller fix only drew attention.)
Flags: needinfo?(luke)
Depends on: 880538
Whiteboard: [jsbugmon:] → [jsbugmon:] fixed on trunk in bug 880538
Comment on attachment 776120 [details] [diff] [review]

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 851421
User impact if declined: exploitable bug
Testing completed (on m-c, etc.): a bigger version of this patch is on m-c
Risk to taking this patch (and alternatives if risky): low
Attachment #776120 - Flags: approval-mozilla-beta?
Attachment #776120 - Flags: approval-mozilla-aurora?
Flags: needinfo?(luke)
Attachment #776120 - Flags: approval-mozilla-beta?
Attachment #776120 - Flags: approval-mozilla-beta+
Attachment #776120 - Flags: approval-mozilla-aurora?
Attachment #776120 - Flags: approval-mozilla-aurora+
(Please land the patch only on Aurora and Beta, thanks!)
Whiteboard: [jsbugmon:] fixed on trunk in bug 880538 → [jsbugmon:][adv-main23+] fixed on trunk in bug 880538
Group: core-security
You need to log in before you can comment on or make changes to this bug.