Closed Bug 409384 Opened 12 years ago Closed 12 years ago

Firefox 3.0b2 fails to compile with gcc 4.3

Categories

(Firefox Build System :: General, defect)

x86
Linux
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: bero, Assigned: benjamin)

References

Details

Attachments

(5 files)

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
Component: General → Build Config
QA Contact: general → build.config
Attached patch Partial patchSplinter Review
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.
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
Status: UNCONFIRMED → NEW
Ever confirmed: true
Attachment #303664 - Flags: review?(dbaron)
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?
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?
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.
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.
This is the alternate solution, to explicitly cast things... I'll have separate patches for netwerk/cookie and xpcom/string
Assignee: nobody → benjamin
Status: NEW → ASSIGNED
Attachment #305541 - Flags: review?
Attachment #305541 - Flags: review? → review?(dwitte)
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.
Attachment #305541 - Flags: review?(dwitte) → review+
Duplicate of this bug: 411442
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.
Attachment #305541 - Flags: approval1.9?
Comment on attachment 305541 [details] [diff] [review]
Fix stricter conversions in netwerk/cookie, rev. 1

a1.9+=damons
Attachment #305541 - Flags: approval1.9? → approval1.9+
Comment on attachment 305542 [details] [diff] [review]
Fix stricter conversions in nsCharTraits.h, rev. 1

r=dbaron
Attachment #305542 - Flags: review?(dbaron) → review+
Attachment #305542 - Flags: approval1.9+
Fixed on trunk. AFAIK this is all the necessary GCC 4.3 fixes, because we imported the breakpad fixes from upstream.
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
For what it's worth, these were the additional changes I needed to build with gcc 4.3 on x86_64.
(That said, it might be better to change the return type on nsCharTraits<T>::length.)
Attachment #306054 - Flags: review?(benjamin) → review+
Comment on attachment 306054 [details] [diff] [review]
build fixes for x86_64

This patch is also needed to compile on ppc64.
However, it doesn't fix sparc which is bug 417345
Comment on attachment 306054 [details] [diff] [review]
build fixes for x86_64

Fixes to compile on gcc4.3 on 64-bit platforms; pretty trivial.
Attachment #306054 - Flags: approval1.9?
Comment on attachment 306054 [details] [diff] [review]
build fixes for x86_64

a1.9=beltzner
Attachment #306054 - Flags: approval1.9? → approval1.9+
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.
Attachment #303664 - Flags: review?(dbaron) → review-
Comment on attachment 303664 [details] [diff] [review]
Use -Wno-conversion when available, rev. 1

Looks ok to me.
Attachment #303664 - Flags: review+
Component: Build Config → General
Product: Firefox → Firefox Build System
You need to log in before you can comment on or make changes to this bug.