108862 - unresolved symbols in link of libaccessibility.so

Status: VERIFIED FIXED

(SeaMonkey :: Build Config, defect)

Description

[Colin Blake]

In the OpenVMS build, I am now getting two unresolved symbols when I link
libaccessibility.so:

nsCaseInsensitiveStringComparator::operator()(const unsigned short *, const
unsigned short *, unsigned int) const
nsCaseInsensitiveStringComparator::operator()(unsigned short, unsigned short) const

This problem is caused by the fact that libaccessibility now needs to be linked
against libunicharutil_s (MOZ_UNICHARUTIL_LIBS).

I'll attach the patch to fix this next. I've also taken the liberty of moving
libgkconshared_s from EXTRA_DSO_LDOPTS to SHARED_LIBRARY_LIBS since the latter
appears to be the "correct" way to access this library.

I'd like to get this in today before we freeze for M0.9.6.

Comment 1

[Colin Blake]

Attachment: Patch to fix unresolved symbols

Comment 2

[Colin Blake]

Attachment: Cleaner version of patch

Comment 3

[Colin Blake]

Damn, I thought the freeze was tonight, not last night. Now I need to get driver
approval for this too.

Comment 4

[hacker formerly known as [e-mail]]

I wonder why none of our other resolve-symbols-at-link-time boxes are breaking
on this dependency.  I'm not so sure if you need to pull in all of
libgkconshared_s .  It should be just a glue lib like libunicharutil_s. 

Comment 5

[Colin Blake]

I believe the way I have it linking on OpenVMS is that when I encounter one of 
the _s libraries that I bring in everything from that library, not just the 
modules which are needed to resolve unresolved symbols at that time. I found 
that I needed to treat _s libraries this way, otherwise the .so they were going 
into wouldn't contain everything it ultimately needed (and cause unresolved 
symbol errors). 

Comment 6

[hacker formerly known as [e-mail]]

Um, if that's the case, then why can you get away with just linking against
libunicharutil_s without explicitly pulling in all of its symbols (which is what
SHARED_LIBRARY_LIBS does)?

When the _s lib is part of a larger composite library (usually of the same
module), then you need to make sure that you pull in all of the symbols.  If
it's just a glue library, then you only need to link against the necessary
symbols.  I'm fairly certain that gkconshared_s is just a glue lib when used
outside of content & layout.

Comment 7

[Colin Blake]

I guess no one has ever explained how all of this linking stuff is MEANT to 
work, and it sure isn't documented. Are you saying that there's a difference 
between something specified via EXTRA_DSO_LDOPTS and SHARED_LIBRARY_LIBS? Does 
one mean pull in all symbols and the other only pull in what you need? 

Comment 8

[hacker formerly known as [e-mail]]

The purpose of SHARED_LIBRARY_LIBS is to designate the sub-libraries that need
to have all of their symbols linked into the current library being built.  It
was introduced way back when we made an effort to reduce the number of shared
libraries what we loaded.  We use MKSHLIB_FORCE_ALL & MKSHLIB_UNFORCE_ALL to set
linker flags to turn on & off the inclusion of all of the static libraries
symbols when we can.  Otherwise, we use the "brute-force" method of extracting
the object files from the static libs and linking the individual objects into
the current library instead of the libs.

EXTRA_DSO_LDOPTS is just the extra flags that are used when linking the shared
library.  Libraries added to this variable have their symbols resolved or used
using the default method, which is usually as needed.

Comment 9

[Colin Blake]

Thanks for the explanation. And therein lies the problem, or at least the reason 
why only OpenVMS is seeing this issue. The way I have things linking, an _s 
library is always treated as a "load everything" library, regardless of how it 
got onto the ld command line.

Let me see how much work it'd be to do things "the right way".

Comment 10

[Colin Blake]

Chris,

I'm relinking now with a much better fix, one that uses MKSHLIB_FORCE_ALL and 
MKSHLIB_UNFORCE_ALL. I can't believe I never knew about these two defines.

If this full relink works I'll then kick off a full build to verify that I 
haven't broken anything else. Assuming all goes well, I should my very simple, 
OpenVMS specific, fix ready for checkin tomorrow afternoon.

Any idea when the branch is going to occur?

Comment 11

[hacker formerly known as [e-mail]]

I believe they plan to branch by Friday (tomorrow).

Comment 12

[Colin Blake]

I'm about to attach a new patch. This one does it right. Now I use -all and
-none on the link command lines to control when I need everything. As a result I
was able to remove a couple of other "fixes" that I'd previously had to put in
in order to work around linking issues.

Each change is OpenVMS specific. Any chance of a quick review so that I can ask
drivers to get this in before the branch?

Comment 13

[Colin Blake]

Attachment: New patch. Don't need to fix accessible/build any more!

Comment 14

[hacker formerly known as [e-mail]]

Comment on attachment 57134 [details] [diff] [review]
New patch. Don't need to fix accessible/build any more!

r=cls

Comment 15

[Asa Dotzler [:asa]]

a=asa (on behalf of drivers) for checkin to 0.9.6

Comment 16

[Colin Blake]

Checked in. Thanks for the fast approvals guys.

Comment 17

[Jon Granrose]

marking verified.