Closed Bug 1440539 Opened 7 years ago Closed 7 years ago

Add Jitter to the JS Shell

Categories

(Core :: JavaScript Engine, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla60
Tracking Status
firefox60 --- fixed

People

(Reporter: tjr, Assigned: tjr)

References

Details

Attachments

(1 file)

In Bug 1425462 we're adding a callback so the JS Engine relies on the ResistFingerprinting Component to centralize the logic time precision reduction and jittering. Up until now we've had time precision reduction done in the JS Engine also. But we aren't adding jitter logic in the JS Engine in Bug 1425462. In this bug we're going to add a non-cryptographyically secure jitter algorithm exclusively for the JS shell. This will give us behavior parity between the browser and the shell for testing; although obviously not security-parity.
I will only ask for a flag to disable the jitter during benchmarks.
We have some tests that observe the time and may sensitive to jitter, so we should be on the lookout for issues. The atomics tests (for better and worse) use sleep() and similar methods to coordinate actions among threads; it would be helpful if sleep(), which is only a shell function, did not have significant jitter - it's dicey enough as it is.
Priority: -- → P3
Comment on attachment 8955633 [details] Bug 1440539 Support time jitter in the JS Shell, and expose a function to enable it https://reviewboard.mozilla.org/r/224726/#review230752 ::: js/src/builtin/TestingFunctions.cpp:4940 (Diff revision 1) > +SetTimeResolution(JSContext* cx, unsigned argc, Value* vp) > +{ > + CallArgs args = CallArgsFromVp(argc, vp); > + RootedObject callee(cx, &args.callee()); > + > + args.rval().setUndefined(); nit: Could you move this to right before the 'return true' to match the general style of setting the return value right before the return? (Note: args.rval() is ignored on 'return false') ::: js/src/builtin/TestingFunctions.cpp:4943 (Diff revision 1) > + RootedObject callee(cx, &args.callee()); > + > + args.rval().setUndefined(); > + > + if (args.length() < 2) { > + ReportUsageErrorASCII(cx, callee, "Wrong number of arguments"); nit: can you use args.requireAtLeast() instead? ::: js/src/jsdate.cpp:1326 (Diff revision 1) > + // with trying to prevent an attacker from calculating the midpoint themselves. > + // So we use a very simple, very fast CRC with a hardcoded seed. > + > + uint64_t midpoint = *((uint64_t*)&clamped); > + midpoint ^= 0x0F00DD1E2BAD2DED; // XOR in a 'secret' > + // MurmurHash3 from chromium//third_party/WebKit/Source/platform/TimeClamper.cpp Searchfoxing murmur just now, it seems we have a few implementations lying around. Could we perhaps hoist one of them into mfbt (or use something already in mfbt)?
Comment on attachment 8955633 [details] Bug 1440539 Support time jitter in the JS Shell, and expose a function to enable it https://reviewboard.mozilla.org/r/224726/#review230752 > Searchfoxing murmur just now, it seems we have a few implementations lying around. Could we perhaps hoist one of them into mfbt (or use something already in mfbt)? It could, but I am loathe to do so in this bug. Technically we are not using MurmurHash3, but one of its internal functions. Even if we abstract Murmur in mfbt, exposing an internal function would be odd.
Assignee: nobody → tom
Comment on attachment 8955633 [details] Bug 1440539 Support time jitter in the JS Shell, and expose a function to enable it https://reviewboard.mozilla.org/r/224726/#review230794 Fair enough.
Attachment #8955633 - Flags: review?(luke) → review+
Keywords: checkin-needed
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again. hg error in cmd: hg rebase -s 1021640878958d55db50a684012e61fa87d2106e -d b8e64a94e635: rebasing 450473:102164087895 "Bug 1440539 Support time jitter in the JS Shell, and expose a function to enable it r=luke" (tip) merging js/src/builtin/TestingFunctions.cpp merging js/src/jsdate.cpp warning: conflicts while merging js/src/jsdate.cpp! (edit, then use 'hg resolve --mark') unresolved conflicts (see hg resolve, then hg rebase --continue)
Pushed by archaeopteryx@coole-files.de: https://hg.mozilla.org/integration/mozilla-inbound/rev/a66754828b42 Support time jitter in the JS Shell, and expose a function to enable it. r=luke
Keywords: checkin-needed
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: