Firefox 3.0b2 fails to compile with gcc 4.3

RESOLVED FIXED

Status

()

Firefox
Build Config
RESOLVED FIXED
11 years ago
10 years ago

People

(Reporter: Bernhard Rosenkraenzer, Assigned: Benjamin Smedberg)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(5 attachments)

(Reporter)

Description

11 years ago
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
(Reporter)

Comment 1

11 years ago
Created attachment 294204 [details] [diff] [review]
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.
(Assignee)

Comment 2

11 years ago
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
(Assignee)

Comment 3

11 years ago
Created attachment 303664 [details] [diff] [review]
Use -Wno-conversion when available, rev. 1
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?
(Assignee)

Comment 6

11 years ago
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.
(Assignee)

Comment 7

11 years ago
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.
(Assignee)

Comment 8

11 years ago
Created attachment 305541 [details] [diff] [review]
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
Assignee: nobody → benjamin
Status: NEW → ASSIGNED
Attachment #305541 - Flags: review?
(Assignee)

Comment 9

11 years ago
Created attachment 305542 [details] [diff] [review]
Fix stricter conversions in nsCharTraits.h, rev. 1
Attachment #305542 - Flags: review?(dbaron)
(Assignee)

Updated

11 years ago
Attachment #305541 - Flags: review? → review?(dwitte)

Comment 10

11 years ago
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+

Updated

11 years ago
Duplicate of this bug: 411442
(Assignee)

Comment 12

11 years ago
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+
(Assignee)

Updated

11 years ago
Attachment #305542 - Flags: approval1.9+
(Assignee)

Comment 15

11 years ago
Fixed on trunk. AFAIK this is all the necessary GCC 4.3 fixes, because we imported the breakpad fixes from upstream.
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
Created attachment 306054 [details] [diff] [review]
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.
(That said, it might be better to change the return type on nsCharTraits<T>::length.)
(Assignee)

Updated

11 years ago
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+
You need to log in before you can comment on or make changes to this bug.