libxml2: Add patch to remove weak symbol trickery

This avoids runtime problems when linking libpthread statically and it fixes compilation with musl libc.

The patch is based on this one:

https://github.com/sabotage-linux/sabotage/blob/master/KEEP/libxml2-pthread.patch

I added the configure modification to ensure that libxml-2.0.pc will list libpthread as a private dependency.

---
 ...bxml2-pthread-remove-weak-symbol-trickery.patch | 82 ++++++++++++++++++++++
 1 file changed, 82 insertions(+)
 create mode 100644 package/libxml2/libxml2-pthread-remove-weak-symbol-trickery.patch

new file mode 100644
index 0000000..8e9a139
@@ -0,0 +1,82 @@
+libxml2 contains some trickery to avoid linking to libpthread.
+However, this is unsafe in several ways:
+
+Bug 704904 - Incorrect and unsafe use of weak references to pthread functions
+https://bugzilla.gnome.org/show_bug.cgi?id=704904
+
+As a side effect, the following compilation error with musl libc is also
+fixed by this patch:
+
+Bug 704908 - Failure to suppress macros when re-declaring pthread functions
+https://bugzilla.gnome.org/show_bug.cgi?id=704908
+
+--- libxml2-2.9.1.org/threads.c	2013-04-05 17:08:04.000000000 +0200
++++ libxml2-2.9.1/threads.c	2014-09-11 06:18:30.988687882 +0200
+@@ -47,49 +47,7 @@
+ #ifdef HAVE_PTHREAD_H
+ 
+ static int libxml_is_threaded = -1;
+-#ifdef __GNUC__
+-#ifdef linux
+-#if (__GNUC__ == 3 && __GNUC_MINOR__ >= 3) || (__GNUC__ > 3)
+-extern int pthread_once (pthread_once_t *__once_control,
+-                         void (*__init_routine) (void))
+-	   __attribute((weak));
+-extern void *pthread_getspecific (pthread_key_t __key)
+-	   __attribute((weak));
+-extern int pthread_setspecific (pthread_key_t __key,
+-                                __const void *__pointer)
+-	   __attribute((weak));
+-extern int pthread_key_create (pthread_key_t *__key,
+-                               void (*__destr_function) (void *))
+-	   __attribute((weak));
+-extern int pthread_key_delete (pthread_key_t __key)
+-	   __attribute((weak));
+-extern int pthread_mutex_init ()
+-	   __attribute((weak));
+-extern int pthread_mutex_destroy ()
+-	   __attribute((weak));
+-extern int pthread_mutex_lock ()
+-	   __attribute((weak));
+-extern int pthread_mutex_unlock ()
+-	   __attribute((weak));
+-extern int pthread_cond_init ()
+-	   __attribute((weak));
+-extern int pthread_cond_destroy ()
+-	   __attribute((weak));
+-extern int pthread_cond_wait ()
+-	   __attribute((weak));
+-extern int pthread_equal ()
+-	   __attribute((weak));
+-extern pthread_t pthread_self ()
+-	   __attribute((weak));
+-extern int pthread_key_create ()
+-	   __attribute((weak));
+-extern int pthread_key_delete ()
+-	   __attribute((weak));
+-extern int pthread_cond_signal ()
+-	   __attribute((weak));
+-#endif
+-#endif /* linux */
+-#endif /* __GNUC__ */
++
+ #endif /* HAVE_PTHREAD_H */
+ 
+ /*
+--- libxml2-2.9.1.org/configure.in	2013-04-19 09:25:20.000000000 +0200
++++ libxml2-2.9.1/configure.in	2014-09-11 06:14:21.357860768 +0200
+@@ -1001,14 +1001,6 @@
+ 		   then
+ 		       THREAD_LIBS=""
+ 		       BASE_THREAD_LIBS="-lpthread"
+-		   else
+-		   if expr ${GCC_MAJOR} \> 3 > /dev/null
+-		   then
+-		       THREAD_LIBS=""
+-		       BASE_THREAD_LIBS="-lpthread"
+-		   else
+-		       echo old GCC disabling weak symbols for pthread
+-		   fi
+ 		   fi
+ 	       fi
+ 	   fi

1.8.4.5

Dear Maarten ter Huurne,

Missing SoB line.

I'm a bit worried about upstream reaction: they say that they won't support static linking anymore. So this means that this patch will never be upstreamed, while we generally try to not carry patches that really have zero chance to go upstream, unless they are cross-compilation fixes.

Patch should follow the patch naming convention:

<pkg>-<seqnumber>-<description>.patch

Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Is there a policy for including 3rd party patches? In this case the fragment I took from Sabotage Linux only removes code, so I don't think there could be any copyright issues there (no creativity involved), but in general would my SoB line be sufficient or would I need one from the 3rd party author as well?

There are more failure modes than just static linking. For example, if libpthread is loaded after libxml2 initializes, for example as a result of a dlopen(), libxml2 will have already concluded that it runs in a single threaded environment while that is no longer true.

So the code is unsafe, but I assume the scenarios in which it breaks are relatively rare, otherwise it would have been fixed already. On the other hand, threading issues are hard to diagnose, so maybe there are applications out there misbehaving in ways no-one has figured out yet.

An alternative safe solution would be to build two versions of libxml2: one for applications that use threads and one for applications that don't and have the application developer pick the right one to link with. However, until upstream acknowledges that there is a problem, no solution will have any chance of getting accepted.

OK.

Note that fixing the musl compilation error was my motivation to look into this problem. At first I was working on a patch to deal with some of the pthread calls being macros rather than functions in musl, but after a discussion on the musl IRC channel, I decided that it would be better to remove the unsafe feature rather than making it compile.

I could make a patch that only fixes bug 704908 and leaves 704904 unfixed; that might have a better chance at getting accepted upstream. But personally I think Buildroot users would be better off if 704904 is fixed too and fixing that bug automatically fixes 704908 as well.

All the changed code is within a #ifdef HAVE_PTHREAD_H. Assuming that libxml2's configure script does its job, it will leave HAVE_PTHREAD_H undefined when building for a system without pthread support.

Bye,
Maarten

Maarten, All,

On 2014-10-06 01:29 +0200, Maarten ter Huurne spake thusly:

When using a patch from somewhere else:
- take the patch as-is,
- add to the description of that patch the URL where you got it: an upstream commit, a bug-tracker, a mailing...
- add your own SoB line.

That way, proper attribution is done, both for the original author(s) and you.

Regards,
Yann E. MORIN.

.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'