[Buildroot] [PATCH] libxml2: Add patch to remove weak symbol trickery

[Buildroot] [PATCH] libxml2: Add patch to remove weak symbol trickery

[Maarten ter Huurne]

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

diff --git a/package/libxml2/libxml2-pthread-remove-weak-symbol-trickery.patch b/package/libxml2/libxml2-pthread-remove-weak-symbol-trickery.patch
new file mode 100644
index 0000000..8e9a139
--- /dev/null
+++ b/package/libxml2/libxml2-pthread-remove-weak-symbol-trickery.patch
@@ -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

[Buildroot] [PATCH] libxml2: Add patch to remove weak symbol trickery

[Thomas Petazzoni]

Dear Maarten ter Huurne,

On Thu, 11 Sep 2014 06:38:45 +0200, Maarten ter Huurne wrote:
> 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.

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.

> ---
>  ...bxml2-pthread-remove-weak-symbol-trickery.patch | 82 ++++++++++++++++++++++

Patch should follow the patch naming convention:

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

>  1 file changed, 82 insertions(+)
>  create mode 100644 package/libxml2/libxml2-pthread-remove-weak-symbol-trickery.patch
> 
> diff --git a/package/libxml2/libxml2-pthread-remove-weak-symbol-trickery.patch b/package/libxml2/libxml2-pthread-remove-weak-symbol-trickery.patch
> new file mode 100644
> index 0000000..8e9a139
> --- /dev/null
> +++ b/package/libxml2/libxml2-pthread-remove-weak-symbol-trickery.patch
> @@ -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
> +

Missing SoB line.

Also, our libxml2 package does not depends on threads right now. What
happens once we apply your patch?

I'll mark this patch as Changes Requested in patchwork, so please
resend if needed.

Thanks!

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

[Buildroot] [PATCH] libxml2: Add patch to remove weak symbol trickery

[Maarten ter Huurne]

On Sunday 05 October 2014 19:35:57 Thomas Petazzoni wrote:
> Dear Maarten ter Huurne,
> 
> On Thu, 11 Sep 2014 06:38:45 +0200, Maarten ter Huurne wrote:
> > 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-pt
> >   hread.patch> 
> > I added the configure modification to ensure that libxml-2.0.pc will
> > list libpthread as a private dependency.
> 
> Missing SoB line.

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?

> 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.

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.

> > ---
> > 
> >  ...bxml2-pthread-remove-weak-symbol-trickery.patch | 82
> >  ++++++++++++++++++++++
> Patch should follow the patch naming convention:
> 
> 	<pkg>-<seqnumber>-<description>.patch

OK.

> >  1 file changed, 82 insertions(+)
> >  create mode 100644
> >  package/libxml2/libxml2-pthread-remove-weak-symbol-trickery.patch> 
> > diff --git
> > a/package/libxml2/libxml2-pthread-remove-weak-symbol-trickery.patch
> > b/package/libxml2/libxml2-pthread-remove-weak-symbol-trickery.patch new
> > file mode 100644
> > index 0000000..8e9a139
> > --- /dev/null
> > +++ b/package/libxml2/libxml2-pthread-remove-weak-symbol-trickery.patch
> > @@ -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
> > +

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.

> Missing SoB line.
> 
> Also, our libxml2 package does not depends on threads right now. What
> happens once we apply your patch?

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

[Buildroot] [PATCH] libxml2: Add patch to remove weak symbol trickery

[Yann E. MORIN]

Maarten, All,

On 2014-10-06 01:29 +0200, Maarten ter Huurne spake thusly:
> On Sunday 05 October 2014 19:35:57 Thomas Petazzoni wrote:
> > On Thu, 11 Sep 2014 06:38:45 +0200, Maarten ter Huurne wrote:
> > > 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-pt
> > >   hread.patch> 
> > > I added the configure modification to ensure that libxml-2.0.pc will
> > > list libpthread as a private dependency.
> > 
> > Missing SoB line.
> 
> 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?

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.  |
'------------------------------^-------^------------------^--------------------'