1427128 - Assertion failure: ret == len (Computed length should match actual length!), at js/src/vm/GeckoProfiler.cpp:330 with TypedObject

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P2)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 01cbfc6c625f (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --disable-profiling --enable-debug --enable-optimize, run with --fuzzing-safe):

enableGeckoProfiling();
var StructType = TypedObject.StructType;
var RgbColor = new StructType({
    get "\u0000\u000b" () {}
});


Backtrace:

received signal SIGSEGV, Segmentation fault.
0x0000000000babee4 in js::GeckoProfilerRuntime::allocProfileString (this=this@entry=0x7ffff5f1a360, script=<optimized out>, maybeFun=maybeFun@entry=0x7ffff44ad470) at js/src/vm/GeckoProfiler.cpp:330
#0  0x0000000000babee4 in js::GeckoProfilerRuntime::allocProfileString (this=this@entry=0x7ffff5f1a360, script=<optimized out>, maybeFun=maybeFun@entry=0x7ffff44ad470) at js/src/vm/GeckoProfiler.cpp:330
#1  0x0000000000bb7df9 in js::GeckoProfilerRuntime::profileString (this=0x7ffff5f1a360, script=script@entry=0x7ffff4491160, maybeFun=0x7ffff44ad470) at js/src/vm/GeckoProfiler.cpp:183
#2  0x0000000000bb7fb1 in js::GeckoProfilerThread::enter (this=0x7ffff5f169a8, cx=0x7ffff5f16000, script=0x7ffff4491160, maybeFun=<optimized out>) at js/src/vm/GeckoProfiler.cpp:221
#3  0x0000000000c776e8 in js::probes::EnterScript (cx=<optimized out>, script=<optimized out>, maybeFun=<optimized out>, fp=0x7ffff40360b8) at js/src/vm/Probes-inl.h:42
#4  0x0000000000c55575 in js::InterpreterFrame::prologue (this=0x7ffff40360b8, cx=0x7ffff5f16000) at js/src/vm/Stack.cpp:249
#5  0x000000000055d4a4 in Interpret (cx=0x7ffff5f16000, state=...) at js/src/vm/Interpreter.cpp:1918
#6  0x000000000056a1d5 in js::RunScript (cx=0x7ffff5f16000, state=...) at js/src/vm/Interpreter.cpp:423
#7  0x000000000056a707 in js::InternalCallOrConstruct (cx=cx@entry=0x7ffff5f16000, args=..., construct=construct@entry=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:495
#8  0x000000000056aa1d in InternalCall (cx=cx@entry=0x7ffff5f16000, args=...) at js/src/vm/Interpreter.cpp:522
#9  0x000000000056ab90 in js::Call (cx=cx@entry=0x7ffff5f16000, fval=..., fval@entry=..., thisv=..., thisv@entry=..., args=..., rval=...) at js/src/vm/Interpreter.cpp:541
#10 0x000000000056ad63 in js::CallGetter (cx=0x7ffff5f16000, thisv=thisv@entry=..., getter=getter@entry=..., rval=rval@entry=...) at js/src/vm/Interpreter.cpp:656
#11 0x0000000000bde16c in CallGetter (vp=..., shape=..., receiver=..., obj=..., cx=0x7ffff5f16000) at js/src/vm/NativeObject.cpp:2125
#12 GetExistingProperty<(js::AllowGC)1> (cx=0x7ffff5f16000, receiver=..., obj=..., shape=..., vp=...) at js/src/vm/NativeObject.cpp:2182
#13 0x0000000000be4774 in NativeGetPropertyInline<(js::AllowGC)1> (cx=cx@entry=0x7ffff5f16000, obj=..., receiver=..., id=..., nameLookup=nameLookup@entry=NotNameLookup, vp=...) at js/src/vm/NativeObject.cpp:2381
#14 0x0000000000be4eb0 in js::NativeGetProperty (cx=cx@entry=0x7ffff5f16000, obj=..., receiver=..., receiver@entry=..., id=..., id@entry=..., vp=..., vp@entry=...) at js/src/vm/NativeObject.cpp:2417
#15 0x000000000053e879 in js::GetProperty (vp=..., id=..., receiver=..., obj=..., cx=0x7ffff5f16000) at js/src/vm/NativeObject.h:1620
#16 js::GetProperty (cx=0x7ffff5f16000, obj=..., receiver=..., id=..., vp=...) at js/src/jsobj.h:812
#17 0x00000000008d7297 in js::StructMetaTypeDescr::create (cx=0x7ffff5f16000, metaTypeDescr=..., metaTypeDescr@entry=..., fields=..., fields@entry=...) at js/src/builtin/TypedObject.cpp:831
#18 0x00000000008d818f in js::StructMetaTypeDescr::construct (cx=0x7ffff5f16000, argc=<optimized out>, vp=<optimized out>) at js/src/builtin/TypedObject.cpp:1014
#19 0x0000000000575dd1 in js::CallJSNative (cx=0x7ffff5f16000, native=native@entry=0x8d8080 <js::StructMetaTypeDescr::construct(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/jscntxtinlines.h:291
[...]
#32 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at js/src/shell/js.cpp:9141
rax	0x0	0
rbx	0x7ffff5f4d078	140737319850104
rcx	0x7ffff6c282ad	140737333330605
rdx	0x0	0
rsi	0x7ffff6ef7770	140737336276848
rdi	0x7ffff6ef6540	140737336272192
rbp	0x7fffffffb590	140737488336272
rsp	0x7fffffffb480	140737488336000
r8	0x7ffff6ef7770	140737336276848
r9	0x7ffff7fe4780	140737354024832
r10	0x58	88
r11	0x7ffff6b9e7a0	140737332766624
r12	0x12	18
r13	0x10	16
r14	0x7ffff412f540	140737288271168
r15	0x7fffffffb620	140737488336416
rip	0xbabee4 <js::GeckoProfilerRuntime::allocProfileString(JSScript*, JSFunction*)+900>
=> 0xbabee4 <js::GeckoProfilerRuntime::allocProfileString(JSScript*, JSFunction*)+900>:	movl   $0x0,0x0
   0xbabeef <js::GeckoProfilerRuntime::allocProfileString(JSScript*, JSFunction*)+911>:	ud2


