857052 - Assertion failure: ptrdiff_t(column) + colspan >= 0, at js/src/jsscript.cpp:2111

Status: RESOLVED DUPLICATE of bug 1355046

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase asserts on mozilla-central revision aae004a3c5d9 (no options required):


var localstr = "";
for (var i = 0; i < 10000; ++i)
    localstr += ("evil = eval; eval('evil(\\\'Math;\\\');');") + i + "; ";
var body = localstr + "for (var i = 0; i < 4; ++i) arr[i](x-1);";
(new Function("x", body))(1000);

Comment 1

[Christian Holler (:decoder)]

Marked s-s because the assertion is length-related and the optimized build spills out an error that seems to contain an overflowed column number:

test.js:5:2190004523 ReferenceError: arr is not defined

Comment 2

[Christian Holler (:decoder)]

JSBugMon: Bisection requested, result:
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   103211:43b106855cbb
user:        Alex Crichton
date:        Thu Aug 23 10:13:52 2012 -0700
summary:     Bug 785094: Fix negative colspans showing up in the wrong places. r=jorendorff

This iteration took 91.605 seconds to run.

Comment 3

[Christian Holler (:decoder)]

Needinfo from Alex based on comment 2 :)

Comment 4

[u443197]

It's been a long time since I last looked at this code, but one thing going on here could be that the source of the function is very long (the column span is massive). I thought that case was covered, but it may not be fully taken care of?

The bisected commit fixed a few places where this assertion cropped up, so this may just be a case of there needing to be another place or two where the colspan needs to be reset back to 0. Sadly I don't have time to look into this right now.

Comment 5

[Christian Holler (:decoder)]

> The bisected commit fixed a few places where this assertion cropped up, so
> this may just be a case of there needing to be another place or two where
> the colspan needs to be reset back to 0.

If that was the case, then the test would also reproduce before the commit, right? According to the bisect, this change makes the test reproduce, where it didn't reproduce before.

> Sadly I don't have time to look into this right now.

Can you please comment on what's exactly happening here security-wise? Is there any way to make this crash or cause out-of-bounds reads, etc? I classified this as a security bug based on the assertion, so we need to find time to either rule out the security aspect or to fix the issue.

Comment 6

[u443197]

