Closed Bug 409027 Opened 17 years ago Closed 17 years ago

transformiix builds with conflicting visibility on 1.8 branch due to header name collision

Categories

(Firefox Build System :: General, defect)

1.8 Branch
defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: mark, Assigned: mark)

References

Details

(Keywords: verified1.8.1.12)

Attachments

(1 file)

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.
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.
Assignee: nobody → mark
Status: NEW → ASSIGNED
Attachment #293878 - Flags: review?(ted.mielczarek)
Blocks: 408959
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.
Attachment #293878 - Flags: review?(ted.mielczarek) → review+
It's on a branch, so we should not attempt to do any sort of CVS move.
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.
Attachment #293878 - Flags: approval1.8.1.12?
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
Attachment #293878 - Flags: approval1.8.1.12? → approval1.8.1.12+
Checked in on the MOZILLA_1_8_BRANCH.
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Keywords: fixed1.8.1.12
Resolution: --- → FIXED
Verified.
Status: RESOLVED → VERIFIED
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.