1382997 - Assertion failure: TraceLogTextIdEnabled(TraceLogger_Scripts), at js/src/jit/CodeGenerator.cpp:9941

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 1b065ffd8a53 (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):

for (var idx = 0; idx < 100 ; ++idx)
  evalInWorker(`
    try {
      let g = newGlobal();
      var du = new Debugger();
      du.setupTraceLogger({ Scripts: true })
      du.setupTraceLogger({ Scripts: false })
      var TIME_0000 = (function () {
        for ( var time = 0, year = 1969; year >= 0; year-- ) {}
      })();
    } catch(exc) {}
  `);



Backtrace:

 received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7ffff0ade700 (LWP 7168)]
0x00000000006933c3 in js::jit::CodeGenerator::link (this=this@entry=0x7ffff1dce000, cx=cx@entry=0x7ffff0ffe000, constraints=<optimized out>) at js/src/jit/CodeGenerator.cpp:9941
#0  0x00000000006933c3 in js::jit::CodeGenerator::link (this=this@entry=0x7ffff1dce000, cx=cx@entry=0x7ffff0ffe000, constraints=<optimized out>) at js/src/jit/CodeGenerator.cpp:9941
#1  0x00000000006af9d6 in LinkCodeGen (cx=cx@entry=0x7ffff0ffe000, builder=builder@entry=0x7ffff1670270, codegen=0x7ffff1dce000) at js/src/jit/Ion.cpp:531
#2  0x000000000043b94f in js::jit::IonCompile (cx=cx@entry=0x7ffff0ffe000, script=<optimized out>, baselineFrame=baselineFrame@entry=0x7ffff0adcd08, osrPc=osrPc@entry=0x7ffff0f41385 "\343\201V", recompile=<optimized out>, optimizationLevel=<optimized out>) at js/src/jit/Ion.cpp:2263
#3  0x0000000000717b2b in js::jit::Compile (cx=cx@entry=0x7ffff0ffe000, script=script@entry=..., osrFrame=osrFrame@entry=0x7ffff0adcd08, osrPc=osrPc@entry=0x7ffff0f41385 "\343\201V", forceRecompile=<optimized out>) at js/src/jit/Ion.cpp:2448
#4  0x000000000071846b in BaselineCanEnterAtBranch (pc=0x7ffff0f41385 "\343\201V", osrFrame=0x7ffff0adcd08, script=..., cx=0x7ffff0ffe000) at js/src/jit/Ion.cpp:2639
#5  js::jit::IonCompileScriptForBaseline (cx=cx@entry=0x7ffff0ffe000, frame=frame@entry=0x7ffff0adcd08, pc=pc@entry=0x7ffff0f41385 "\343\201V") at js/src/jit/Ion.cpp:2697
#6  0x00000000005ff7c6 in js::jit::DoWarmUpCounterFallbackOSR (cx=0x7ffff0ffe000, frame=0x7ffff0adcd08, stub=0x7ffff42e90b8, infoPtr=0x7ffff0adccd0) at js/src/jit/BaselineIC.cpp:145
#7  0x00001a5e99d81355 in ?? ()
[...]
#18 0x0000000000000000 in ?? ()
rax	0x0	0
rbx	0x7ffff0adc650	140737231308368
rcx	0x7ffff6c28a2d	140737333332525
rdx	0x0	0
rsi	0x7ffff6ef7770	140737336276848
rdi	0x7ffff6ef6540	140737336272192
rbp	0x7ffff0adc7e0	140737231308768
rsp	0x7ffff0adc5a0	140737231308192
r8	0x7ffff6ef7770	140737336276848
r9	0x7ffff0ade700	140737231316736
r10	0x58	88
r11	0x7ffff6b9f750	140737332770640
r12	0x7ffff0adc650	140737231308368
r13	0x7ffff0adc690	140737231308432
r14	0x7ffff1dce000	140737251172352
r15	0x7ffff0adc670	140737231308400
rip	0x6933c3 <js::jit::CodeGenerator::link(JSContext*, js::CompilerConstraintList*)+4659>
=> 0x6933c3 <js::jit::CodeGenerator::link(JSContext*, js::CompilerConstraintList*)+4659>:	movl   $0x0,0x0
   0x6933ce <js::jit::CodeGenerator::link(JSContext*, js::CompilerConstraintList*)+4670>:	ud2    



Marking as fuzzblocker because this reproduces highly frequently and is at the same time hard to reduce.

Comment 1

[Christian Holler (:decoder)]

Needinfo for :jandem to disable TraceLogger in our codebase as discussed on IRC.

Comment 2

[Jan de Mooij [:jandem]]

Attachment: Patch

This removes the Debugger tracelogging functions. A bit sad but it's hard to justify spending time bug fixing a feature that's not being used and is unlikely to be used in the near future.

Comment 3

[Nicolas B. Pierron [:nbp]]

Comment on attachment 8895359 [details] [diff] [review]
Patch

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

Sounds like the best plan so far, just removing the TraceLogger from the DebuggerAPI should get rid of the fuzzblocker issue, while keeping the code alive long enough for it to potentially have a new life under the perfstream way of collecting data.
Let's hope the perfstream will simplify the TraceLogger interface.

::: js/src/jit-test/tests/tracelogger/bug1304641.js
@@ +1,3 @@
> +var failed = false;
> +try {
> +    newGlobal().startTraceLogger();

nit: remove this test case as well.

Comment 4

[Jan de Mooij [:jandem]]

(In reply to Nicolas B. Pierron [:nbp] from comment #3)
> Sounds like the best plan so far, just removing the TraceLogger from the
> DebuggerAPI should get rid of the fuzzblocker issue, while keeping the code
> alive long enough for it to potentially have a new life under the perfstream
> way of collecting data.
> Let's hope the perfstream will simplify the TraceLogger interface.

Note that there are also shell functions in TestingFunctions.cpp to start TraceLogger, so I don't know if removing the Debugger API is sufficient...

Also I don't agree we should wait for perfstream - we don't even know yet if/when that will be finished and whether it will use the TL backend. Even if it will, we'll need someone to maintain the code on our end and fix the bugs in it. I don't think we found someone willing to do that.

> > +    newGlobal().startTraceLogger();
> 
> nit: remove this test case as well.

This uses the startTraceLogger shell function I mentioned which is not removed by this patch...

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f088fdd84d4d
Remove TraceLogger Debugger interface. r=nbp

Comment 6

[Jan de Mooij [:jandem]]

@gkw, decoder: this removes just the Debugger interface. If TL still causes problems we should consider removing more code.

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/f088fdd84d4d

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Did you want to let this ride the trains?

Comment 9

[Jan de Mooij [:jandem]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #8)
> Did you want to let this ride the trains?

No this can be wontfix for 56. Bug 1372851 disabled TL by default in opt builds, and the Debugger object isn't exposed to content anyway.