Last Comment Bug 737398 - Linux 64-bit debug builds of WebRTC use non-PIC stl function on builders
: Linux 64-bit debug builds of WebRTC use non-PIC stl function on builders
Status: RESOLVED FIXED
[qa-]
:
Product: Core
Classification: Components
Component: WebRTC (show other bugs)
: Trunk
: x86_64 Linux
: -- normal (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
:
Mentors:
Depends on:
Blocks: 643692 688178
  Show dependency treegraph
 
Reported: 2012-03-20 06:37 PDT by Randell Jesup [:jesup]
Modified: 2012-07-27 10:32 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
filter out most C/C++ compiler flags from the gyp files to avoid conflicts with our code (visibility, etc) (5.78 KB, patch)
2012-03-21 10:12 PDT, Randell Jesup [:jesup]
ted: feedback+
Details | Diff | Review

Description Randell Jesup [:jesup] 2012-03-20 06:37:32 PDT
From https://tbpl.mozilla.org/php/getParsedLog.php?id=10204463&tree=Alder&full=1

...  
/usr/bin/ld: ../../media/webrtc/trunk/third_party/libjingle/libjingle_libjingle/source/talk/base/httpclient.o: relocation R_X86_64_PC32 against `std::pair<std::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::basic_string<char, std::char_traits<char>, std::allocator<char> > >::~pair()' can not be used when making a shared object; recompile with -fPIC
/usr/bin/ld: final link failed: Bad value


This is the function in question, pulled out of above:
std::pair<std::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::basic_string<char, std::char_traits<char>, std::allocator<char> > >::~pair()

Apparently that function is not built as PIC, at least in the GCC version on the builders.  On my Fedora15 machine, I get no errors when building x64 debug.

(Irony: for once the OS and hardware fields autofills in bugzilla are right)
Comment 1 Ted Mielczarek [:ted.mielczarek] 2012-03-20 11:03:32 PDT
This error message is usually wrong. Usually it's a symbol visibility problem, and you have to add a header to the system-headers file:
http://mxr.mozilla.org/mozilla-central/source/config/system-headers

(Although this is an STL header, so there may be something else here.)
Comment 2 Mike Hommey [:glandium] 2012-03-20 11:07:57 PDT
> (Although this is an STL header, so there may be something else here.)

There's something fishy here. I got Randell to send me preprocessed source and corresponding objects, and they don't match what I get when I build the preprocessed source with the same gcc version we use on the buildbots. I have requested access to the buildbot Randell has access to, in order to look for myself.
Comment 3 Randell Jesup [:jesup] 2012-03-21 09:37:42 PDT
Mike found this was due to compiler CFLAG options added by the .gyp* files, including visibility options for inlines.  So this ends up being a bug in the current state of bug 643692.  Solution is probably to filter out most of the compiler flags, with a few specific exceptions (gtk pkg-config options, some MMX/SSE2 flags).
Comment 4 Ted Mielczarek [:ted.mielczarek] 2012-03-21 09:39:41 PDT
Yuck. That was definitely something I was unhappy with when I got that working. The GYP files want to define a full set of C[XX]FLAGS, whereas our build system provides most of the ones we need. We could try just dropping the C[XX]FLAGS from GYP entirely and see what breaks. Maybe we could persuade them to put things like pkg-config bits into a separate variable that gets included into CXXFLAGS, so we could use it separately?
Comment 5 Randell Jesup [:jesup] 2012-03-21 10:12:49 PDT
Created attachment 608002 [details] [diff] [review]
filter out most C/C++ compiler flags from the gyp files to avoid conflicts with our code (visibility, etc)
Comment 6 Randell Jesup [:jesup] 2012-03-21 10:20:14 PDT
(In reply to Ted Mielczarek [:ted] from comment #4)
> Yuck. That was definitely something I was unhappy with when I got that
> working. The GYP files want to define a full set of C[XX]FLAGS, whereas our
> build system provides most of the ones we need. We could try just dropping
> the C[XX]FLAGS from GYP entirely and see what breaks. Maybe we could
> persuade them to put things like pkg-config bits into a separate variable
> that gets included into CXXFLAGS, so we could use it separately?

Already did it. :-)  Builds clean, no bad relocs on ia32 or on my F15 x64 (will need to check x64 debug on a builder, but I'm pretty confident).  Only allows through gtk pkg-config, -mmmx and (for a few specific files) -msse2.
Comment 7 Ted Mielczarek [:ted.mielczarek] 2012-03-21 10:32:25 PDT
Comment on attachment 608002 [details] [diff] [review]
filter out most C/C++ compiler flags from the gyp files to avoid conflicts with our code (visibility, etc)

This doesn't seem great, but it's not the worst thing. Will upstream take this kind of patch?

Can you just remove that block of code in mozmake.py instead of commenting it out? No point in leaving it there.
Comment 8 Timothy B. Terriberry (:derf) 2012-03-21 10:36:01 PDT
(In reply to Ted Mielczarek [:ted] from comment #7)
> This doesn't seem great, but it's not the worst thing. Will upstream take
> this kind of patch?

Even if they don't, I'm sure they're willing to work with us on a solution they do find acceptable.
Comment 9 Ted Mielczarek [:ted.mielczarek] 2012-03-21 10:41:57 PDT
Comment on attachment 608002 [details] [diff] [review]
filter out most C/C++ compiler flags from the gyp files to avoid conflicts with our code (visibility, etc)

Yeah, the concept seems fine, I guess I'm just hung up on the naming.
Comment 10 Randell Jesup [:jesup] 2012-03-21 12:19:04 PDT
I can remove the block, though it does document what would be done 'normally' - perhaps I'll just replace it with a comment.

try build (linux 32 & 64 debug) almost done, if it's clean I'll put it on alder.
Comment 11 Randell Jesup [:jesup] 2012-03-21 14:51:08 PDT
Try build for linux debug and linux x64 debug was green

https://hg.mozilla.org/projects/alder/rev/71b868787bae

Note You need to log in before you can comment on or make changes to this bug.