Closed Bug 1521338 Opened 9 months ago Closed 9 months ago

Disable dumpScopeChain in more-deterministic mode

Categories

(Core :: JavaScript Engine, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla67
Tracking Status
firefox66 --- fixed
firefox67 --- fixed

People

(Reporter: gkw, Assigned: jandem)

Details

Attachments

(2 files)

dumpScopeChain has only been part of shell-only crash bug 1516406, but because it prints debug output, it causes issues with compare_jit differential testing.

My attempts at neutering it have only been partially successful, as nesting dumpScopeChain in evalcx still causes the testcase to print debug information.

Jan suggests marking it fuzzing-unsafe, so setting needinfo? from him here.

Flags: needinfo?(jdemooij)

If this is solely a problem for differential testing, why not disable this function with --enable-more-deterministic builds?

I did mention to neuter the output, but I'll let Jan decide.

If it's just for differential testing, I agree it's best to wrap in #ifdef JS_MORE_DETERMINISTIC

Assignee: nobody → jdemooij
Status: NEW → ASSIGNED
Flags: needinfo?(jdemooij)
Summary: Mark dumpScopeChain as fuzzing-unsafe → Disable dumpScopeChain in more-deterministic mode

May we have a ride-along to neuter this message as well?

print(uneval(newGlobal({
    invisibleToDebugger: true
})));

shows:

Error: All the realms in a compartment must have the same debugger visibility

without --more-compartments, but proceeds with it.

Ref:

https://searchfox.org/mozilla-central/rev/465dbfe030dfec7756b9b523029e90d48dd5ecce/js/src/shell/js.cpp#6275

Tested on a m-c rev 0b28e8a97af4 shell, compiled with --enable-debug --enable-more-deterministic

Flags: needinfo?(jdemooij)
Attached patch second patchSplinter Review

Specifically, this one fixes the ride-along. Or shall I file a new bug?

Attachment #9039256 - Flags: feedback?(jdemooij)

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

May we have a ride-along to neuter this message as well?

Unfortunately this doesn't work because --more-compartments can have observable effects so we shouldn't use it for differential testing. Here's one example:

$ dist/bin/js
js> isProxy(newGlobal())
false

$ dist/bin/js --more-compartments
js> isProxy(newGlobal())
true
Flags: needinfo?(jdemooij)
Attachment #9039256 - Flags: feedback?(jdemooij)

Thanks for the clarification, I will remove it from the list of flags to be tested by compare_jit.

Pushed by jdemooij@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/06dbc0d7cc08
Don't dump anything in dumpScopeChain in more-deterministic builds. r=jorendorff
Status: ASSIGNED → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67

This is NPOTB (fuzzing more-deterministic builds only), can we pls backport to mozilla-beta?

Flags: needinfo?(ryanvm)
Flags: needinfo?(ryanvm)
Whiteboard: [checkin-needed-beta]
Whiteboard: [checkin-needed-beta]
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.