search

qiuzijian / google-breakpad

Automatically exported from code.google.com/p/google-breakpad
0 stars 0 forks source link

Use of dyanmic_cast breaks build #379

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
r550 broke the Mac Chrome build.  (Maybe Linux Chrome too?)

Chrome builds with -fno-rtti.  r550 introduced a bunch of dynamic_casts into 
src/common/dwarf/dwarf2reader.cc.

dynamic_cast must be removed.  The comments where dynamic_cast is used say:

“    // dynamic_cast is prohibited by Google C++ Style Guide, but 
justified.”

I can understand this argument, but it’s unjustified in that it breaks our 
build.

Original issue reported on code.google.com by mark@chromium.org on 30 Mar 2010 at 4:32

GoogleCodeExporter commented 9 years ago 
Hi Mark
Tracking the trunk of Breakpad always carries this risk.  We can come up with a 
workaround but I think sticking to 
a known working revision of Breakpad is a standard(and safer) practice.

Original comment by neal...@gmail.com on 30 Mar 2010 at 4:35

GoogleCodeExporter commented 9 years ago 
Chrome doesn’t track the Breakpad trunk, but we needed to pick up r557, so we 
updated to that revision.

557 > 550, and I don’t think it’s worth branching Breakpad for this, I 
think we should just get rid of 
dynamic_cast.

Original comment by mark@chromium.org on 30 Mar 2010 at 4:38

GoogleCodeExporter commented 9 years ago 
We hit the same issue in Mozilla when we updated our Breakpad snapshot, since 
we also
compile with -fno-rtti. I wound up just enabling RTTI while compiling the DWARF
parsing code, since it's only used for dump_syms and not shipped to users:
http://mxr.mozilla.org/mozilla-central/source/toolkit/crashreporter/google-break
pad/src/common/dwarf/Makefile.in#60

Original comment by ted.mielczarek on 30 Mar 2010 at 4:39

GoogleCodeExporter commented 9 years ago 
Initial patch available here:
http://breakpad.appspot.com/73001/show

Original comment by jimbla...@gmail.com on 30 Mar 2010 at 4:55

GoogleCodeExporter commented 9 years ago 
Lightening up.  :)

Original comment by mark@chromium.org on 30 Mar 2010 at 4:56

GoogleCodeExporter commented 9 years ago 
However you guys want to go with this is fine with me.  Here's a patch that 
removes
the RTTI.
http://breakpad.appspot.com/74001

Original comment by jimbla...@gmail.com on 30 Mar 2010 at 7:24