Bug 689698

Thanks for the report.

As far as I can see, scan_token() isn't part of the public API of any of the
three libraries, so the duplicate symbol shouldn't present a problem. Linking to
a library version instead of the same symbol in your own application, on the
other hand, is no fun, but link order usually takes care of that.

We could rename the symbol in gs, but there are a bunch of related symbols that
should change too, so I'd rather not without a real conflict.

The fact that it's not part of the public API does not really influence the 
issue of symbol collision at all. It's not just the third party application 
that might use the wrong symbol, it's also your own library, that might load 
the symbol from either libt1 or libXfont, rather than its own. The way to fix 
that for _just_ your own library is to use -Bsymbolic. The proper way to fix 
this is to rename the symbol, and make sure that it doesn't collide with other 
libraries.

There's also another solution, maybe easier to handle than renaming, if you 
don't want to rename, and it is to hide the symbol, either using GCC's 
visibility (which could also help the compiler in optimisation, and is 
supported by other compilers like ICC and Sun's CC) or through ld scripts. 
Hiding all the symbols that are not part of public API this way would in all 
effects solve the problem, as then collisions between them and other libraries 
wouldn't be possible anymore, it would also improve startup times, as the 
symbols wouldn't have dynamic binding anymore. The problem with this solution 
is that you need GCC 4 (or 3.4 backported, which is unsafe) to have visibility 
support, or a GNU-compatible ld. It's still better than leaving the collision 
in place, though.

Please don't reopen WONTFIX bugs.

> The fact that it's not part of the public API does not really influence the 
> issue of symbol collision at all. It's not just the third party application 
> that might use the wrong symbol, it's also your own library, that might load 
> the symbol from either libt1 or libXfont, rather than its own.

Do you have an example of this? Given that the GNU toolchain has historically
exported everything, I can't imagine modern software functioning if this were
actually the case.

I am glad to hear gcc 4 has visibility support; that will be much nicer than the
old version script method, and I look forward to it becoming widespread enough
to be worthwhile. :)

Yes, gcc 4.x has visibility support... but OTOH, ghostscript runs on more
platforms where the compiler is not gcc. 

The actual possibility of collision is quite small though, because there 
are not many applications which uses libgs plus one of the others - try naming one? 

On those platforms that support it, you might want to use the '--default-symver'
linker flag. This will make all symbols versioned by the soname of the library.
It should not break anything, but will make sure that collisions in the names do
not actually occur (it also fixes the multiple versions of the same library
issue). Then again, using visibility where applicable is a speedup as well.

It appeared that Ralph already closed this as WONTFIX (but was re-opened).

In general, I think WONTFIX bugs should only be re-opened if the person re-opening it is willing to provide a patch for review (and we will review it if that happens), but we will not spend further time on it ourselves. 

Since this has already happened once, I am afraid if somebody re-opens this bug report without providing a patch, I'll considered disabling that person's bugzilla account. You have been warned.

Surely providing an example of a situation where this is a real problem would be sufficient grounds for re-opening this bug?  Threatening in advance to close people's accounts seems rather draconian.

So, here's a real-world example where this actually causes applications to crash:
https://bugzilla.redhat.com/show_bug.cgi?id=590914

 (In reply to comment #7)
> Surely providing an example of a situation where this is a real problem would
> be sufficient grounds for re-opening this bug?

> 
> So, here's a real-world example where this actually causes applications to
> crash:
> https://bugzilla.redhat.com/show_bug.cgi?id=590914

Sure - I take your point. It appearred Ralph has closed it twice before I did a 3rd time. Ralph is no longer active, so re-assign to support for decisions - there is no gaurantee that support won't close it a 4th time though...

Created attachment 7122 [details]
patch

It doesn't seem unreasonable (to me) for us to rename the function to 'scanner_get_token' (after all, the init function is scanner_init, not scan_init).

Here is a patch that does exactly that. Cluster testing now.

(In reply to comment #9)
> Created an attachment (id=7122) [details]
> patch
> 
> It doesn't seem unreasonable (to me) for us to rename the function to
> 'scanner_get_token' (after all, the init function is scanner_init, not
> scan_init).
> 
> Here is a patch that does exactly that. Cluster testing now.

Why aren't the exported/global functions prefixed with gs_

Henry I guess the answer is "Ralph wouldn't bother even listening to me until an application crashed", and after not being listened to I really couldn't get myself to create an artificial example of an application miscalling the functions. As it happens, Evince did so already.

In general yes I suggest you do prefix all the exported symbols with something like gs_, maybe gs_x_ or something for those that are not supposed to be used (and that could also help you as GNU ld, Apple's and (I think) Sun's allows you to hide symbols from the public table based on their prefix — not a problem for Windows since there you have to list explicitly what you do export).

The less generic a symbol name is the less likely it is to collide (and lead to crashes at one point or another).

Created attachment 7125 [details]
patch

Fixed patch. Passes cluster testing.

Fixed in revision 12023.

Thanks to the committers/commenters for their persistence on this - please let us know if this doesn't solve the issue for you.