1497645 - Improve pdb support in MinGW-Clang pdbs

Status: RESOLVED FIXED

(Firefox Build System :: General: Unsupported Platforms, enhancement, P5)

Description

[Tom Ritter [:tjr]]

We tested the pdbs created by Bug 1475562 in this push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8be85246a96c68aa33d31b4ee704a3871c77c050

While they work; we found a few issues:

1) The size is suspiciously small. We'd expect it to be about 5x the size.
2) Local variable information seems to be missing. (Probably explains the size...)
3) Function names are mangled. A couple examples. c++filt and llvm-undname can't unmangle them, so perhaps WinDbg is trying to unmangle them, failing, and showing the manged name?

> xul!ZN7mozilla6layers17CheckerboardEvent25UpdateRendertracePropertyENS1_19RendertracePropertyERKNS_3gfx9RectTypedINS_8CSSPixelEfEERKNSt3__112basic_stringIcNS9_11char_traitsIcEENS9_9allocatorIcEEEE+0x57

> xul!ZN7mozilla6layers22AsyncPanZoomController17AdvanceAnimationsERKNS_9TimeStampE+0xda

4) We noted that the debugger shows "(pdb symbols)" instead of the usual "(private pdb symbols)" -- the former is what you'd expect from a stripped pdb. (Probably related to the missing variables.)


We aren't running clang trunk; rather the 7 release branch.  In Bug 1495539 we'll be switching over to revision 342383 in trunk, and subsequently we can bump that revision up and/or try out new patches. So I'll try this out again in a few days after I've landed a few patches.

Comment 1

[Martin Storsjö]

(In reply to Tom Ritter [:tjr] from comment #0)
> We tested the pdbs created by Bug 1475562 in this push:
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=8be85246a96c68aa33d31b4ee704a3871c77c050
> 
> While they work; we found a few issues:
> 
> 1) The size is suspiciously small. We'd expect it to be about 5x the size.
> 2) Local variable information seems to be missing. (Probably explains the
> size...)

Ok, thanks for testing! No clue about this - I have only tested the PDBs a little, but I remember being able to inspect some local variables in a trivial-sized example in Visual Studio. I presume you've built all object files with both -g and -gcodeview?

> 3) Function names are mangled. A couple examples. c++filt and llvm-undname
> can't unmangle them, so perhaps WinDbg is trying to unmangle them, failing,
> and showing the manged name?
> 
> > xul!ZN7mozilla6layers17CheckerboardEvent25UpdateRendertracePropertyENS1_19RendertracePropertyERKNS_3gfx9RectTypedINS_8CSSPixelEfEERKNSt3__112basic_stringIcNS9_11char_traitsIcEENS9_9allocatorIcEEEE+0x57
> 
> > xul!ZN7mozilla6layers22AsyncPanZoomController17AdvanceAnimationsERKNS_9TimeStampE+0xda

It seems like the itanium name after the ! is lacking the leading underscore; with an added leading underscore, c++filt can unmangle it for me.

> 4) We noted that the debugger shows "(pdb symbols)" instead of the usual
> "(private pdb symbols)" -- the former is what you'd expect from a stripped
> pdb. (Probably related to the missing variables.)
> 
> 
> We aren't running clang trunk; rather the 7 release branch.  In Bug 1495539
> we'll be switching over to revision 342383 in trunk, and subsequently we can
> bump that revision up and/or try out new patches. So I'll try this out again
> in a few days after I've landed a few patches.

I don't expect there to be any significant changes wrt this since the 7 release branch, afaik; I think I tested this aspect last time around the 7 branch.

Comment 2

[(Away)]