Not marking s-s because it looks like profiler-only.

Comment 1

[Jeff Walden [:Waldo]]

This is asserting that the number of characters printed out for a formatted-string-print equals a particular value, via snprintf.  The property name here, is one of the arguments passed to snprintf.  But when the property name includes u+0000, snprintf will print not the entirety of the property name, because of null-termination.

The assertion-botch is easily fixed by making it |ret <= len|.  But arguably maybe this should be printing out "\u0000" or something instead of just spewing literal characters into the output.  (And similar complaints about embedded newlines and similar, except those wouldn't break this assertion.)  So I'll just leave this bit of triage here and then let someone more familiar with what the profiler is doing, and what it should be doing, to decide how to handle this.

Comment 2

[Fuzzing Team]

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/09841bae5caf
user:        Nicholas Nethercote
date:        Wed Jan 25 09:08:15 2017 +1100
summary:     Bug 1333296 (part 6) - Remove SPS references in js/. r=shu.

This iteration took 240.801 seconds to run.

Comment 3

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

enableGeckoProfiling() used to be enableSPSProfiling() and I think :djvj used to work on this, so setting needinfo?.

Comment 4

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

autobisectjs shows this is probably related to the following changeset:

The first bad revision is:
changeset: https://hg.mozilla.org/mozilla-central/rev/310418e4db4b
user: Tom Schuster
date: Wed Apr 13 13:43:43 2016 +0200
summary: Bug 1255925 - Give a name to getters/setters and integer-named methods. r=efaust

I renamed enableGeckoProfiling() to enableSPSProfiling() then bisected more backwards to this.

Tom, is bug 1255925 a likely regressor?

Comment 5

[Tom S]

Didn't Waldo already analyze this in comment 1.

Comment 6

[Fuzzing Team]

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

Comment 7

[Fuzzing Team]

JSBugMon: The testcase found in this bug no longer reproduces (tried revision 6fd64908d113).
JSBugMon: Fix Bisection requested, result:
autoBisect shows this is probably related to the following changeset:

The first good revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/39af0d6ccd3d
user:        Jan de Mooij
date:        Fri May 03 10:15:51 2019 +0000
summary:     Bug 1548510 part 2 - Deduplicate GeckoProfilerRuntime::allocProfileString and JitcodeGlobalEntry::createScriptString. r=jonco

This iteration took 510.066 seconds to run.

Comment 8

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Jan, is bug 1548510 a likely fix?

Comment 9

[Jan de Mooij [:jandem]]

(In reply to Gary Kwong [:gkw] [:nth10sd] from comment #8)

Jan, is bug 1548510 a likely fix?

Yes. We used to have two copies of this code and one of them handled this case correctly; we now use that one everywhere.