951400 - IonMonkey: IONFLAGS=logs completely broken

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Sean Stangl [:sstangl]]

It looks like recent threading changes to Ion compilation broke debugging output. The following command in a debug build trips an assertion in JSFunction::hasScript() via MConstant::printOpcode():

(run.js is part of Octane)
> IONFLAGS=logs ~/dev/mozilla-inbound/js/src/dbg64/js --ion-parallel-compile=off run.js
> Assertion failure: js::CurrentThreadCanReadCompilationData(), at /home/sstangl/dev/mozilla-inbound/js/src/jsfun.h:144

Comment 1

[Sean Stangl [:sstangl]]

Attachment: Permit reading compilation data when parallel compilation is disabled

Comment 2

[Brian Hackett [Laid off!]]

Comment on attachment 8349038 [details] [diff] [review]
Permit reading compilation data when parallel compilation is disabled

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

Even when running on the main thread, we want Ion compilation to be able to trigger these assertions if operations are performed that would not be threadsafe if compilation was running off thread.  This improves coverage of the thread safety assertions even in non-threadsafe builds etc.

It would be better if the logging functions that accessed this stuff had an AutoLockForCompilation wrapped around it.  Putting it at the way top of the logging callbacks would probably be the easiest way to do this.

Comment 3

[Hannes Verschore [:h4writer]]

Attachment: My idea

I only found this bug when I was going to create my own bug report. I had already a patch. I added it here. It is different than Sean his approach and I think also has the niceties of still allowing the asserts (Whenever IONFLAGS=logs is not enabled). And that way we don't need to add the AutoLock everywhere...

I find it cleaner. But feel free to throw away. On the other hand if you think this is also good, you may review it also. (Patch is finished)

Comment 4

[Hannes Verschore [:h4writer]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f7fcd112d0c7

Comment 5

[Ryan VanderMeulen [:RyanVM]]

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