289010 - Want to statically link nsVoidArray+nsCOMArray into strict-API binaries.

Status: RESOLVED FIXED

(Core :: XPCOM, defect, P2)

Description

[Benjamin Smedberg]

One of the major obstacles to adoption of the XPCOM glue around the tree now is
the common usage of nsVoidArray and nsCOMArray. I'm now confused about what
happens if we were to include nsVoidArray.o and nsCOMArray.o in the XPCOM glue.
Would every moz component (dynamic) take the hit? Or only strict-api/glue
consumers? Do we need yet another (xpcom_util) lib?

Comment 1

[Darin Fisher]

You can use the glue library for this.  Only strict-api components will take the
hit.

What happens today is that components like libnecko.so import glue symbols (like
nsCOMPtr::~nsCOMPtr) from libxpcom_core.so instead of linking against
libxpcomglue_s.a.

I'm of the opinion that we should move many of our data structures into
libxpcomglue_s.a (and libxpcomglue.a), so that they may be used by strict-api
components.  Remember: the GNU linker is able to discard .o files that are not
referenced when linking against a static lib, so we can put a lot in
libxpcomglue_s.a without worrying that it may bloat certain consumers who don't
care about the extra stuff.  I think the Microsoft linker is even smarter about
pruning out unnecessary code.

Comment 2

[Benjamin Smedberg]

Want the basic parts of this for the 1.8 cycle, to support various real-world
clients who have appeared out of the woodwork in the past few weeks.

Comment 3

[Christian :Biesinger (don't email me, ping me on IRC)]

do we want this for all of xpcom/ds?

Comment 4

[Benjamin Smedberg]

Not the entire directory, a decent part of that dir is interface impls that are
accessed through the component manager (e.g. nsSupportsPrimitives).

My current list for dicussion is:

nsVoidArray
nsCOMArray
nsDeque
nsCRT
pldhash
the templatized hashtables
nsEscape

Note that none of these are frozen; I'm not quite sure whether the headers ought
to live in the SDK or not. It's not a binary-compatibility issue, but it is a
code-compatibility issue.

Comment 5

[Darin Fisher]

we probably do for most of it... and we probably want it for parts of other
xpcom chunks.  we need to make sure that any code added to the glue library
works properly with the frozen string API.  that may impact nsStringArray and so on.

Comment 6

[Darin Fisher]

I sort of think that our SDK should have separate header file directories for
core and glue.  That way, if you compile against glue, you know that you also
need to link against the glue library.

Comment 7

[Benjamin Smedberg]

*** Bug 207248 has been marked as a duplicate of this bug. ***

Comment 8

[Benjamin Smedberg]

Attachment: voidarray/comarray/quicksort

Comment 9

[Benjamin Smedberg]

Attachment: Related patch

Comment 10

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 193178 [details] [diff] [review]
Related patch

r=shaver.

I can't believe there's precedent for "strspnp".  What a world.

Comment 11

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 193176 [details]
voidarray/comarray/quicksort

r=shaver

Comment 12

[Darin Fisher]

Comment on attachment 193178 [details] [diff] [review]
Related patch

Some inconsistent indentation in xpcom/glue/objs.mk

thanks for adding Truncate to nsStringAPI.h... i had meant to do that :)

sr=darin

Comment 13

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Will this limit our ability to much around with nsVoidArray in any way? I.e.
would these classes get statically linked into extensions/plugins which could
potentially hand us back pointers to these objects making us crash when we treat
the old versions of the classes as newer versions on the trunk?

Comment 14

[Benjamin Smedberg]

If we're handing around voidarray pointers across module boundaries that should
be fixed. This actually makes it *easier* for extensions to use voidarray
because they are not subject to our internal API shifts.

Comment 15

[Darin Fisher]

Provided that people linking against xpcomglue take care not to export any
xpcomglue symbols from their DSO or executable image, we should be fine.  The
problem is that an embedder using xpcomglue has to be careful to somehow hide
the symbols from the Gecko process.  This is easy to do if you are creating a
DSO under Linux as you can use a linker script to only export NSGetModule.  On
Windows, you generally don't have to worry since symbols are hidden by default.
 The problem I don't know how to deal with is an embedding application
(executable) on Linux that links to xpcomglue.  As far as I can tell there's no
good way to hide those symbols from Gecko.  In the past, I've had to create a
DSO that is loaded by my executable to get around this problem.  I suspect that
few embedders will know to avoid this problem in advance of discovering it when
xpcomglue changes occur between Gecko releases.

Comment 16

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

The issue i'm worried about is something like this:

1. Embedder statically links to xpcomglue which includes nsVoidArray
2. Embedder compiles and ships
3. We change the structure of nsVoidArray
4. Embedder upgrades to newer version of gecko without recompiling
5. Embedder calls some API in gecko that requires an nsVoidArray
6. We expect an nsVoidArray with the new structure but embedder is still running
   old code and has instantiated the older version of nsVoidArray
7. Things go bad

Comment 17

[Benjamin Smedberg]

>  The problem I don't know how to deal with is an embedding application
> (executable) on Linux that links to xpcomglue.  As far as I can tell there's no
> good way to hide those symbols from Gecko.  In the past, I've had to create a

On the contrary, the NS_COM_GLUE macro explicitly marks these symbols as
ELF-hidden, so there is no possibility of accidentally sharing them.

> 5. Embedder calls some API in gecko that requires an nsVoidArray

This should never happen, because there should not be any kind of gecko frozen
API or even non-frozen interface that references nsvoidarray.

Comment 18

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

> This should never happen, because there should not be any kind of gecko frozen
> API or even non-frozen interface that references nsvoidarray.

Doh! Of course.

Comment 19

[Darin Fisher]

> On the contrary, the NS_COM_GLUE macro explicitly marks these symbols as
> ELF-hidden, so there is no possibility of accidentally sharing them.

Have you verified this with a testcase?  (It definitely doesn't work in a debug
build because we don't put NS_COM_GLUE on many things that get inlined away in a
release build.)  I was not using Gecko 1.8 at the time when I tested this, so
perhaps we are simply better now.

Comment 20

[Benjamin Smedberg]

Fixed on trunk, yay!