409384 - Firefox 3.0b2 fails to compile with gcc 4.3

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Bernhard Rosenkraenzer]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.6) Gecko/20070731 Firefox/2.0.0.6
Build Identifier: 3.0b2

Firefox 3.0b2 fails to compile with gcc 4.3 snapshots - at least part of the blame lies with firefox for not including the correct headers (e.g. <string> used to #include <cstring>, but doesn't anymore and was never guaranteed to --> missing prototypes for strlen(), memset() and friends).



Reproducible: Always

Steps to Reproduce:
1. Try to build firefox 3.0b2 with a gcc 4.3 snapshot
2. Watch it barf

Comment 1

[Bernhard Rosenkraenzer]

Attachment: Partial patch

This patch fixes various issues, but the build still ends up barfing with

c++ -o dump_syms -O2 -march=i586 -mtune=i686 -fomit-frame-pointer -fweb -frename-registers -fvisibility-inlines-hidden  host_dump_syms.o ../../../../../../../toolkit/crashreporter/google-breakpad/src/common/linux/libhost_breakpad_linux_common_s.a ../../../../../../../toolkit/crashreporter/google-breakpad/src/common/libhost_breakpad_common_s.a
../../../../../../../toolkit/crashreporter/google-breakpad/src/common/linux/libhost_breakpad_linux_common_s.a(host_dump_symbols.o): In function `(anonymous namespace)::ComputeSizeAndRVA(unsigned int, (anonymous namespace)::SymbolInfo*)':
dump_symbols.cc:(.text+0xd5d): undefined reference to `void std::sort<__gnu_cxx::__normal_iterator<(anonymous namespace)::SourceFileInfo**, std::vector<(anonymous namespace)::SourceFileInfo*, std::allocator<(anonymous namespace)::SourceFileInfo*> > >, std::pointer_to_binary_function<(anonymous namespace)::SourceFileInfo*, (anonymous namespace)::SourceFileInfo*, bool> >(__gnu_cxx::__normal_iterator<(anonymous namespace)::SourceFileInfo**, std::vector<(anonymous namespace)::SourceFileInfo*, std::allocator<(anonymous namespace)::SourceFileInfo*> > >, __gnu_cxx::__normal_iterator<(anonymous namespace)::SourceFileInfo**,std::vector<(anonymous namespace)::SourceFileInfo*, std::allocator<(anonymous namespace)::SourceFileInfo*> > >, std::pointer_to_binary_function<(anonymous namespace)::SourceFileInfo*, (anonymous namespace)::SourceFileInfo*, bool>)'
dump_symbols.cc:(.text+0xedb): undefined reference to `void std::sort<__gnu_cxx::__normal_iterator<(anonymous namespace)::FuncInfo**, std::vector<(anonymous namespace)::FuncInfo*, std::allocator<(anonymous namespace)::FuncInfo*> > >, std::pointer_to_binary_function<(anonymous namespace)::FuncInfo*, (anonymous namespace)::FuncInfo*, bool> >(__gnu_cxx::__normal_iterator<(anonymous namespace)::FuncInfo**, std::vector<(anonymous namespace)::FuncInfo*, std::allocator<(anonymous namespace)::FuncInfo*> > >, __gnu_cxx::__normal_iterator<(anonymous namespace)::FuncInfo**, std::vector<(anonymous namespace)::FuncInfo*, std::allocator<(anonymous namespace)::FuncInfo*> > >, std::pointer_to_binary_function<(anonymous namespace)::FuncInfo*, (anonymous namespace)::FuncInfo*, bool>)'
dump_symbols.cc:(.text+0xf51): undefined reference to `__gnu_cxx::__normal_iterator<(anonymous namespace)::FuncInfo**, std::vector<(anonymous namespace)::FuncInfo*, std::allocator<(anonymous namespace)::FuncInfo*> > > std::find_if<__gnu_cxx::__normal_iterator<(anonymous namespace)::FuncInfo**, std::vector<(anonymous namespace)::FuncInfo*, std::allocator<(anonymous namespace)::FuncInfo*> > >, std::binder1st<std::pointer_to_binary_function<(anonymous namespace)::FuncInfo*, (anonymous namespace)::FuncInfo*, bool> > >(__gnu_cxx::__normal_iterator<(anonymous namespace)::FuncInfo**, std::vector<(anonymous namespace)::FuncInfo*, std::allocator<(anonymous namespace)::FuncInfo*> > >, __gnu_cxx::__normal_iterator<(anonymous namespace)::FuncInfo**, std::vector<(anonymous namespace)::FuncInfo*, std::allocator<(anonymous namespace)::FuncInfo*> > >, std::binder1st<std::pointer_to_binary_function<(anonymous namespace)::FuncInfo*, (anonymous namespace)::FuncInfo*, bool> >)'
dump_symbols.cc:(.text+0x109e): undefined reference to `__gnu_cxx::__normal_iterator<(anonymous namespace)::SourceFileInfo**, std::vector<(anonymous namespace)::SourceFileInfo*, std::allocator<(anonymous namespace)::SourceFileInfo*> > > std::find_if<__gnu_cxx::__normal_iterator<(anonymous namespace)::SourceFileInfo**, std::vector<(anonymous namespace)::SourceFileInfo*, std::allocator<(anonymous namespace)::SourceFileInfo*> > >, std::binder1st<std::pointer_to_binary_function<(anonymous namespace)::FuncInfo*, (anonymous namespace)::SourceFileInfo*, bool> > >(__gnu_cxx::__normal_iterator<(anonymous namespace)::SourceFileInfo**, std::vector<(anonymous namespace)::SourceFileInfo*, std::allocator<(anonymous namespace)::SourceFileInfo*> > >, __gnu_cxx::__normal_iterator<(anonymous namespace)::SourceFileInfo**, std::vector<(anonymous namespace)::SourceFileInfo*, std::allocator<(anonymous namespace)::SourceFileInfo*> > >, std::binder1st<std::pointer_to_binary_function<(anonymous namespace)::FuncInfo*, (anonymous namespace)::SourceFileInfo*, bool> >)'
collect2: ld returned 1 exit status

I don't have the time to look into this at the moment.

Comment 2

[Benjamin Smedberg]

Probably needs another header added to system_headers.

Note that toolkit/crashreporter/google-breakpad is imported code and those fixes are already posted upstream at http://code.google.com/p/google-breakpad/issues/detail?id=238

I'll have a patch for -Wno-conversion here imminently

Comment 3

[Benjamin Smedberg]

Attachment: Use -Wno-conversion when available, rev. 1

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Why do we want to turn off useful warnings?  These catch bugs.  (Unless I'm confused about what -Wconversion covers.)

And what does turning off -Wconversion have to do with undefined references?

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Or are we turning off -Wconversion to fix a single warning in the cookie code, which uses warnings-as-errors?  Why not fix the warning?  What is it?

Comment 6

[Benjamin Smedberg]

dbaron, this turns off about 100,000 warnings issued during compilation of mozilla, of the form:

../../../dist/include/string/nsCharTraits.h:318: error: conversion to ‘PRUnichar’ from ‘int’ may alter its value
../../../dist/include/string/nsCharTraits.h:573: error: conversion to ‘char’ from ‘int’ may alter its value
../../../../src/netwerk/cookie/src/nsCookie.h:126: error: conversion to ‘PRPackedBool’ from ‘PRBool’ may alter its value

I think that most of these are bogus warnings that would require explicit casts to suppress manually, which would make our code less readable, not more correct.

Comment 7

[Benjamin Smedberg]

From reading online docs, the behavior of -Wconversion changed between gcc4.2 and 4.3: previously it would only warn on conversions in function calls with mismatched prototypes. Now it will issue warnings on any assignment.

Comment 8

[Benjamin Smedberg]

Attachment: Fix stricter conversions in netwerk/cookie, rev. 1

This is the alternate solution, to explicitly cast things... I'll have separate patches for netwerk/cookie and xpcom/string

Comment 9

[Benjamin Smedberg]

Attachment: Fix stricter conversions in nsCharTraits.h, rev. 1

Comment 10

[[e-mail]]

Comment on attachment 305541 [details] [diff] [review]
Fix stricter conversions in netwerk/cookie, rev. 1

r=dwitte

see also bug 411442. i had a patch there to just remove -Wconversion, but i'll dupe that bug to this one so you can take up the argument here.

Comment 12

[Benjamin Smedberg]

Comment on attachment 305541 [details] [diff] [review]
Fix stricter conversions in netwerk/cookie, rev. 1

Very low risk, gets us closer to building 1.9 with gcc 4.3 out of the box.

Comment 13

[Damon Sicore (:damons)]

Comment on attachment 305541 [details] [diff] [review]
Fix stricter conversions in netwerk/cookie, rev. 1

a1.9+=damons

Comment 14

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 305542 [details] [diff] [review]
Fix stricter conversions in nsCharTraits.h, rev. 1

r=dbaron

Comment 15

[Benjamin Smedberg]

Fixed on trunk. AFAIK this is all the necessary GCC 4.3 fixes, because we imported the breakpad fixes from upstream.

Comment 16

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: build fixes for x86_64

For what it's worth, these were the additional changes I needed to build with gcc 4.3 on x86_64.

Comment 17

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(That said, it might be better to change the return type on nsCharTraits<T>::length.)

Comment 18

[Christopher Aillon (sabbatical, not receiving bugmail)]

Comment on attachment 306054 [details] [diff] [review]
build fixes for x86_64

This patch is also needed to compile on ppc64.

Comment 19

[Christopher Aillon (sabbatical, not receiving bugmail)]

However, it doesn't fix sparc which is bug 417345

Comment 20

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 306054 [details] [diff] [review]
build fixes for x86_64

Fixes to compile on gcc4.3 on 64-bit platforms; pretty trivial.

Comment 21

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 306054 [details] [diff] [review]
build fixes for x86_64

a1.9=beltzner

Comment 22

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment 306054 [details] [diff] checked in to trunk.

Comment 23

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 303664 [details] [diff] [review]
Use -Wno-conversion when available, rev. 1

It sounds like this isn't needed anymore, and I think we do want this warning.

Comment 24

[(not currently active) Ted Mielczarek]

Comment on attachment 303664 [details] [diff] [review]
Use -Wno-conversion when available, rev. 1

Looks ok to me.