1270832 - turn on debug mode for libstdc++ headers

Status: REOPENED

Description

[Nathan Froyd [:froydnj]]

STL usages have crept into the tree and will continue to creep in, especially
as we move to a C++11 STL implementation everywhere.  Let's at least enable a
bit of checking for those data structures so we're not completely shooting
ourselves in the foot.

Comment 1

[Nathan Froyd [:froydnj]]

Attachment: turn on debug mode for libstdc++ headers

This patch seems to work; the FIXME appears to be no longer applicable with
newer GCC:

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

Comment 2

[Mike Hommey [:glandium]]

Comment on attachment 8749640 [details] [diff] [review]
turn on debug mode for libstdc++ headers

Review of attachment 8749640 [details] [diff] [review]:
-----------------------------------------------------------------

::: config/gcc-stl-wrapper.template.h
@@ -45,5 @@
>  
>  #if defined(DEBUG) && !defined(_GLIBCXX_DEBUG)
>  // Enable checked iterators and other goodies
> -//
> -// FIXME/bug 551254: gcc's debug STL implementation requires -frtti.

Is this comment outdated?

Comment 3

[Nathan Froyd [:froydnj]]

(In reply to Mike Hommey [:glandium] from comment #2)
> Comment on attachment 8749640 [details] [diff] [review]
> turn on debug mode for libstdc++ headers
> 
> Review of attachment 8749640 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: config/gcc-stl-wrapper.template.h
> @@ -45,5 @@
> >  
> >  #if defined(DEBUG) && !defined(_GLIBCXX_DEBUG)
> >  // Enable checked iterators and other goodies
> > -//
> > -// FIXME/bug 551254: gcc's debug STL implementation requires -frtti.
> 
> Is this comment outdated?

This comment appears to stem from bug 551254 comment 30, but the libstdc++ debug mode documentation makes no mention of the -frtti constraint:

https://gcc.gnu.org/onlinedocs/libstdc++/manual/debug_mode_using.html#debug_mode.using.mode

The only instances of |typeid| I can find in the debug headers are all guarded by preprocessor checks that RTTI is enabled.  Spelunking in libstdc++ leads me to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=40160 , which would have been fixed somewhere in the GCC 4.5 timeframe.  So yes, I think this comment is outdated.  OK to commit, then?

Comment 4

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/fe3f7921e2dc

Comment 5

[Pulsebot]

Backout:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1d918e13194c

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ccff1c4580ab

Comment 7

[Nathan Froyd [:froydnj]]

(In reply to Pulsebot from comment #6)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/ccff1c4580ab

This revision landed with a change to ignore debug headers for static analysis.  We're not running the static analysis build, so I figure that not having the debug-ness turned on here is not a bad thing.

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/ccff1c4580ab

Comment 9

[Mike Hommey [:glandium]]

Backed out in
https://hg.mozilla.org/integration/mozilla-inbound/rev/a640e6fa8ab9

Comment 10

[Nathan Froyd [:froydnj]]

The fuzzing team was interested in this; they may also be interested in comment 9, and the commit message for the backout.