Latest merge causing build errors with gcc 4.6

RESOLVED FIXED in mozilla21

Status

()

defect
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: jwir3, Assigned: nrc)

Tracking

({regression})

unspecified
mozilla21
x86_64
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

Reporter

Description

7 years ago
I'm receiving the following errors on building fennec with gcc-4.6, ndk-r8c, and sdk-16:

    /usr/local/src/android-ndk-r8c/toolchains/arm-linux-androideabi-4.6/prebuilt/linux-x86/bin/../lib/gcc/arm-linux-androideabi/4.6/../../../../arm-linux-androideabi/bin/ld: /home/sjohnson/Source/mozilla-android/obj-android/mfbt/tests/TestWeakPtr.o: in function mozilla::RefCounted<mozilla::SupportsWeakPtr<C>::WeakReference>::Release():../../dist/include/mozilla/RefPtr.h:70: error: undefined reference to 'mozilla::RefCounted<mozilla::SupportsWeakPtr<C>::WeakReference>::dead'
    /usr/local/src/android-ndk-r8c/toolchains/arm-linux-androideabi-4.6/prebuilt/linux-x86/bin/../lib/gcc/arm-linux-androideabi/4.6/../../../../arm-linux-androideabi/bin/ld: /home/sjohnson/Source/mozilla-android/obj-android/mfbt/tests/TestWeakPtr.o: in function mozilla::RefCounted<mozilla::SupportsWeakPtr<C>::WeakReference>::~RefCounted():../../dist/include/mozilla/RefPtr.h:53: error: undefined reference to 'mozilla::RefCounted<mozilla::SupportsWeakPtr<C>::WeakReference>::dead'
    /usr/local/src/android-ndk-r8c/toolchains/arm-linux-androideabi-4.6/prebuilt/linux-x86/bin/../lib/gcc/arm-linux-androideabi/4.6/../../../../arm-linux-androideabi/bin/ld: /home/sjohnson/Source/mozilla-android/obj-android/mfbt/tests/TestWeakPtr.o: in function mozilla::RefCounted<mozilla::SupportsWeakPtr<A>::WeakReference>::Release():../../dist/include/mozilla/RefPtr.h:70: error: undefined reference to 'mozilla::RefCounted<mozilla::SupportsWeakPtr<A>::WeakReference>::dead'
    /usr/local/src/android-ndk-r8c/toolchains/arm-linux-androideabi-4.6/prebuilt/linux-x86/bin/../lib/gcc/arm-linux-androideabi/4.6/../../../../arm-linux-androideabi/bin/ld: /home/sjohnson/Source/mozilla-android/obj-android/mfbt/tests/TestWeakPtr.o: in function mozilla::RefCounted<mozilla::SupportsWeakPtr<A>::WeakReference>::~RefCounted():../../dist/include/mozilla/RefPtr.h:53: error: undefined reference to 'mozilla::RefCounted<mozilla::SupportsWeakPtr<A>::WeakReference>::dead'
    collect2: ld returned 1 exit status 

After using hg bisect to try and nail down the problem, I found the following changeset to be bad:

changeset:   120478:a4f8cb70cc5d
tag:         tip
parent:      120467:50cf5bbcb180
parent:      120477:44772e261a55
user:        Tim Taubert <ttaubert@mozilla.com>
date:        Fri Feb 01 10:17:01 2013 -0500
summary:     merge m-c to fx-team
I think nrc was seeing this as well, based on backscroll in #mobile. Not sure why your build would fail while the tbpl builders still build fine. What does your mozconfig look like?
Assignee

Comment 2

6 years ago
Yes, I did, and still do. Only on debug builds, as long as I enable opt it builds fine. Error is:

APKOpen.cpp
/home/ncameron/android-ndk-r8c/toolchains/arm-linux-androideabi-4.6/prebuilt/linux-x86/bin/../lib/gcc/arm-linux-androideabi/4.6/../../../../arm-linux-androideabi/bin/ld: /home/ncameron/gfx-android/mozglue/build/../android/APKOpen.o: in function mozilla::RefCounted<Zip>::Release():../../dist/include/mozilla/RefPtr.h:70: error: undefined reference to 'mozilla::RefCounted<Zip>::dead'
/home/ncameron/android-ndk-r8c/toolchains/arm-linux-androideabi-4.6/prebuilt/linux-x86/bin/../lib/gcc/arm-linux-androideabi/4.6/../../../../arm-linux-androideabi/bin/ld: /home/ncameron/gfx-android/mozglue/build/../linker/Zip.o: in function mozilla::RefCounted<Zip>::~RefCounted():../../dist/include/mozilla/RefPtr.h:53: error: undefined reference to 'mozilla::RefCounted<Zip>::dead'
collect2: ld returned 1 exit status
make[6]: *** [libmozglue.so] Error 1
make[5]: *** [libs] Error 2
make[4]: *** [libs_tier_base] Error 2
make[3]: *** [tier_base] Error 2
make[2]: *** [default] Error 2
make[1]: *** [realbuild] Error 2
make: *** [build] Error 2

