1905094 - Improve signatures for JS context checks

Status: RESOLVED FIXED

(Socorro :: Signature, task, P3)

Description

[Andrew McCreight [:mccr8]]

In Nightly, we have special checks on JS contexts. There's a lot of infrastructure to do these checks, so all of these crashes get bucketed together which makes it harder to understand what is going on. I looked at the actual crashes we're hitting to come up with this list to improve things.

Irrelevant signature list

• JSContext::checkImpl: this is an "interior" function and is always paired with JSContext::check, so we don't need both. I think this should also match JSContext::checkImpl<T> which is also present.
• js::ContextChecks::check: we can sometimes end up with multiple frames of this, and when we fail, we also always have js::ContextChecks::fail on the top of the stack, so let's just use that and get rid of this one.

Prefix signature list

• JSContext::check: This frame is always right before the actual JS API call that is doing the check, so we want to include it in the signature so we know that we're failing a check, but we also want to see the JS API call in the signature. I think this should match JSContext::check<T> which is also present.
• js::ContextChecks::fail: When this is present, it seems to also always have a JSContext::check also in the stack, so it could probably go in the irrelevant signature list, but it is only a single frame so I'm just leaving it in. But we definitely need to prefix it.
• JS::Call, JS_CallFunctionValue: The above changes mean we'll end up with a JS API call at the end of the signature. This is not entirely ideal, as we really want to know the specific call site, but I also don't think we want to add every single JS API method to the signature regexps. In practice, we aren't likely to have a large number of distinct call sites. However, these two call methods are quite generic, so I'm adding them to the prefix list. I only see a single crash for JS_CallFunctionValue so it isn't a big deal there, but for JS::Call I do in fact see 3 separate call sites, so this will split them up nicely. This might also improve other signatures where these are present, but I don't see more than a few so it doesn't seem to actually be relevant.

Comment 1

[Andrew McCreight [:mccr8]]

https://github.com/mozilla-services/socorro/pull/6648

Comment 2

[Will Kahn-Greene [:willkg] ET needinfo? me]

Attachment: [mozilla-services/socorro] Bug 1905094 - Improve signatures for JS context checks. (#6648)

Comment 3

[Will Kahn-Greene [:willkg] ET needinfo? me]

willkg merged PR [mozilla-services/socorro]: Bug 1905094 - Improve signatures for JS context checks. (#6648) in fd0c242.

Comment 4

[Daniel Thorn [:relud]]

This was deployed in bug 1906174 just now.