Last Comment Bug 409027 - transformiix builds with conflicting visibility on 1.8 branch due to header name collision
: transformiix builds with conflicting visibility on 1.8 branch due to header n...
Status: VERIFIED FIXED
: verified1.8.1.12
Product: Core
Classification: Components
Component: Build Config (show other bugs)
: 1.8 Branch
: All All
: -- normal (vote)
: ---
Assigned To: Mark Mentovai
:
Mentors:
Depends on:
Blocks: 408959
  Show dependency treegraph
 
Reported: 2007-12-19 09:33 PST by Mark Mentovai
Modified: 2008-01-30 16:33 PST (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Rename transformiix' List.h to txList.h (8.66 KB, patch)
2007-12-19 09:39 PST, Mark Mentovai
ted: review+
dveditz: approval1.8.1.12+
Details | Diff | Splinter Review

Description Mark Mentovai 2007-12-19 09:33:11 PST
When building on the 1.8 branch, some parts of transformiix are built with default visibility (external linkage) while other parts are built with hidden visibility.  A linker that notices such things will complain, for example:

ld: warning vtable for txObjecthas different visibility (2) in ../../dist/lib/components/libtransformiix.a(txStylesheet.o) and (1) in ../../dist/lib/components/libtransformiix.a(txMozillaXSLTProcessor.o)
ld: warning txObject::~txObject()has different visibility (2) in ../../dist/lib/components/libtransformiix.a(txStylesheet.o) and (1) in ../../dist/lib/components/libtransformiix.a(txMozillaXSLTProcessor.o)

while linking CaminoStaticApp using Xcode 3.0 ld (sources compiled by gcc 4.0).

The problem is that transformiix includes a header called List.h, which it includes with |#include "List.h"|.  However, there's also an objdir/dist/include/system_wrappers/List.h intended to wrap a system List.h header, its contents are:

#pragma GCC system_header
#pragma GCC visibility push(default)
#include_next <List.h>
#pragma GCC visibility pop

When building transformiix, something that #includes "List.h" will actually include the header above due to its being first in the include path, and the #include_next will find transformiix' own copy of List.h.  Unfortunately, transformiix' List.h and any other header it includes will be included with default visibility.  The ld warnings above are produced when transformiix' List.h includes txCore.h and winds up with default-visibility versions of txObject being compiled into some objects.  transformiix code that gets txCore.h before List.h winds up with correct hidden-visibility versions of txObject.

The correct solution is to rename transformiix' List.h to txList.h and make the corresponding change to its List.cpp.  This change was made on the trunk when transformiix moved from extensions/transformiix into content/xslt in bug 304494.

This is a safe build-config-only change that we'd like for the 1.8 branch for Camino, as we're enabling hidden visibility for Camino 1.6.  This is the only core change that we need to enable hidden visibility for Camino 1.6.
Comment 1 Mark Mentovai 2007-12-19 09:39:12 PST
Created attachment 293878 [details] [diff] [review]
Rename transformiix' List.h to txList.h

This was done on the trunk as part of bug 304494.

The patch doesn't show the add/remove diffs; apply this patch and then do the "mv" commands above the diffs in the file.
Comment 2 Ted Mielczarek [:ted.mielczarek] 2007-12-19 11:53:20 PST
Comment on attachment 293878 [details] [diff] [review]
Rename transformiix' List.h to txList.h

r=me, I don't know if anyone cares about losing blame from the cvs remove/add, you might want to double check on that.
Comment 3 Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg] 2007-12-19 11:56:01 PST
It's on a branch, so we should not attempt to do any sort of CVS move.
Comment 4 Mark Mentovai 2007-12-19 12:27:05 PST
Comment on attachment 293878 [details] [diff] [review]
Rename transformiix' List.h to txList.h

I won't do a cvs move, but the checkin comment will include both the old and new paths for anyone looking forward or backward through cvs history.

This change is safe for the branch; the only change is that a header is renamed to avoid being accidentally wrapped in the wrong visibility pragma.
Comment 5 Daniel Veditz [:dveditz] 2007-12-21 11:51:41 PST
Comment on attachment 293878 [details] [diff] [review]
Rename transformiix' List.h to txList.h

approved for 1.8.1.12, a=dveditz for release-drivers
Comment 6 Mark Mentovai 2007-12-21 12:48:30 PST
Checked in on the MOZILLA_1_8_BRANCH.
Comment 7 Al Billings [:abillings] 2008-01-30 16:33:07 PST
Verified.

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