286506 - warning: locally defined symbol ... imported in function ...

Status: RESOLVED FIXED

(Toolkit Graveyard :: XULRunner, defect)

Description

[Darin Fisher]

While linking xul.dll, we get a ton of these kinds of warnings:

docshell.lib(nsMIMEInfoImpl.obj) : warning LNK4217: locally defined symbol
?RemoveCStringA
t@nsCStringArray@@QAEHH@Z (public: int __thiscall
nsCStringArray::RemoveCStringAt(int)) im
ported in function "public: virtual unsigned int __stdcall
nsMIMEInfoBase::SetPrimaryExten
sion(class nsACString const &)"
(?SetPrimaryExtension@nsMIMEInfoBase@@UAGIABVnsACString@@@
Z)

I believe that these are caused because we do not compile with _IMPL_NS_COM=1
defined across all translation units that are to be linked into xul.dll.  I'm
not sure, but I think this means that the compiler is not optimizing the code as
well as it could since it'll think that it cannot encode a PC-relative jump for
the various function calls.  (I'd be surprised if the linker rewrote the code
for the various functions where this is happening.)  So, we could probably
improve performance and codesighs by fixing these warnings.

And, even if it doesn't make a significant performance improvement, it'd be nice
to clean up the excessive warnings ;-)

Comment 1

[Benjamin Smedberg]

That is a correct assesment of the problem, but it's not easy to fix. There are
static libs (rdfutil/unicharutil) that are currently linked into both libxul and
non-libxul units. I think this warning is not worth spending time on yet... when
the rest of libxul is done (tier 50) and when RDF resource factories are gone.

Comment 2

[Darin Fisher]

well.. solving this should be just as simple as defining _IMPL_NS_COM=1 in the
right modules (any that define LIBXUL_LIBRARY I would think).  that define has
no bearing on what is actually exported from libxul ;-)

i agree that we will see this warning for other things that are not part of
xpcom, but that should be a much smaller set.  i'd like to get the xpcom set
taken care of for gecko 1.8.

Comment 3

[Benjamin Smedberg]

LIBXUL_LIBRARY already defines _IMPL_NS_COM. For the vast majority of our code,
we do this correctly. The "hard parts" are the static libs I mentioned, which
are linked into both libxul and non-libxul code.

http://lxr.mozilla.org/mozilla/source/config/config.mk#479

Comment 4

[Darin Fisher]

Ah, so we do.  Then, what I see the problem being is that at least in a good
number of cases we do not define LIBXUL_LIBRARY when we should.  See for example
the directories under uriloader/.  Shouldn't we specify LIBXUL_LIBRARY in every
Makefile that builds object files that will be linked into xul.dll?

Comment 5

[Benjamin Smedberg]

Yes, we should list LIBXUL_LIBRARY in every Makefile that gets linked into
xul.dll, except for unicharutil/rdfutil (and maybe one or two others I'm
forgetting). Certainly uriloader should have it.

Comment 6

[Darin Fisher]

OK, then I'll write up a patch for that.

Comment 7

[Darin Fisher]

Attachment: add LIBXUL_LIBRARY=1 in some places where it is missing

Comment 8

[Darin Fisher]

fixed-on-trunk

resolving this bug as FIXED since the remaining warnings only pertain to
unicharutil and rdfutil, which i presume are to be handled in a separate bug.