(In reply to Martin Storsjö from comment #1)
> > > xul!ZN7mozilla6layers22AsyncPanZoomController17AdvanceAnimationsERKNS_9TimeStampE+0xda
> 
> It seems like the itanium name after the ! is lacking the leading
> underscore; with an added leading underscore, c++filt can unmangle it for me.

For me, llvm-undname complains even if I add the leading underscore.

My theory is that the string in the binary does contain the underscore, but the debugger fails to demangle it as C++, and assumes it's a C mangling with a leading underscore, which it then removes.

Comment 3

[Martin Storsjö]

(In reply to David Major [:dmajor] from comment #2)
> (In reply to Martin Storsjö from comment #1)
> > > > xul!ZN7mozilla6layers22AsyncPanZoomController17AdvanceAnimationsERKNS_9TimeStampE+0xda
> > 
> > It seems like the itanium name after the ! is lacking the leading
> > underscore; with an added leading underscore, c++filt can unmangle it for me.
> 
> For me, llvm-undname complains even if I add the leading underscore.

Ah, right. llvm-undname only supports the MSVC C++ ABI mangling, which this isn't.

> My theory is that the string in the binary does contain the underscore, but
> the debugger fails to demangle it as C++, and assumes it's a C mangling with
> a leading underscore, which it then removes.

Ok, if it's for i686, removing the underscore might be understandable for a tool to do (but in those cases, the itanium symbol name would have had two leading underscores).

Comment 4

[Tom Ritter [:tjr]]

(In reply to Martin Storsjö from comment #1)
> Ok, thanks for testing! No clue about this - I have only tested the PDBs a
> little, but I remember being able to inspect some local variables in a
> trivial-sized example in Visual Studio. I presume you've built all object
> files with both -g and -gcodeview?

Noooo, I need both?  I was building with only -gcodeview =)  I will add -g !

Comment 5

[Martin Storsjö]

(In reply to Tom Ritter [:tjr] from comment #4)
> (In reply to Martin Storsjö from comment #1)
> > Ok, thanks for testing! No clue about this - I have only tested the PDBs a
> > little, but I remember being able to inspect some local variables in a
> > trivial-sized example in Visual Studio. I presume you've built all object
> > files with both -g and -gcodeview?
> 
> Noooo, I need both?  I was building with only -gcodeview =)  I will add -g !

Indeed, I spent more than a reasonable amount of time chasing the arguments through the innards of clang until I realized I was missing a plain "-g" at some point as well. That's why I try to make it clear at https://github.com/mstorsjo/llvm-mingw#pdb-support, to avoid others to make the same mistake. :P

Comment 6

[(Away)]

(In reply to Martin Storsjö from comment #3)
> Ah, right. llvm-undname only supports the MSVC C++ ABI mangling, which this isn't.

Presumably tools like WinDbg etc. will only understand the MSVC mangling as well. Is it possible to convince mingw-clang to use that style of mangling in the PDBs?

Comment 7

[Martin Storsjö]

(In reply to David Major [:dmajor] from comment #6)
> (In reply to Martin Storsjö from comment #3)
> > Ah, right. llvm-undname only supports the MSVC C++ ABI mangling, which this isn't.
> 
> Presumably tools like WinDbg etc. will only understand the MSVC mangling as
> well. Is it possible to convince mingw-clang to use that style of mangling
> in the PDBs?

I wouldn't expect it to be easy. The differences between the C++ ABIs run very deep all throughout the code. I guess it might be possible to just do a demangle+mangle right before writing the string though, but I'm not sure if that'd be enough to get legible names, or if that's a rabbit hole of trying to rewrite the whole ABI.

While MinGW environments gradually can gain more and more features of the native/MSVC ones, the C++ ABI in general is a rather hard barrier, unfortunately. I've also been told that the Visual Studio debugger makes some assumptions about the binary layout of the MSVC C++ ABI.

Comment 8

[(Away)]

Interestingly, the push in bug 1475562 comment 20, with `-g`, fixes most of the symbol names, but not the stdlib ones. Also, local variables are now present, even though xul.pdb is still much smaller than I'd expect.

0:040> k8
 # Child-SP          RetAddr           Call Site
00 00000000`1213f050 00000000`05e62a82 xul!ZNSt3__19__num_putIcE23__widen_and_group_floatEPcS2_S2_S2_RS2_S3_RKNS_6localeE+0x31b
01 00000000`1213f130 00000000`05ea692b xul!ZNKSt3__17num_putIcNS_19ostreambuf_iteratorIcNS_11char_traitsIcEEEEE6do_putES4_RNS_8ios_baseEcd+0x2e2
02 00000000`1213f2a0 00000000`0566ec99 xul!ZNSt3__113basic_ostreamIcNS_11char_traitsIcEEElsEd+0xfb
03 00000000`1213f320 00000000`0566eb67 xul!mozilla::layers::CheckerboardEvent::LogInfo+0x99 [/builds/worker/workspace/build/src/obj-firefox/gfx/layers//builds/worker/workspace/build/src/gfx/layers/apz/src/CheckerboardEvent.cpp @ 112]
04 00000000`1213f3a0 00000000`056c495a xul!mozilla::layers::CheckerboardEvent::UpdateRendertraceProperty+0x57 [/builds/worker/workspace/build/src/obj-firefox/gfx/layers//builds/worker/workspace/build/src/gfx/layers/apz/src/CheckerboardEvent.cpp @ 91]
05 00000000`1213f430 00000000`05fd6667 xul!mozilla::layers::AsyncPanZoomController::AdvanceAnimations+0xda [/builds/worker/workspace/build/src/obj-firefox/gfx/layers//builds/worker/workspace/build/src/gfx/layers/apz/src/AsyncPanZoomController.cpp @ 3779]
06 00000000`1213f4f0 00000000`0571e2c4 xul!mozilla::layers::APZSampler::SampleAnimations::<unnamed-tag>::operator()+0x37 [/builds/worker/workspace/build/src/obj-firefox/gfx/layers//builds/worker/workspace/build/src/gfx/layers/apz/src/APZSampler.cpp @ 127]
07 00000000`1213f530 00000000`0569efea xul!mozilla::layers::ForEachNode<mozilla::layers::ForwardIterator,mozilla::layers::LayerMetricsWrapper,`lambda at /builds/worker/workspace/build/src/gfx/layers/TreeTraversal.h:185:32',`lambda at /builds/worker/workspace/build/src/gfx/layers/apz/src/APZSampler.cpp:123:7'>+0xb4 [/builds/worker/workspace/build/src/obj-firefox/gfx/layers//builds/worker/workspace/build/src/gfx/layers/TreeTraversal.h @ 146]
0:040> .frame 4
04 00000000`1213f3a0 00000000`056c495a xul!mozilla::layers::CheckerboardEvent::UpdateRendertraceProperty+0x57 [/builds/worker/workspace/build/src/obj-firefox/gfx/layers//builds/worker/workspace/build/src/gfx/layers/apz/src/CheckerboardEvent.cpp @ 91]
0:040> dv
           this = 0x00000000`022f1000
      aProperty = UserVisible (0n4)
          aRect = 0x00000000`1213f450
     aExtraInfo = 0x00000000`1213f470
           lock = class mozilla::MonitorAutoLock

Comment 9

[Jacek Caban]

(In reply to David Major [:dmajor] from comment #8)
> Interestingly, the push in bug 1475562 comment 20, with `-g`, fixes most of
> the symbol names, but not the stdlib ones.

We use static libc++, so it ends up linked into xul.dll, but it's built without debugging information. I guess we could change toolchain build script to build libc++ with debug info.

Comment 10

[(not currently active) Ted Mielczarek]

That'd definitely help for debugging purposes. Since we aren't shipping these builds it's not critical, just nice-to-have.

Comment 11

[Tom Ritter [:tjr]]

Did we address all the outstanding issues? Were there other things you noticed dmajor?

Comment 12

[(Away)]

Seems fine, it was good enough that I was able to debug those two startup crashes. :)