MozConfig is:

mk_add_options MOZ_OBJDIR=@TOPSRCDIR@/../obj-android
mk_add_options MOZ_MAKE_FLAGS="-j12"
ac_add_options --enable-debug
ac_add_options --disable-tests
ac_add_options --disable-optimize
ac_add_options --with-android-ndk="$HOME/android-ndk-r8c"
ac_add_options --with-android-toolchain="$HOME/android-ndk-r8c/toolchains/arm-linux-androideabi-4.6/prebuilt/linux-x86"
ac_add_options --with-android-sdk="$HOME/android-sdk-linux/platforms/android-17"
ac_add_options --with-android-version=5
ac_add_options --enable-application=mobile/android
ac_add_options --target=arm-linux-androideabi
ac_add_options --with-ccache=/usr/bin/ccache
Reporter

Comment 3

6 years ago
It looks like, after doing a more thorough bisection, that the following is the first bad revision:

changeset:   120455:799de1cf584b
user:        Mike Hommey <mh+mozilla@glandium.org>
date:        Tue Jan 29 09:35:16 2013 +0100
summary:     Bug 834769 - Change the "destroyed" state value for RefCounted. r=Waldo
Blocks: 834769
Reporter

Updated

6 years ago
Keywords: regression
Assignee

Comment 4

6 years ago
Posted patch rename RefCounted::dead (obsolete) — Splinter Review
Attachment #709530 - Flags: review?(mh+mozilla)
Assignee

Updated

6 years ago
Attachment #709530 - Attachment is obsolete: true
Attachment #709530 - Flags: review?(mh+mozilla)
Assignee

Comment 5

6 years ago
Posted patch change dead field to a define (obsolete) — Splinter Review
Attachment #709533 - Flags: review?(mh+mozilla)
Comment on attachment 709533 [details] [diff] [review]
change dead field to a define

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

::: mfbt/RefPtr.h
@@ -54,5 @@
>  
>    public:
>      // Compatibility with nsRefPtr.
>      void AddRef() {
> -      MOZ_ASSERT(refCnt >= 0);

This test is there for good reasons, so changing the ref count type and removing it is not a great thing to do.
Attachment #709533 - Flags: review?(mh+mozilla) → review-
Assignee

Comment 7

6 years ago
Posted patch patch v2 (obsolete) — Splinter Review
Without the signed/unsigned change.
Attachment #709533 - Attachment is obsolete: true
Attachment #709639 - Flags: review?(mh+mozilla)
Comment on attachment 709639 [details] [diff] [review]
patch v2

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

LGTM, but I'm not a peer for MFBT.
Attachment #709639 - Flags: review?(mh+mozilla)
Attachment #709639 - Flags: review?(jwalden+bmo)
Attachment #709639 - Flags: feedback+
Comment on attachment 709639 [details] [diff] [review]
patch v2

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

Sigh, I guess C++ is screwy-ish in not allowing this.  Too bad we don't have constexpr yet; I think that'd get us around this trivially.

::: mfbt/RefPtr.h
@@ +41,5 @@
>   * use-after-destroy (refcount==0xffffdead).
>   */
> +#ifdef DEBUG
> +#define DEAD int(0xffffdead)
> +#endif

#defines leak, and leaking is going to come back to bite us.  In this case we could #undef when we're done, but it's still messy.  We'd also want to rename to MOZ_DEAD or something, too, to avoid conflicts.

I think the best workaround, rather than introducing a #define even temporarily, is to have a static const int that's *not* inside a class.  Then you can define it and use it without having to have a class-external definition and all the potential pain that might entail:

namespace detail {
static const int DEAD = 0xffffDEAD;
}

And then use detail::DEAD as necessary.
Attachment #709639 - Flags: review?(jwalden+bmo) → review-
Component: General → MFBT
Product: Firefox for Android → Core
Assignee

Comment 11

6 years ago
Posted patch patch v3Splinter Review
Assignee: nobody → ncameron
Attachment #709639 - Attachment is obsolete: true
Attachment #709985 - Flags: review?(jwalden+bmo)
Attachment #709985 - Flags: review?(jwalden+bmo) → review+
https://hg.mozilla.org/mozilla-central/rev/15dd558ca101
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
You need to log in before you can comment on or make changes to this bug.