"Assertion failure: chars >= base->chars() && chars < base->chars() + base->length(),"

VERIFIED FIXED in mozilla19

Status

()

--
critical
VERIFIED FIXED
6 years ago
6 years ago

People

(Reporter: gkw, Assigned: terrence)

Tracking

(Blocks: 1 bug, {assertion, regression, testcase})

Trunk
mozilla19
x86_64
Mac OS X
assertion, regression, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [jsbugmon:update])

Attachments

(2 attachments)

(Reporter)

Description

6 years ago
Created attachment 678224 [details]
stack

pSandbox = newGlobal('new-compartment');
evalcx("\
    x = ArrayBuffer;\
    y = Map();\
    x += 1;\
    w = x;\
    x += '0';\
    z = x;\
", pSandbox);
evalcx("\
    x + '0';\
", pSandbox);
evalcx("\
    y.delete(z);\
    w.slice(2);\
", pSandbox)

asserts js debug shell on m-c changeset 2937fd8e35a1 without any CLI arguments at Assertion failure: chars >= base->chars() && chars < base->chars() + base->length(),

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   107556:326ee1d5c9b0
user:        Terrence Cole
date:        Tue Aug 28 10:28:19 2012 -0700
summary:     Bug 785551 - Do not allow inline JSString chars in the frontend; r=luke
(Assignee)

Comment 1

6 years ago
Marking S-S until I investigate because the assertion is an out-of-range access attempt.
Assignee: general → terrence
Group: core-security
(Assignee)

Comment 2

6 years ago
Unmarking s-s, since this turned out not to be s-s after all.  Patch coming as soon as jit-tests finish running.
Group: core-security
(Assignee)

Comment 3

6 years ago
Created attachment 678501 [details] [diff] [review]
v0

The assertion that the chars pointer passed to JSDependentString::new_ be within the most base dependent string is wrong if any string in the chain of dependent strings has been undepended.  Instead, we want to assert that the chars we are pointing into are kept alive by some string in the list of base strings, not just the one we happened to land on.

Luke: I also rolled in a trivial bug fix for the case where we want to create a short string.  I think I clearly just made a mistake there because the test should clearly depend on length and not on base->length().
Attachment #678501 - Flags: review?(luke)
Comment on attachment 678501 [details] [diff] [review]
v0

r+ on all the changes.  I'd split the short-string change into a separate patch (rs=me), though, for the purpose of later bisection if this moves the needle on perf.  (Conceivably it could (for the negative) since short-strings can entail more copying (esp. on 64 bit).)
Attachment #678501 - Flags: review?(luke) → review+
https://hg.mozilla.org/mozilla-central/rev/0f11212f4869
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
(Reporter)

Comment 8

6 years ago
A type of test for this bug has already been landed because it is already marked in-testsuite+ -> VERIFIED.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.