In order to help get a better understanding of what is causing corruption in Bug 1367896, I propose landing this diagnostic patch in nightly. Based on observations of crashes in Bug 1367896, it appears that the JSScript::data buffer is somehow corrupted. This buffer allocated outside the GC. A number of crash signatures show that accessing JSScript::scopes()->vector[i] is either an outright memory violation or accessing other data. Since both |vector| and its target are in the same buffer that experiences corruption, I propose we check that |vector| still points within the buffer. The patch checks this condition in js::XDRScript (decode + encode) and js::CopyScript.

Does this seem reasonable to try?

Comment on attachment 8910992 [details] [diff] [review] 0001-Bug-1402167-Check-JSScript-data-integrity-in-XDR-r-n.patch Review of attachment 8910992 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jsscript.cpp @@ +82,5 @@ > + uint8_t* ptr = reinterpret_cast<uint8_t*>(sa->vector); > + > + // Check scope data - who's pointer is stored in data region - also points > + // within the data region. > + MOZ_DIAGNOSTIC_ASSERT(ptr >= script->data && nit: Use a MOZ_RELEASE_ASSERT, as this assertion ensure safety, and the performance of computing it is likely neglectable compared to decoding/encoding the JSScript. follow-up: check other ScriptData fields as well?

Comment on attachment 8910992 [details] [diff] [review] 0001-Bug-1402167-Check-JSScript-data-integrity-in-XDR-r-n.patch Review of attachment 8910992 [details] [diff] [review]: ----------------------------------------------------------------- This seems like a good start. My only concern is that these crashes have been so infrequent on nightlies that it might be weeks before we get any useful data unless we uplift to beta and use release asserts.

Will make them release asserts and land in Nightly. I'm not going to check other fields for now since they are more involved and are unlikely to tell us more. One improvement could be to check that the scope list is sane. If this doesn't explode on nightly, I think a beta uplift is worth doing. The scopes() check is extremely cheap and if the assertion would trip, we are at serious memory corruption and wont make it much further anyways. Before that I should probably clarify what the experiment will tell us.

Turned the DIAGNOSTIC_ASSERT into a RELEASE_ASSERT.

leave-open until we get results and decide next steps.

Pushed by tcampbell@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/ddff620ec86a Check JSScript::data integrity in XDR r=nbp

This diagnostic has only been hit once in Beta. Current efforts at mitigation and diagnostics are in Bug 1418894, so I am closing this bug.

Removing leave-open keyword from resolved bugs, per :sylvestre.