Assertion failure: hasBase(), at vm/String.h:1361 with newExternalString/dumpStringRepresentation

RESOLVED FIXED in Firefox 54

Status

()

--
critical
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: decoder, Assigned: jandem)

Tracking

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

Trunk
mozilla54
x86_64
Linux
assertion, jsbugmon, testcase
Points:
---

Firefox Tracking Flags

(firefox51 wontfix, firefox52 wontfix, firefox-esr52 wontfix, firefox53 wontfix, firefox54 fixed)

Details

(Whiteboard: [jsbugmon:update])

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
The following testcase crashes on mozilla-central revision 20a8536b0bfa (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-debug --enable-optimize, run with --fuzzing-safe --ion-offthread-compile=off):

var j = newExternalString('');
dumpStringRepresentation(j);



Backtrace:

 received signal SIGSEGV, Segmentation fault.
0x0000000000bcc7d0 in JSString::base (this=<optimized out>) at js/src/vm/String.h:1361
#0  0x0000000000bcc7d0 in JSString::base (this=<optimized out>) at js/src/vm/String.h:1361
#1  0x0000000000bbb1eb in JSExternalString::dumpRepresentation (this=0x7ffff06ad028, fp=0x7ffff6ef6540 <_IO_2_1_stderr_>, indent=2) at js/src/vm/String.cpp:1116
#2  0x0000000000bbafe1 in JSString::dumpRepresentation (this=<optimized out>, fp=<optimized out>, indent=<optimized out>) at js/src/vm/String.cpp:189
#3  0x000000000084e0b4 in DumpStringRepresentation (cx=<optimized out>, argc=<optimized out>, vp=<optimized out>) at js/src/builtin/TestingFunctions.cpp:3417
#4  0x0000000000534a58 in js::CallJSNative (cx=cx@entry=0x7ffff6921000, native=0x84e030 <DumpStringRepresentation(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/jscntxtinlines.h:281
[...]
#17 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at js/src/shell/js.cpp:8151
rax	0x0	0
rbx	0x2	2
rcx	0x7ffff6c28a2d	140737333332525
rdx	0x0	0
rsi	0x7ffff6ef7770	140737336276848
rdi	0x7ffff6ef6540	140737336272192
rbp	0x7fffffffcb20	140737488341792
rsp	0x7fffffffcb20	140737488341792
r8	0x7ffff6ef7770	140737336276848
r9	0x7ffff7fe4740	140737354024768
r10	0x58	88
r11	0x7ffff6b9f750	140737332770640
r12	0x7ffff06ad028	140737226919976
r13	0x7ffff6ef6540	140737336272192
r14	0x84e030	8708144
r15	0x7ffff02340a8	140737222230184
rip	0xbcc7d0 <JSString::base() const+48>
=> 0xbcc7d0 <JSString::base() const+48>:	movl   $0x0,0x0
   0xbcc7db <JSString::base() const+59>:	ud2

Updated

2 years ago
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]

Comment 1

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

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/eed0b80462a2
user:        Jan de Mooij
date:        Thu Jan 26 18:40:41 2017 +0100
summary:     Bug 1330593 part 1 - Allow non-flat external strings. r=jwalden,bz

This iteration took 261.798 seconds to run.
(Assignee)

Comment 2

2 years ago
Ugh, another pre-existing bug exposed by newExternalString.

The DEBUG-only JSExternalString::dumpRepresentation function makes no sense. It calls base() but external strings don't have a base string so we assert.
Flags: needinfo?(jdemooij)
(Assignee)

Comment 3

2 years ago
Created attachment 8837639 [details] [diff] [review]
Patch

The test uses the representativeStringArray function that I still need to land.

Output looks like this:

js> dumpStringRepresentation(newExternalString("abc"))
((JSExternalString*) 0x1112ae028) length: 3  flags: 0x20
  finalizer: ((JSStringFinalizer*) 0x1108dad10)
  chars: ((char16_t*) 0x11112c110) "abc"
Assignee: nobody → jdemooij
Status: NEW → ASSIGNED
Flags: needinfo?(jdemooij)
Attachment #8837639 - Flags: review?(jwalden+bmo)

Updated

2 years ago
Attachment #8837639 - Flags: review?(jwalden+bmo) → review+

Comment 4

2 years ago
Pushed by jandemooij@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d08acf88f5b3
Fix JSExternalString::dumpRepresentation. r=jwalden

Updated

2 years ago
Whiteboard: [jsbugmon:update] → [jsbugmon:update,ignore]

Comment 5

2 years ago
JSBugMon: The testcase found in this bug no longer reproduces (tried revision 34c6c2f302e7).

Comment 6

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/d08acf88f5b3
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox54: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Whiteboard: [jsbugmon:update,ignore] → [jsbugmon:update]
Is this worth taking on Beta in the interest of reducing noise for the fuzzers?
status-firefox51: --- → wontfix
status-firefox52: --- → wontfix
status-firefox53: --- → affected
status-firefox-esr52: --- → wontfix
Flags: needinfo?(jdemooij)
Per IRC discussion, might as well with a=NPOTB.
Flags: needinfo?(jdemooij)
Whiteboard: [jsbugmon:update] → [jsbugmon:update][checkin-needed-beta]

Comment 9

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/068094999e10
status-firefox53: affected → fixed
Whiteboard: [jsbugmon:update][checkin-needed-beta] → [jsbugmon:update]
This depends on a shell function that doesn't exist on Beta. Not worth the effort to chase down and uplift. Backed out.
https://hg.mozilla.org/releases/mozilla-beta/rev/42bbb960062c48636be15ad5cef187353a3be84d
status-firefox53: fixed → wontfix
You need to log in before you can comment on or make changes to this bug.