Opened 12 years ago

Closed 11 years ago

Last modified 11 years ago

#5170 closed Feature Requests (fixed)

Some local functions have external linkage

Reported by: Luke Moore <luke.moore@…> Owned by: Anthony Williams
Milestone: To Be Determined Component: thread
Version: Boost Development Trunk Severity: Problem
Keywords: Cc:

Description

The thread_proxy and tls_destructor functions in libs/thread/src/pthread/thread.cpp have external linkage (the symbols are exported in the final library). However, these functions are only intended for use inside this compilation unit. Similar functions in once.cpp also have unintended external linkage, and potentially others in other areas of boost. A consequence of these symbols being visible is that it's hard to use two different versions of boost.Thread in the same binary, even when one of the versions of boost has been wrapped in a namespace to avoid name collisions. The name collisions still occur for thread_proxy, tls_destructor, and the others because they are declared as extern "C", so their names are not mangled.

It may seem unusual to attempt to use two different versions of boost in the same program, but it is a scenario our customers are running into more and more often and I'd like to give some explanation of why it happens. As more development projects start to use boost, more libraries start to link against it. A program that attempts to use multiple of those libraries won't work if they use different versions of boost. When one or more of these libraries is available in binary form and not source form, it is difficult to ensure the boost versions match. As a result, our customers attempt to version boost's symbols themselves for the boost version they use by compiling a version of boost wrapped in a namespace. Unfortunately, symbols from extern "C" functions are not versioned.

The thread_proxy, tls_destructor, etc., functions in question need to be declared as extern "C" since they are passed as callbacks into the pthreads library, and the calling conventions for C and C++ may differ on some platforms (http://lists.boost.org/Archives/boost/2011/01/175910.php).

Prior to revision 40424, these functions were declared as static. However, in revision 40424 the static specifier was removed and they were put inside an unnamed namespace. However, despite the claim at http://stackoverflow.com/questions/2594178/why-do-you-need-extern-c-for-c-callbacks-to-c-functions that "[the extern "C" function] is in an unnamed namespace, and not accessible outside the translation unit.", these functions do have external linkage, at least on g++ Linux systems. The bug at http://gcc.gnu.org/bugzilla/show_bug.cgi?id=28409 mentions that "extern C should make the function have external linkage in the object file. Namespaces have no impact on extern "C" functions, except from the point of view of lexical scoping", and this can be confirmed by looking for thread_proxy in the output of nm -D.

It it possible to please add back the static specifier to the extern "C" functions in question? Attached is a patch file showing the proposed changes.

Attachments (1)

thread.patch (1.8 KB ) - added by Luke Moore <luke.moore@…> 12 years ago.
Patch to add static specifier to extern "C" functions

Download all attachments as: .zip

Change History (6)

by Luke Moore <luke.moore@…>, 12 years ago

Attachment: thread.patch added

Patch to add static specifier to extern "C" functions

comment:1 by viboes, 11 years ago

Type: BugsFeature Requests

I would not consider this as a bug on Boost.Thread, but a feature requests, as you are using the library in context that are outside of what the author designed it for. Please be free to change if you don't agree.

comment:2 by luke.moore@…, 11 years ago

Whether it's categorized as a bug or a feature request doesn't matter so much to me -- I only care that the patch is applied. I truly think it is a bug, though. The original version of this code made these symbols private and the intention of the current code is to make them private. The only reason they are not private is because of misunderstandings about how extern "C" functions work inside unnamed namespaces.

comment:3 by Neil Groves, 11 years ago

This is a significant issue. I have also just run into serious problems integrating a third-party .so that uses Boost.Thread. Since our client code also uses Boost.Thread we see SIGBUS and SEGV from the tls_destructor.

comment:4 by Neil Groves, 11 years ago

Resolution: fixed
Status: newclosed

I have taken the liberty of putting in a fix. This defect is extremely serious, very difficult to debug and occurs frequently with the use of third-party shared libraries.

I have modified pthread/thread.cpp to make thread_proxy and tls_destructor internal linkage. I have tested that this resolves the issue.

comment:5 by luke.moore@…, 11 years ago

Could you please also apply the patch to libs/thread/src/pthread/once.cpp (see https://svn.boost.org/trac/boost/attachment/ticket/5170/thread.patch)? I believe it also suffers from the same problem.

Note: See TracTickets for help on using tickets.