If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Normalize nsCOMPtr<nsIFoo>... and friends in signatures.

RESOLVED WORKSFORME

Status

Socorro
Backend
RESOLVED WORKSFORME
8 years ago
2 months ago

People

(Reporter: jst, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

8 years ago
We've got several templetized classes in our code like nsCOMPtr, nsRefPtr, nsTArray, etc, in which we're rarely seeing crashes, but we frequently see crash signatures pointing to those classes. We also commonly see crashes that move from the same method but for a different template type, i.e. nsCOMPtr<nsIFoo>::~nsCOMPtr<nsIFoo>() can be the same crash as nsCOMPtr<nsIBar>::~nsCOMPtr<nsIBar>() in a different release. I'd suggest we normalize the types in those signatures to be 'T' and add every single one of them to the signature skip list.

Here's the code that I use (and I'm no python guy, so there might very well be a way cleaner way to do this):

    if signature.startswith("nsCOMPtr<") or \
            signature.startswith("nsRefPtr<") or \
            signature.startswith("nsAutoPtr<") or \
            signature.startswith("nsCharTraits<") or \
            signature.startswith("nsGetterAddRefs<") or \
            signature.startswith("getter_AddRefs<") or \
            signature.startswith("nsAutoTArray<") or \
            signature.startswith("nsTArray<") or \
            signature.startswith("nsTHashtable<") or \
            signature.startswith("ns_if_addref<"):
        depth = 0
        typestart = 0
        typeend = 0

        for i in range(8, len(signature) - 1):
            if signature[i] == '<':
                if typestart == 0:
                    typestart = i + 1

                depth += 1
            elif signature[i] == '>':
                depth -= 1

                if depth == 0:
                    if signature[i - 1] == ' ':
                        i -= 1

                    typeend = i

                    break

        type = signature[typestart:typeend]
        newsignature = signature.replace(type, "T").replace(' >', '>')

In addition, we should also simply skip signatures starting with either "nsInterfaceHashtable<" or "operator!=<", but doing the above normalization for those gets much trickier.
See also bug 530684.
I think the nsRefPtr<> type weirdness is a VC++ compiler bug. I've seen the goofy types in the stack even when looking at a stack in a Microsoft debugger. I'm guessing the compiler gets confused and writes out one set of debug info and uses it regardless of templated type.

Comment 3

8 years ago
It's not a bug, really, it's an intended side effect of COMDAT folding: if two functions have the same assembly, the linker picks one of them and folds them together, and then the debuginfo will be correct for only one of the folded functions. This happens a lot with our style of template metaprogramming on binary-identical types like nsCOMPtr and nsRefPtr.
Ah, that makes sense. Sure makes crash reports confusing currently. jst's suggestion makes sense then, we ought to just normalize all of the types out of these template types when we find them in a stack frame, from nsCOMPtr<Foo> -> nsCOMPtr<T>. I think we should do that for every frame in the report, not just signatures, since otherwise it's confusing.
Yup, agreed
(Assignee)

Updated

6 years ago
Component: Socorro → General
Product: Webtools → Socorro

Comment 6

6 years ago
If it's in *every* frame, not just signatures, then this is a processor change.
Component: General → Backend
QA Contact: socorro → backend

Comment 7

4 years ago
Is this still an issue?

Updated

2 months ago
Status: NEW → RESOLVED
Last Resolved: 2 months ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.