Note: There are a few cases of duplicates in user autocompletion which are being worked on.

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




Build Config
10 years ago
10 years ago


(Reporter: Mark Mentovai, Assigned: Mark Mentovai)



1.8 Branch

Firefox Tracking Flags

(Not tracked)



(1 attachment)



10 years ago
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

10 years ago
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.
Assignee: nobody → mark
Attachment #293878 - Flags: review?(ted.mielczarek)


10 years ago
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 4

10 years ago
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, a=dveditz for release-drivers
Attachment #293878 - Flags: approval1.8.1.12? → approval1.8.1.12+

Comment 6

10 years ago
Checked in on the MOZILLA_1_8_BRANCH.
Last Resolved: 10 years ago
Keywords: fixed1.8.1.12
Resolution: --- → FIXED
Keywords: fixed1.8.1.12 → verified1.8.1.12
You need to log in before you can comment on or make changes to this bug.