Closed Bug 1370886 Opened 7 years ago Closed 7 years ago

Assertion failure: begin + len <= length(), at js/src/jsscript.cpp:1691 with columnNumber property and asm.js

Categories

(Core :: JavaScript Engine, defect)

x86_64
Linux
defect
Not set
critical

Tracking

()

RESOLVED DUPLICATE of bug 1366927
Tracking Status
firefox55 --- wontfix
firefox56 --- fixed

People

(Reporter: decoder, Unassigned)

References

Details

(4 keywords, Whiteboard: [jsbugmon:update,bisect,ignore])

Attachments

(1 file, 1 obsolete file)

The following testcase crashes on mozilla-central revision 130efc657df7 (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-stdcxx-compat --disable-profiling --enable-debug --enable-optimize, run with --fuzzing-safe --ion-offthread-compile=off min.js):

assertEq(evaluate(`var f = x=>(function evaluate() { "use asm"; function assertEq() {} return assertEq }); f()`, {
    columnNumber: 1729
}), 1741);


Backtrace:

 received signal SIGSEGV, Segmentation fault.
0x0000000000a22e18 in js::ScriptSource::chars (this=this@entry=0x7ffff43493e0, cx=cx@entry=0x7ffff6952000, holder=..., begin=12, len=len@entry=1802) at js/src/jsscript.cpp:1691
#0  0x0000000000a22e18 in js::ScriptSource::chars (this=this@entry=0x7ffff43493e0, cx=cx@entry=0x7ffff6952000, holder=..., begin=12, len=len@entry=1802) at js/src/jsscript.cpp:1691
#1  0x0000000000a2317a in js::ScriptSource::PinnedChars::PinnedChars (len=1802, begin=<optimized out>, holder=..., source=0x7ffff43493e0, cx=0x7ffff6952000, this=0x7fffffffce50) at js/src/jsscript.cpp:1653
#2  js::ScriptSource::substring (this=0x7ffff43493e0, cx=cx@entry=0x7ffff6952000, start=<optimized out>, stop=<optimized out>) at js/src/jsscript.cpp:1774
#3  0x0000000000cd2526 in js::AsmJSModuleToString (cx=cx@entry=0x7ffff6952000, fun=..., addParenToLambda=addParenToLambda@entry=true) at js/src/wasm/AsmJS.cpp:8907
#4  0x00000000009b0d57 in js::FunctionToString (cx=cx@entry=0x7ffff6952000, fun=..., prettyPrint=prettyPrint@entry=false) at js/src/jsfun.cpp:999
#5  0x00000000009b1280 in fun_toStringHelper (cx=cx@entry=0x7ffff6952000, obj=..., obj@entry=..., indent=indent@entry=32768) at js/src/jsfun.cpp:1132
#6  0x00000000009d1322 in fun_toSource (cx=0x7ffff6952000, argc=<optimized out>, vp=<optimized out>) at js/src/jsfun.cpp:1185
#7  0x000000000053ec9f in js::CallJSNative (cx=cx@entry=0x7ffff6952000, native=0x9d1190 <fun_toSource(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/jscntxtinlines.h:293
[...]
#29 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at js/src/shell/js.cpp:8511
rax	0x0	0
rbx	0xc	12
rcx	0x7ffff6c28a2d	140737333332525
rdx	0x0	0
rsi	0x7ffff6ef7770	140737336276848
rdi	0x7ffff6ef6540	140737336272192
rbp	0x7fffffffce20	140737488342560
rsp	0x7fffffffcd60	140737488342368
r8	0x7ffff6ef7770	140737336276848
r9	0x7ffff7fe4740	140737354024768
r10	0x58	88
r11	0x7ffff6b9f750	140737332770640
r12	0x7ffff43493e0	140737290474464
r13	0x716	1814
r14	0x7ffff6952000	140737330356224
r15	0x70a	1802
rip	0xa22e18 <js::ScriptSource::chars(JSContext*, js::UncompressedSourceCache::AutoHoldEntry&, unsigned long, unsigned long)+1096>
=> 0xa22e18 <js::ScriptSource::chars(JSContext*, js::UncompressedSourceCache::AutoHoldEntry&, unsigned long, unsigned long)+1096>:	movl   $0x0,0x0
   0xa22e23 <js::ScriptSource::chars(JSContext*, js::UncompressedSourceCache::AutoHoldEntry&, unsigned long, unsigned long)+1107>:	ud2
Again NI for :shu. Some of these might be dups but they keep popping up even after other bugs are fixed. So I want to make sure we have them all on file by now :)
Flags: needinfo?(shu)
Attached patch fix.patch (obsolete) — Splinter Review
After reading bug 1364648, I think that's the same kind of issue. From reading the code, it appears that srcBodyStart is unused and we can remove it. Also, we seem to assume column-0-based code everywhere, so a possible fix is to just subtract the source column offset every time we read a column.
Assignee: nobody → bbouvier
Status: NEW → ASSIGNED
Flags: needinfo?(shu)
Attachment #8880462 - Flags: review?(shu)
Comment on attachment 8880462 [details] [diff] [review]
fix.patch

Review of attachment 8880462 [details] [diff] [review]:
-----------------------------------------------------------------

I've reworked the concept of "column" in the frontend to be separate from "buffer offset in source" in bug 1366927. It's still pending review -- this patch would need to be redone after that lands. Sorry about the wait.
Attachment #8880462 - Flags: review?(shu)
(Note for regression triage group: the patch in Bug 1366927 --that this bug depends on -- was r+'d on 07-07)
Flags: needinfo?(bbouvier)
Setting 55 wontfix per IRC conversation with bbouvier.
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update,bisect,ignore]
JSBugMon: The testcase found in this bug no longer reproduces (tried revision 04b6be50a252).
autoBisect shows this is probably related to the following changeset:

The first good revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/19a82f6ac49e
user:        Shu-yu Guo
date:        Mon Jul 17 18:45:52 2017 -0700
summary:     Bug 1366927 - Rework column handling in frontend by separating column from offset from root ScriptSource buffer. (r=jimb)

Benjamin, is bug 1366927 a likely fix, or is your patch also needing continued rework?
Yes indeed. It's nice and ~free to check in the test, so let's do it.
Flags: needinfo?(bbouvier)
Attached patch test.patchSplinter Review
Attachment #8880462 - Attachment is obsolete: true
Attachment #8907255 - Flags: review?(luke)
Attachment #8907255 - Flags: review?(luke) → review+
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Keywords: checkin-needed
Resolution: --- → DUPLICATE
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b21cba0005ef
Test for source script offset behavior in asm.js. r=luke
Keywords: checkin-needed
Assignee: bugzilla → nobody
You need to log in before you can comment on or make changes to this bug.