Closed Bug 1521462 Opened 6 years ago Closed 5 years ago

Don't report irrelevant bottom stack frames in Untrusted Modules ping

Categories

(External Software Affecting Firefox :: Telemetry, enhancement, P3)

enhancement

Tracking

(firefox66 affected)

RESOLVED WONTFIX
Tracking Status
firefox66 --- affected

People

(Reporter: ccorcoran, Unassigned)

References

(Blocks 1 open bug)

Details

(Whiteboard: inj+)

In the Untrusted Modules ping, every stack frame reported begins with two frames:

    mozglue!long patched_LdrLoadDll(wchar_t*, ulong*, struct _UNICODE_STRING*, void**)+0x1ec
    kernelbase!LoadLibraryExW+0x161

These will always be the same and shouldn't be sent.

Correction: We shouldn't eliminate the LoadLibrary frame; patched_LdrLoadDll could be called from different places.

Note about calculating the # of frames to skip. First, the theoretical real stack trace at this site (with 0 skip frames) is:

mozglue!mozilla::glue::UntrustedDllsHandlerImpl::OnAfterTopLevelModuleLoad+0x491
mozglue!mozilla::glue::UntrustedDllsHandler::OnAfterModuleLoad+0x242
mozglue!patched_LdrLoadDll+0xbc
...

Our current code skips 1 frame, and with this we are seeing:

mozglue!patched_LdrLoadDll+0xbc
...

It seems the optimizer is eliminating one stack frame, likely the static call to OnAfterModuleLoad. On my local builds this frame is not optimized out.

Ideally we could search the stack for patched_LdrLoadDll, but the address in the stack trace doesn't point to the top of the function, so there's no reliable way to know which stack frame actually points to that function without something fuzzy like searching the first N frames for the closest address to patched_LdrLoadDll.

Another idea is to determine the return address within patched_LdrLoadDll or OnAfterModuleLoad (not sure if compilers support this). I'll have to dig into this.

I'm going to do more research and get back with a solution on this.

Priority: P1 → P3

Matt, how important do you think this is? Is it worth doing?

Assignee: ccorcoran → nobody
Blocks: 1435773
Flags: needinfo?(mhowell)
Whiteboard: inj+

It's easy enough to find these frames and handle them appropriately in the symbolicated stacks we have to query against that I'm not sure going to the trouble of doing this on the client side is worth it.

Flags: needinfo?(mhowell)

I agree... the only think that I can think of is that it might reduce the computational load on the server side, but I don't know whether this would make a sufficiently significant dent in that. I'll resolve this as WONTFIX for now, and we can always revisit it in the future.

Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → WONTFIX
Component: General → Telemetry
Product: Core → External Software Affecting Firefox
Version: Trunk → unspecified
You need to log in before you can comment on or make changes to this bug.