(In reply to Christian Holler (:decoder) from comment #5)
> > The bisected commit fixed a few places where this assertion cropped up, so
> > this may just be a case of there needing to be another place or two where
> > the colspan needs to be reset back to 0.
> 
> If that was the case, then the test would also reproduce before the commit,
> right? According to the bisect, this change makes the test reproduce, where
> it didn't reproduce before.

That's a good point. Was the previous commit run and determined to be green for this test?

> Can you please comment on what's exactly happening here security-wise?

As far as I know, the column numbers are only used for reporting and they're never used as an actual index. Odd, possibly negative, column numbers will be reported (if the assertion fails). In what I was doing related to column numbers, nothing was related to using the column numbers as indices into arrays, so this would just cause some weird results to show up in error messages.

That being said, I'm not sure that no one's doing this. I can definitely say that within the function which has the assertion, nothing is getting dereferenced badly or accessed out-of-bounds if this assertion silently fails. The callers just get weird results.

Comment 7

[Andrew McCreight [:mccr8]]

Jason, do you agree with comment 6?  It sounds like this doesn't have to be a security bug.

Comment 8

[Jason Orendorff [:jorendorff]]

This is not security-sensitive.

Incidentally, I don't get this assertion either in rev aae004a3c5d9 or in mozilla-inbound tip (de487abe67a2) on Mac.

Comment 9

[Andrew McCreight [:mccr8]]

Thanks.

Comment 10

[Christian Holler (:decoder)]

JSBugMon: The testcase found in this bug no longer reproduces (tried revision a35163f83d22).

Comment 11

[Christian Holler (:decoder)]

This is an automated crash issue comment:

Summary: Assertion failure: ptrdiff_t(column) + colspan >= 0, at js/src/jsscript.cpp:2867
Build version: mozilla-central revision 502e1a5e722f
Build flags: --enable-optimize --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --target=i686-pc-linux-gnu --disable-tests --enable-debug
Runtime options: --fuzzing-safe --thread-count=2 --ion-eager --ion-extra-checks

Testcase:

var nlocals = 0Xbddac ;
var localstr = "";
for (var i = 0; i < nlocals; ++i)
    localstr += (0) + i + "; ";
var arg = "x";
var body = localstr +
           "for (var i = 0; i < 4; ++i) arr[i](x-1);";
try { (new Function(arg, body))(1000); } catch (e) {}


Backtrace:

Program terminated with signal 11, Segmentation fault.

#0  0x087506b1 in js::PCToLineNumber (startLine=<optimized out>, notes=0xec90006d "\210\003\210\006\210\t\210\f\210\017\210\022\210\025\210\030\210\033\210\036\210\"\210&\210*\210.\210\062\210\066\210:\210>\210B\210F\210J\210N\210R\210V\210Z\210^\210b\210f\210j\210n\210r\210v\210z\210~\210\200", code=0xec900009 "T", pc=<optimized out>, pc@entry=0xec900015 "\232", columnp=columnp@entry=0xfff006a8) at js/src/jsscript.cpp:2867
#1  0x08750737 in js::PCToLineNumber (script=0xf5a72dc0, pc=pc@entry=0xec900015 "\232", columnp=columnp@entry=0xfff006a8) at js/src/jsscript.cpp:2885
#2  0x083748e3 in js::FrameIter::computeLine (this=this@entry=0xfff003a0, column=column@entry=0xfff006a8) at js/src/vm/Stack.cpp:935
#3  0x086ef1f1 in PopulateReportBlame (cx=cx@entry=0xf717d040, report=report@entry=0xfff006a0) at js/src/jscntxt.cpp:263
#4  0x086efe19 in js::ReportErrorNumberVA (cx=0xf717d040, flags=flags@entry=0, callback=callback@entry=0x86d1650 <js::GetErrorMessage(void*, unsigned int)>, userRef=userRef@entry=0x0, errorNumber=errorNumber@entry=1, argumentsType=argumentsType@entry=js::ArgumentsAreASCII, ap=ap@entry=0xfff00770 "\320 \234\365\240}\245\365\240}\245\365\357-\016\b\320 \234\365\001\304f\t\314\a\360\377\030\304f\t\240}\245\365\205\377\377\377\270\a\360\377\357\017\016\b\254\a\360\377") at js/src/jscntxt.cpp:738
#5  0x086eff29 in JS_ReportErrorNumberVA (cx=cx@entry=0xf717d040, errorCallback=errorCallback@entry=0x86d1650 <js::GetErrorMessage(void*, unsigned int)>, userRef=userRef@entry=0x0, errorNumber=errorNumber@entry=1, ap=ap@entry=0xfff00770 "\320 \234\365\240}\245\365\240}\245\365\357-\016\b\320 \234\365\001\304f\t\314\a\360\377\030\304f\t\240}\245\365\205\377\377\377\270\a\360\377\357\017\016\b\254\a\360\377") at js/src/jsapi.cpp:5123
#6  0x086eff69 in JS_ReportErrorNumber (cx=cx@entry=0xf717d040, errorCallback=errorCallback@entry=0x86d1650 <js::GetErrorMessage(void*, unsigned int)>, userRef=userRef@entry=0x0, errorNumber=errorNumber@entry=1) at js/src/jsapi.cpp:5112
#7  0x086f0fa4 in js::ReportIsNotDefined (cx=cx@entry=0xf717d040, id=id@entry=...) at js/src/jscntxt.cpp:823
#8  0x086f10c4 in js::ReportIsNotDefined (cx=cx@entry=0xf717d040, name=name@entry=...) at js/src/jscntxt.cpp:831
#9  0x082853ec in js::GetScopeName (cx=cx@entry=0xf717d040, scopeChain=scopeChain@entry=..., name=name@entry=..., vp=vp@entry=...) at js/src/vm/Interpreter.cpp:4134
#10 0x084e6180 in js::jit::DoGetNameFallback (cx=0xf717d040, frame=0xfff00ab0, stub_=0xf5149980, scopeChain=..., res=...) at js/src/jit/BaselineIC.cpp:6713
#11 0xf58f4ef8 in ?? ()
#12 0xf58f0bd5 in ?? ()
#13 0x0846928e in EnterBaseline (cx=0xf5149980, cx@entry=0xf717d040, data=...) at js/src/jit/BaselineJIT.cpp:125
#14 0x0847e36d in js::jit::EnterBaselineMethod (cx=cx@entry=0xf717d040, state=...) at js/src/jit/BaselineJIT.cpp:156
#15 0x082c1fd0 in js::RunScript (cx=cx@entry=0xf717d040, state=...) at js/src/vm/Interpreter.cpp:667
#16 0x082c2645 in js::Invoke (cx=cx@entry=0xf717d040, args=..., construct=construct@entry=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:746
#17 0x082c415a in js::Invoke (cx=cx@entry=0xf717d040, thisv=..., fval=..., argc=argc@entry=1, argv=argv@entry=0xfff015c0, rval=rval@entry=...) at js/src/vm/Interpreter.cpp:783
#18 0x084f73b1 in js::jit::DoCallFallback (cx=0xf717d040, frame=0xfff01620, stub_=0xf539b6d0, argc=1, vp=0xfff015b0, res=...) at js/src/jit/BaselineIC.cpp:10424
#19 0xf58f4acc in ?? ()
#20 0xf539b6d0 in ?? ()
#21 0xf58f0bd5 in ?? ()
eax	0x0	0
ebx	0x966c418	157729816
ecx	0xf75888ac	-145192788
edx	0x0	0
esi	0xec91fe16	-325976554
edi	0x7fff2c9f	2147429535
ebp	0xfff00328	4293919528
esp	0xfff00300	4293919488
eip	0x87506b1 <js::PCToLineNumber(unsigned int, unsigned char*, unsigned char*, unsigned char*, unsigned int*)+369>
=> 0x87506b1 <js::PCToLineNumber(unsigned int, unsigned char*, unsigned char*, unsigned char*, unsigned int*)+369>:	movl   $0xb33,0x0
   0x87506bb <js::PCToLineNumber(unsigned int, unsigned char*, unsigned char*, unsigned char*, unsigned int*)+379>:	call   0x80e6270 <abort()>

Comment 12

[Christian Holler (:decoder)]

Bug still reproduces per comment 11. Needinfo from Jason.

Comment 13

[Christian Holler (:decoder)]

JSBugMon: Cannot process bug: Unable to automatically reproduce, please track manually.

Comment 14

[Jason Orendorff [:jorendorff]]

Christian, does this assertion still happen? I get the bogus column number, but no assertion.

Comment 15

[Jason Orendorff [:jorendorff]]

Bah. I can't build with the options given in comment 11, at the moment:

/home/jorendorff/dev/gecko/js/src/x-obj/_virtualenv/bin/python /home/jorendorff/dev/gecko/config/expandlibs_gen.py -o libicu.a.desc  ../../../intl/icu/target/lib/libicui18n.a ../../../intl/icu/target/lib/libicuuc.a ../../../intl/icu/target/lib/libicudata.a 
Traceback (most recent call last):
  File "/home/jorendorff/dev/gecko/config/expandlibs_gen.py", line 41, in <module>
    print >>outfile, generate(args)
  File "/home/jorendorff/dev/gecko/config/expandlibs_gen.py", line 27, in generate
    raise Exception("File not found: %s" % arg)
Exception: File not found: ../../../intl/icu/target/lib/libicui18n.a
/home/jorendorff/dev/gecko/config/rules.mk:770: recipe for target 'libicu.a.desc' failed

But I don't think the assertion matters. The bogus column number is enough to go on. Clearing ni? and taking (though it may be a while before I get to this).