Cross compiling mfbt/tests/TestAtomics.cpp for MIPS fails due to undefined references to atomics.

RESOLVED FIXED in Firefox 45

Status

()

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: aleksandar.zlicic, Assigned: glandium)

Tracking

Trunk
mozilla45
Other
Linux
Points:
---

Firefox Tracking Flags

(firefox45 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Cross compilation of SpiderMonkey for MIPS fails when mfbt/tests/TestAtomics.cpp is compiled due to undefined references to symbols from libatomic.

A part of failed build log follows:

> TestArrayUtils
> /home/zlicic32/work/Mozilla-gecko-dev-2/js-build-mips32/_virtualenv/bin/python /home/zlicic32/work/Mozilla-gecko-dev-2/gecko-dev/config/expandlibs_exec.py --uselist --  mips-linux-gnu-g++ -EL  -Wall -Wsign-compare -Wtype-limits -Wno-invalid-offsetof -Wcast-align -fno-rtti -fno-exceptions -fno-math-errno -std=gnu++0x -pthread -pipe  -DDEBUG -DTRACING -g -O0 -fno-omit-frame-pointer   -o TestArrayUtils TestArrayUtils.o  -lpthread  -Wl,-z,noexecstack -Wl,-z,text -Wl,--build-id    -Wl,-rpath-link,../../dist/bin -Wl,-rpath-link,/usr/local/lib   ../../mfbt/libmfbt.a    -lm -ldl  
> TestAtomics.o
> mips-linux-gnu-g++ -EL -o TestAtomics.o -c  -I../../dist/system_wrappers -include /home/zlicic32/work/Mozilla-gecko-dev-2/gecko-dev/config/gcc_hidden.h -DIMPL_MFBT -DAB_CD= -DNO_NSPR_10_SUPPORT -I/home/zlicic32/work/Mozilla-gecko-dev-2/gecko-dev/mfbt/tests -I.  -I../../dist/include  -I../../dist/include/testing  -I/home/zlicic32/work/Mozilla-gecko-dev-2/js-build-mips32/dist/include/nspr        -fPIC   -DMOZILLA_CLIENT -include ../../js/src/js-confdefs.h -MD -MP -MF .deps/TestAtomics.o.pp  -Wall -Wsign-compare -Wtype-limits -Wno-invalid-offsetof -Wcast-align -fno-rtti -fno-exceptions -fno-math-errno -std=gnu++0x -pthread -pipe  -DDEBUG -DTRACING -g -O0 -fno-omit-frame-pointer      /home/zlicic32/work/Mozilla-gecko-dev-2/gecko-dev/mfbt/tests/TestAtomics.cpp
> TestAtomics
> /home/zlicic32/work/Mozilla-gecko-dev-2/js-build-mips32/_virtualenv/bin/python /home/zlicic32/work/Mozilla-gecko-dev-2/gecko-dev/config/expandlibs_exec.py --uselist --  mips-linux-gnu-g++ -EL  -Wall -Wsign-compare -Wtype-limits -Wno-invalid-offsetof -Wcast-align -fno-rtti -fno-exceptions -fno-math-errno -std=gnu++0x -pthread -pipe  -DDEBUG -DTRACING -g -O0 -fno-omit-frame-pointer   -o TestAtomics TestAtomics.o  -lpthread  -Wl,-z,noexecstack -Wl,-z,text -Wl,--build-id    -Wl,-rpath-link,../../dist/bin -Wl,-rpath-link,/usr/local/lib   ../../mfbt/libmfbt.a    -lm -ldl  
> Executing: mips-linux-gnu-g++ -EL -Wall -Wsign-compare -Wtype-limits -Wno-invalid-offsetof -Wcast-align -fno-rtti -fno-exceptions -fno-math-errno -std=gnu++0x -pthread -pipe -DDEBUG -DTRACING -g -O0 -fno-omit-frame-pointer -o TestAtomics /home/zlicic32/work/Mozilla-gecko-dev-2/js-build-mips32/mfbt/tests/tmpMmKIqC.list -lpthread -Wl,-z,noexecstack -Wl,-z,text -Wl,--build-id -Wl,-rpath-link,../../dist/bin -Wl,-rpath-link,/usr/local/lib -lm -ldl
> /home/zlicic32/work/Mozilla-gecko-dev-2/js-build-mips32/mfbt/tests/tmpMmKIqC.list:
>     INPUT("TestAtomics.o")
>     INPUT("../Compression.o")
>     INPUT("../Decimal.o")
>     INPUT("../Unified_cpp_mfbt0.o")
> 
> TestAtomics.o: In function `std::__atomic_base<unsigned long long>::load(std::memory_order) const':
> /home/zlicic32/work/toolchain/mips-2015.05/mips-linux-gnu/include/c++/4.9.2/bits/atomic_base.h:500: undefined reference to `__atomic_load_8'
> /home/zlicic32/work/toolchain/mips-2015.05/mips-linux-gnu/include/c++/4.9.2/bits/atomic_base.h:500: undefined reference to `__atomic_load_8'
> TestAtomics.o: In function `std::__atomic_base<unsigned long long>::fetch_add(unsigned long long, std::memory_order)':
> /home/zlicic32/work/toolchain/mips-2015.05/mips-linux-gnu/include/c++/4.9.2/bits/atomic_base.h:618: undefined reference to `__atomic_fetch_add_8'
> /home/zlicic32/work/toolchain/mips-2015.05/mips-linux-gnu/include/c++/4.9.2/bits/atomic_base.h:618: undefined reference to `__atomic_fetch_add_8'
> TestAtomics.o: In function `std::__atomic_base<unsigned long long>::fetch_sub(unsigned long long, std::memory_order)':
> /home/zlicic32/work/toolchain/mips-2015.05/mips-linux-gnu/include/c++/4.9.2/bits/atomic_base.h:628: undefined reference to `__atomic_fetch_sub_8'
> /home/zlicic32/work/toolchain/mips-2015.05/mips-linux-gnu/include/c++/4.9.2/bits/atomic_base.h:628: undefined reference to `__atomic_fetch_sub_8'
> TestAtomics.o: In function `std::__atomic_base<unsigned long long>::store(unsigned long long, std::memory_order)':
> /home/zlicic32/work/toolchain/mips-2015.05/mips-linux-gnu/include/c++/4.9.2/bits/atomic_base.h:478: undefined reference to `__atomic_store_8'
Posted patch Bug_1178266.patch (obsolete) — Splinter Review
Attached patch fixes the build for MIPS.
Comment on attachment 8627174 [details] [diff] [review]
Bug_1178266.patch

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

::: mfbt/tests/moz.build
@@ +60,5 @@
>  ]
> +
> +if CONFIG['CPU_ARCH'] == 'mips':
> +  OS_LIBS += [
> +    '-latomic',

You don't need "-l" here.

(facepalm, gcc uses intrisics that you need to link a library for?!)
Attachment #8627174 - Flags: feedback+
You're right, '-l' is not needed.

Regarding gcc atomic builtins, when lock free instructions are not available (either through hardware or OS support) atomic operations are left as function calls to be resolved by a library. This is the case with 64-bit atomic operations on a 32-bit mips.

So, yes, linking to atomic library in this case is necessary.
(In reply to Aleksandar Zlicic from comment #3)
> You're right, '-l' is not needed.
> 
> Regarding gcc atomic builtins, when lock free instructions are not available
> (either through hardware or OS support) atomic operations are left as
> function calls to be resolved by a library. This is the case with 64-bit
> atomic operations on a 32-bit mips.
> 
> So, yes, linking to atomic library in this case is necessary.

It seems pretty awful that it doesn't do it on its own, provided it's a detail of its implementation. It also appears to affect powerpc, on top of mips.
Assignee: nobody → mh+mozilla
Attachment #8627174 - Attachment is obsolete: true
Attachment #8654693 - Flags: review?(nfroyd)
Comment on attachment 8654693 [details] [diff] [review]
Link against libatomic when necessary

Turns out it's not enough.
Attachment #8654693 - Flags: review?(nfroyd)
(In reply to Mike Hommey [:glandium] from comment #6)
> Comment on attachment 8654693 [details] [diff] [review]
> Link against libatomic when necessary
> 
> Turns out it's not enough.

Because mfbt is not linked to e.g. libxul. So that worked for TestAtomics.cpp, but not other things.
I also met this problem. It could solve the build problem. Because I build it on Loongson 3A platform
Attachment #8654693 - Flags: review?(hwjeastd07)
Attachment #8654693 - Flags: review?(hwjeastd07) → review+
https://hg.mozilla.org/mozilla-central/rev/3ced6f84960c
Status: UNCONFIRMED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
As per comment 6 and 7, the patch does *not* fix the issue. And it's not been reviewed by a peer, so was not proper to land in the first place.
Status: RESOLVED → REOPENED
Ever confirmed: true
Resolution: FIXED → ---
Attachment #8654693 - Attachment is obsolete: true
Attachment #8693932 - Flags: review?(nfroyd)
Comment on attachment 8693932 [details] [diff] [review]
Link against libatomic when necessary

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

I'm assuming you've verified that our x86 and ARM builds don't trigger linking with -latomic.  (x86 has a 64-bit cmpxchg; not sure about ARM)
Attachment #8693932 - Flags: review?(nfroyd) → review+
https://hg.mozilla.org/mozilla-central/rev/39842091cf38
Status: REOPENED → RESOLVED
Closed: 4 years ago4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.