1156300 - FixFilenameCase doesn't fix up directory names

Status: RESOLVED FIXED

(Toolkit :: Crash Reporting, defect)

Description

[(Away)]

The file links on this crash report point to 'libglesv2' when it should be 'libGLESv2': https://crash-stats.mozilla.com/report/index/0f483195-ad7b-4096-a594-47b212150404

Comment 1

[(not currently active) Ted Mielczarek]

I think nowadays we could use GetFinalPathNameByHandle:
https://msdn.microsoft.com/en-us/library/windows/desktop/aa364962.aspx

Comment 3

[(not currently active) Ted Mielczarek]

This bug is lacking context, but:
https://dxr.mozilla.org/mozilla-central/rev/aa90f482e16db77cdb7dea84564ea1cbd8f7f6b3/toolkit/crashreporter/tools/symbolstore.py#733

When we dump symbols from PDB files we attempt to normalize the case of source filenames to match what's on the filesystem. Visual C++ tries its hardest to make life difficult for us by using arbitrary case. Unfortunately until recently there weren't actually any APIs for getting the canonical filename case, so this is a PITA.

Comment 6

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Who do I need to bribe to get this fixed?

Comment 7

[Nicholas Nethercote [inactive]]

I was just stymied by this bug. See https://bugzilla.mozilla.org/show_bug.cgi?id=1262351#c6.

Comment 8

[(not currently active) Ted Mielczarek]

I will fix this when I'm back home with my Windows machine on Friday.

Comment 9

[(not currently active) Ted Mielczarek]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=dd359dac5063

Comment 10

[(not currently active) Ted Mielczarek]

I did `mach buildsymbols` with this patch locally, and grepping xul.sym file it produced shows lines like:
```
FILE 40997 hg:192.168.1.10//build/mozilla-central:dom/indexedDB/ActorsChild.h:d9b1a9829c8e
```

So this seems to do the trick. (Also I wrote tests for it, and they pass, so that's nice.) Try push just to sanity check I didn't break anything in weird ways.

Comment 11

[(not currently active) Ted Mielczarek]

Attachment: MozReview Request: bug 1156300 - symbolstore.py FixFilenameCase doesn't fix up directory names. r?gps

Review commit: https://reviewboard.mozilla.org/r/45451/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/45451/

Comment 12

[(not currently active) Ted Mielczarek]

Apparently Python 3 has os._getfinalpathname on Windows that calls this under the hood:
https://hg.python.org/cpython/file/v3.5.1/Modules/posixmodule.c#l3801

That would have made this patch easier!

Comment 13

[Gregory Szorc [:gps]]

Comment on attachment 8739945 [details]
MozReview Request: bug 1156300 - symbolstore.py FixFilenameCase doesn't fix up directory names. r?gps

https://reviewboard.mozilla.org/r/45451/#review42025

::: toolkit/crashreporter/tools/symbolstore.py:765
(Diff revision 1)
> +            size = ctypes.windll.kernel32.GetFinalPathNameByHandleW(handle,
> +                                                                    None,
> +                                                                    0,
> +                                                                    0)
> +            buf = ctypes.create_unicode_buffer(size)
> +            if ctypes.windll.kernel32.GetFinalPathNameByHandleW(handle,
> +                                                                buf,
> +                                                                size,
> +                                                                0) > 0:

I would have allocated a fixed length buffer and called GetFinalPathNameByHandleW once then sliced. But meh.

Comment 14

[(not currently active) Ted Mielczarek]

https://reviewboard.mozilla.org/r/45451/#review42025

> I would have allocated a fixed length buffer and called GetFinalPathNameByHandleW once then sliced. But meh.

Unfortunately this is the only sensible way to handle arbitrary path sizes. I, too, hate the Win32 API for this, but it's the best you can do with C.

Comment 15

[(not currently active) Ted Mielczarek]

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #9)
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=dd359dac5063

I downloaded the symbols from this build to further sanity check and they look sensible:
```
FILE 174525 hg:hg.mozilla.org/try:dom/indexedDB/IndexedDatabase.h:c92a58752328
```

Comment 16

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7fb9838b488a

Comment 17

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/7fb9838b488a

Comment 18

[(not currently active) Ted Mielczarek]

I did a test crash and it looks good:
https://crash-stats.mozilla.com/report/index/23ba3e17-ff30-41e8-96b9-8db122160412