1490743 - [clang, visual studio debugging] Identification of a concrete class in a general pointer variable doesn't work any more

Status: NEW

(Firefox Build System :: Toolchains, defect)

Description

[Honza Bambas (:mayhemer)]

STR:
- Windows 10, up to date m-c, bootstrapped
- build m-c with .mozconfig [1]
- put a breakpoint in visual studio at [2], expected to hit on the parent process only
- run firefox.exe from inside the vs ide (or attach, will likely make no difference)
- the breakpoint will likely hit very soon, or just load an arbitrary page, if not
- hover over or locate in the Autos window the mURI (nsCOMPtr<nsIURI>) member of the class (declared in HttpChannelBase base class)
- try to examine the mRawPtr (the final URI class)

ER:
- nsHttpChannel->mURI.mRawPtr is revealed as nsStandardURL class with its members

AR:
- nsHttpChannel->mURI.mRawPtr is only seen as nsISupports (not even as nsIURI!) and I can't examine the content

Note that adding (nsStandardURL*)(value of mRawPtr) to Watches shows what's expected, confirming the address is correct.


Filing as a blocker as I really can't work now.  Feel free to drop the severity, but I would appreciate some look at this soon.


[1] 
ac_add_options --build-backends=-VisualStudio
ac_add_options --host=x86_64-pc-mingw32
ac_add_options --target=x86_64-pc-mingw32
export MOZ_DEBUG_SYMBOLS=1
ac_add_options --enable-warnings-as-errors
ac_add_options --enable-application=browser
ac_add_options --enable-debug
ac_add_options --disable-optimize
ac_add_options --disable-crashreporter
ac_add_options --enable-tests
ac_add_options --enable-chrome-format=flat


[2] https://searchfox.org/mozilla-central/rev/9e7995b3c384945740985107a4de601b27904718/netwerk/protocol/http/nsHttpChannel.cpp#3912

Comment 1

[Honza Bambas (:mayhemer)]

my VS IDE version is 15.8.4 (the latest)

Comment 2

[Honza Bambas (:mayhemer)]

In general it seems to me that the debugger now simply uses the first base class of the type of the variable.  I've also seen this: an argument of type nsAHttpTransaction* keeping nsHttpTransaction final class translates to nsSupportsWeakReference* - the first base class of nsAHttpTransaction.

Comment 3

[(Away)]

(In reply to Honza Bambas (:mayhemer) from comment #2)
> In general it seems to me that the debugger now simply uses the first base
> class of the type of the variable.  I've also seen this: an argument of type
> nsAHttpTransaction* keeping nsHttpTransaction final class translates to
> nsSupportsWeakReference* - the first base class of nsAHttpTransaction.

Confirmed in WinDbg too: we're not being offered `vtcast` views of these pointers. MSVC on the left, clang on the right: https://irccloud.mozilla.com/file/EgMPnKIR/vtcast (This was from the Win64 debug builds from https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&selectedJob=198925654&revision=0b59dc25e8749f34590f2e5b29f3906f98f29730)

Comment 4

[(Away)]

I filed this upstream as https://bugs.llvm.org/show_bug.cgi?id=38944.

Comment 5

[(Away)]

Attachment: Possible workaround

Per LLVM's IRC, -fno-limit-debug-info might work around the issue. Honza, does this improve things for you?

Comment 6

[Honza Bambas (:mayhemer)]

Comment on attachment 9008915 [details] [diff] [review]
Possible workaround

Yes, this fixes the problem!  Thanks.  It seems (unconfirmed in more scientific way) to slightly prolong build times.

Comment 7

[Honza Bambas (:mayhemer)]

Comment on attachment 9008915 [details] [diff] [review]
Possible workaround

This workaround seems to stop working recently again.

I can give STR if wanted.

Comment 8

[(Away)]

(In reply to Honza Bambas (:mayhemer) from comment #7)
> Comment on attachment 9008915 [details] [diff] [review]
> Possible workaround
> 
> This workaround seems to stop working recently again.
> 
> I can give STR if wanted.

Yes please.

Comment 9

[Honza Bambas (:mayhemer)]

(In reply to David Major [:dmajor] from comment #8)
> (In reply to Honza Bambas (:mayhemer) from comment #7)
> > Comment on attachment 9008915 [details] [diff] [review]
> > Possible workaround
> > 
> > This workaround seems to stop working recently again.
> > 
> > I can give STR if wanted.
> 
> Yes please.

Please consider comment 7 unconfirmed now.  I can't reproduce any more during my debug sessions with the patch applied and lost the STR from comment 7 already.

Comment 10

[Jan Henning [:JanH]]

Is there anything that would keep this from finally landing?

Comment 11

[Honza Bambas (:mayhemer)]

David, can we please land this? I used to have this patch in mq, but now I use hg bookmarks - it's hard to have custom patches with that workflow.

This is a valid workaround for a clang bug and NOT having it in the tree actually disallows me from doing much work. Thanks.

Comment 12

[(Away)]

I'm not comfortable landing this change during code freeze. Upstream says this flag comes with a substantial cost in file size and build speed. Remember that this flag is a big-hammer workaround for a bug, and not a true "fix".

Ideally the upstream bug would get properly fixed -- I'll prod it. Failing that, maybe we can try this early next cycle, but in the meantime you should be able to export CFLAGS in your mozconfig regardless of mq/bookmarks/etc.

Comment 13

[Honza Bambas (:mayhemer)]

Thanks for the tip. Adding:

mk_add_options "export CFLAGS=-fno-limit-debug-info"
mk_add_options "export CXXFLAGS=-fno-limit-debug-info"

to my mozconfig solves the problem locally for me.

Please advise how to resolve this bug (WONTFIX, INCOMPLETE or somehow leave open dependent on the clang issue)

Comment 14

[(Away)]

Now that we have a fresh cycle, I'm willing to give it a try. We'll see if the sheriffs yell at me...

Comment 15

[(Away)]

Attachment: Work around missing type information in Windows debug symbols

See https://bugs.llvm.org/show_bug.cgi?id=38944. clang thought it could safely leave out type information for some of our classes, but that information would have been helpful for debugging. The flag in this patch turns off the debug-info limiting.

Comment 16

[Honza Bambas (:mayhemer)]

Comment on attachment 9008915 [details] [diff] [review]
Possible workaround

Confirmed that adding

mk_add_options "export CFLAGS=-fno-limit-debug-info"
mk_add_options "export CXXFLAGS=-fno-limit-debug-info" 

to my mozconfig fixes the problem.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/70ce7207292e
Work around missing type information in Windows debug symbols r=chmanchester

Comment 18

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/70ce7207292e

Comment 19

[Raul Gurzau (:RaulG)]

Backed out changeset 70ce7207292e (bug 1490743) for increasing build times on Windows platforms. a=backout

Backout link: https://hg.mozilla.org/mozilla-central/rev/df256c98c3fdb56de9bc2900803a6c4d58671100

https://treeherder.mozilla.org/perf.html#/alerts?id=20008

Comment 20

[BugBot [:suhaib / :marco]]

In the process of migrating remaining bugs to the new severity system, the severity for this bug cannot be automatically determined. Please retriage this bug using the new severity system.