Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH 1/2] fwup: bump version to v0.8.2
@ 2016-08-10 22:35 Frank Hunleth
  2016-08-10 22:35 ` [Buildroot] [PATCH 2/2] fwup: link in pthreads for static builds Frank Hunleth
  2016-08-20 13:04 ` [Buildroot] [PATCH 1/2] fwup: bump version to v0.8.2 Thomas Petazzoni
  0 siblings, 2 replies; 10+ messages in thread
From: Frank Hunleth @ 2016-08-10 22:35 UTC (permalink / raw)
  To: buildroot

This version brings in several bug fixes: one of which partially
addresses Buildroot autobuilder failures for static configurations.

Signed-off-by: Frank Hunleth <fhunleth@troodon-software.com>
---
 package/fwup/fwup.hash | 2 +-
 package/fwup/fwup.mk   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/package/fwup/fwup.hash b/package/fwup/fwup.hash
index 6d9a1ea..e21eff0 100644
--- a/package/fwup/fwup.hash
+++ b/package/fwup/fwup.hash
@@ -1,2 +1,2 @@
 # Locally calculated
-sha256	  56d30f4ddf7929d693c022455978aebef1f6adb5896437a199bde43d5459da68	fwup-v0.8.0.tar.gz
+sha256   64c3a0ac38dc11c0c05b2588a89638cbc37f5552ac0f1c290f7ee5365f2bba49      fwup-v0.8.2.tar.gz
diff --git a/package/fwup/fwup.mk b/package/fwup/fwup.mk
index ae85716..3d3e3be 100644
--- a/package/fwup/fwup.mk
+++ b/package/fwup/fwup.mk
@@ -4,7 +4,7 @@
 #
 #############################################################
 
-FWUP_VERSION = v0.8.0
+FWUP_VERSION = v0.8.2
 FWUP_SITE = $(call github,fhunleth,fwup,$(FWUP_VERSION))
 FWUP_LICENSE = Apache-2.0
 FWUP_LICENSE_FILES = LICENSE
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Buildroot] [PATCH 2/2] fwup: link in pthreads for static builds
  2016-08-10 22:35 [Buildroot] [PATCH 1/2] fwup: bump version to v0.8.2 Frank Hunleth
@ 2016-08-10 22:35 ` Frank Hunleth
  2016-08-11 22:06   ` Thomas Petazzoni
  2016-10-16 14:32   ` Thomas Petazzoni
  2016-08-20 13:04 ` [Buildroot] [PATCH 1/2] fwup: bump version to v0.8.2 Thomas Petazzoni
  1 sibling, 2 replies; 10+ messages in thread
From: Frank Hunleth @ 2016-08-10 22:35 UTC (permalink / raw)
  To: buildroot

Both libconfuse (via libintl) and libarchive have references to pthreads
but do not specify pthreads in their pkg-config descriptions. This
change adds pthreads to the fwup linker options so that the link step
succeeds.

Fixes autobuilder failures:

http://autobuild.buildroot.net/results/ce3793c79d5dc4e296d645c75f22e121f35030d3/
http://autobuild.buildroot.net/results/0c5ffec5438f766dade951a64ebfa1b734a3c0aa/
http://autobuild.buildroot.net/results/802/802b6d77e1b77b7c8fcb8f3b394cdabfd406de7a/

Signed-off-by: Frank Hunleth <fhunleth@troodon-software.com>
---
 package/fwup/fwup.mk | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/package/fwup/fwup.mk b/package/fwup/fwup.mk
index 3d3e3be..c5accc6 100644
--- a/package/fwup/fwup.mk
+++ b/package/fwup/fwup.mk
@@ -13,5 +13,13 @@ HOST_FWUP_DEPENDENCIES = host-libconfuse host-libarchive host-libsodium
 FWUP_AUTORECONF = YES
 FWUP_CONF_ENV = ac_cv_path_HELP2MAN=""
 
+# The following is needed to fix link errors in static configurations
+# due to libconfuse requiring pthreads via libintl and libarchive
+# using pthread_mutex_*. Neither list pthreads in their pkg-config
+# description.
+ifeq ($(BR2_STATIC_LIBS)$(BR2_TOOLCHAIN_HAS_THREADS),yy)
+FWUP_CONF_ENV += LDFLAGS="$(TARGET_LDFLAGS) -pthread"
+endif
+
 $(eval $(autotools-package))
 $(eval $(host-autotools-package))
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Buildroot] [PATCH 2/2] fwup: link in pthreads for static builds
  2016-08-10 22:35 ` [Buildroot] [PATCH 2/2] fwup: link in pthreads for static builds Frank Hunleth
@ 2016-08-11 22:06   ` Thomas Petazzoni
  2016-08-12  2:32     ` Frank Hunleth
  2016-08-12  4:54     ` Khem Raj
  2016-10-16 14:32   ` Thomas Petazzoni
  1 sibling, 2 replies; 10+ messages in thread
From: Thomas Petazzoni @ 2016-08-11 22:06 UTC (permalink / raw)
  To: buildroot

Hello,

On Wed, 10 Aug 2016 18:35:46 -0400, Frank Hunleth wrote:
> Both libconfuse (via libintl) and libarchive have references to pthreads
> but do not specify pthreads in their pkg-config descriptions. This
> change adds pthreads to the fwup linker options so that the link step
> succeeds.

I've finally taken some time to look into this, and I believe your fix
is not the right fix, and the bug is in fact in uClibc (and I have
already posted a mail on the uClibc mailing list about this).

The fact that libintl and libarchive do *not* link with libpthread is
intentional and actually a feature. As explained in my mail to the
uClibc mailing list [1], the idea is that those libraries don't need
threads, they only need to be thread-safe if threads are used by the
application using them.

So the libc part of the C library contains a stub implementation of
those functions, while the libpthread part of the C library contains
the real implementation.

So, libintl and libarchive are only linked against libc.so. If you make
a regular, non-threaded application, linked against libintl and/or
libarchive, then those libraries will use the stub version of the
pthread functions. This is correct and actually good: your application
is not multi-threaded, so why suffer the cost of acquiring/releasing a
mutex?

If on the other hand, your application is multi-threaded, it will link
against libpthread. In this case, the pthread_mutex_*() calls from
libarchive or libintl will use the real implementation from libpthread,
and those libraries will have a thread-safe behavior.

The problem is that this doesn't work with static linking with uClibc.
I believe it's a uClibc bug, but I'm not entirely sure either.

So, if fwup links fine dynamically (i.e without linking against
pthread), there should be no reason to link it with pthread for static
linking.

[1] http://mailman.uclibc-ng.org/pipermail/devel/2016-August/001132.html

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Buildroot] [PATCH 2/2] fwup: link in pthreads for static builds
  2016-08-11 22:06   ` Thomas Petazzoni
@ 2016-08-12  2:32     ` Frank Hunleth
  2016-08-12  4:54     ` Khem Raj
  1 sibling, 0 replies; 10+ messages in thread
From: Frank Hunleth @ 2016-08-12  2:32 UTC (permalink / raw)
  To: buildroot

Hi Thomas,

On Thu, Aug 11, 2016 at 6:06 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Hello,
>
> On Wed, 10 Aug 2016 18:35:46 -0400, Frank Hunleth wrote:
>> Both libconfuse (via libintl) and libarchive have references to pthreads
>> but do not specify pthreads in their pkg-config descriptions. This
>> change adds pthreads to the fwup linker options so that the link step
>> succeeds.
>
> I've finally taken some time to look into this, and I believe your fix
> is not the right fix, and the bug is in fact in uClibc (and I have
> already posted a mail on the uClibc mailing list about this).
>
> The fact that libintl and libarchive do *not* link with libpthread is
> intentional and actually a feature. As explained in my mail to the
> uClibc mailing list [1], the idea is that those libraries don't need
> threads, they only need to be thread-safe if threads are used by the
> application using them.
>
> So the libc part of the C library contains a stub implementation of
> those functions, while the libpthread part of the C library contains
> the real implementation.
>
> So, libintl and libarchive are only linked against libc.so. If you make
> a regular, non-threaded application, linked against libintl and/or
> libarchive, then those libraries will use the stub version of the
> pthread functions. This is correct and actually good: your application
> is not multi-threaded, so why suffer the cost of acquiring/releasing a
> mutex?
>
> If on the other hand, your application is multi-threaded, it will link
> against libpthread. In this case, the pthread_mutex_*() calls from
> libarchive or libintl will use the real implementation from libpthread,
> and those libraries will have a thread-safe behavior.
>
> The problem is that this doesn't work with static linking with uClibc.
> I believe it's a uClibc bug, but I'm not entirely sure either.
>
> So, if fwup links fine dynamically (i.e without linking against
> pthread), there should be no reason to link it with pthread for static
> linking.

Thank you so much for investigating this further and posting to the
uclibc mailing list. I much prefer the behaviour you describe since
fwup does not use pthreads. I'll monitor the responses to your post.

Thanks!
Frank

>
> [1] http://mailman.uclibc-ng.org/pipermail/devel/2016-August/001132.html
>
> Thomas
> --
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Buildroot] [PATCH 2/2] fwup: link in pthreads for static builds
  2016-08-11 22:06   ` Thomas Petazzoni
  2016-08-12  2:32     ` Frank Hunleth
@ 2016-08-12  4:54     ` Khem Raj
  2016-08-12 13:51       ` Thomas Petazzoni
  1 sibling, 1 reply; 10+ messages in thread
From: Khem Raj @ 2016-08-12  4:54 UTC (permalink / raw)
  To: buildroot


> On Aug 11, 2016, at 3:06 PM, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote:
> 
> Hello,
> 
> On Wed, 10 Aug 2016 18:35:46 -0400, Frank Hunleth wrote:
>> Both libconfuse (via libintl) and libarchive have references to pthreads
>> but do not specify pthreads in their pkg-config descriptions. This
>> change adds pthreads to the fwup linker options so that the link step
>> succeeds.
> 
> I've finally taken some time to look into this, and I believe your fix
> is not the right fix, and the bug is in fact in uClibc (and I have
> already posted a mail on the uClibc mailing list about this).
> 
> The fact that libintl and libarchive do *not* link with libpthread is
> intentional and actually a feature. As explained in my mail to the
> uClibc mailing list [1], the idea is that those libraries don't need
> threads, they only need to be thread-safe if threads are used by the
> application using them.
> 
> So the libc part of the C library contains a stub implementation of
> those functions, while the libpthread part of the C library contains
> the real implementation.
> 
> So, libintl and libarchive are only linked against libc.so. If you make
> a regular, non-threaded application, linked against libintl and/or
> libarchive, then those libraries will use the stub version of the
> pthread functions. This is correct and actually good: your application
> is not multi-threaded, so why suffer the cost of acquiring/releasing a
> mutex?
> 
> If on the other hand, your application is multi-threaded, it will link
> against libpthread. In this case, the pthread_mutex_*() calls from
> libarchive or libintl will use the real implementation from libpthread,
> and those libraries will have a thread-safe behavior.
> 
> The problem is that this doesn't work with static linking with uClibc.
> I believe it's a uClibc bug, but I'm not entirely sure either.

As per elf specs.

"A weak definition does not change the rules by which object files are
selected from libraries. However, if a link set contains both a weak
definition and a non-weak definition, the non-weak definition will always
be used.?

The "link set" is the set of objects that have been loaded by the linker.
It does not include objects from libraries that are not required.

So if libc.a appears first and provides the symbol it will be used
even if its weak. If you exchange the order where libpthread appears
first it will use the libpthread provided symbols. another option
could be to use

-Wl,--whole-archive -lpthread -Wl,--no-whole-archive

This would link the right strong symbols into binary irrespective of
link order

but this will also include unused symbols into binary, probably not
optimal, and may using -Wl,--gc-sections could help in letting linker
remove the unused functions, maybe worth trying.


> 
> So, if fwup links fine dynamically (i.e without linking against
> pthread), there should be no reason to link it with pthread for static
> linking.
> 
> [1] http://mailman.uclibc-ng.org/pipermail/devel/2016-August/001132.html
> 
> Thomas
> --
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 204 bytes
Desc: Message signed with OpenPGP using GPGMail
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20160811/38344f9d/attachment.asc>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Buildroot] [PATCH 2/2] fwup: link in pthreads for static builds
  2016-08-12  4:54     ` Khem Raj
@ 2016-08-12 13:51       ` Thomas Petazzoni
  2016-08-12 15:05         ` Khem Raj
  0 siblings, 1 reply; 10+ messages in thread
From: Thomas Petazzoni @ 2016-08-12 13:51 UTC (permalink / raw)
  To: buildroot

Hello,

On Thu, 11 Aug 2016 21:54:24 -0700, Khem Raj wrote:

> > The problem is that this doesn't work with static linking with uClibc.
> > I believe it's a uClibc bug, but I'm not entirely sure either.  
> 
> As per elf specs.
> 
> "A weak definition does not change the rules by which object files are
> selected from libraries. However, if a link set contains both a weak
> definition and a non-weak definition, the non-weak definition will always
> be used.?
> 
> The "link set" is the set of objects that have been loaded by the linker.
> It does not include objects from libraries that are not required.
> 
> So if libc.a appears first and provides the symbol it will be used
> even if its weak. If you exchange the order where libpthread appears
> first it will use the libpthread provided symbols. another option
> could be to use
> 
> -Wl,--whole-archive -lpthread -Wl,--no-whole-archive
> 
> This would link the right strong symbols into binary irrespective of
> link order
> 
> but this will also include unused symbols into binary, probably not
> optimal, and may using -Wl,--gc-sections could help in letting linker
> remove the unused functions, maybe worth trying.

Thanks for those details, but I'm not sure to understand what is the
conclusion. Do you mean that it's impossible to implement this dual
symbol definition mechanism with static libraries? Indeed, while shared
libraries have the weak/non-weak mechanism that allows to be sure the
pthread implementation of the symbols will be used if available, with
static libraries, only the link order will decide if the libpthread of
libc implementation is used.

So, if this cannot be fixed in uClibc, how should this problem be
addressed?

Thanks,

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Buildroot] [PATCH 2/2] fwup: link in pthreads for static builds
  2016-08-12 13:51       ` Thomas Petazzoni
@ 2016-08-12 15:05         ` Khem Raj
  0 siblings, 0 replies; 10+ messages in thread
From: Khem Raj @ 2016-08-12 15:05 UTC (permalink / raw)
  To: buildroot

On Fri, Aug 12, 2016 at 6:51 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Hello,
>
> On Thu, 11 Aug 2016 21:54:24 -0700, Khem Raj wrote:
>
>> > The problem is that this doesn't work with static linking with uClibc.
>> > I believe it's a uClibc bug, but I'm not entirely sure either.
>>
>> As per elf specs.
>>
>> "A weak definition does not change the rules by which object files are
>> selected from libraries. However, if a link set contains both a weak
>> definition and a non-weak definition, the non-weak definition will always
>> be used.?
>>
>> The "link set" is the set of objects that have been loaded by the linker.
>> It does not include objects from libraries that are not required.
>>
>> So if libc.a appears first and provides the symbol it will be used
>> even if its weak. If you exchange the order where libpthread appears
>> first it will use the libpthread provided symbols. another option
>> could be to use
>>
>> -Wl,--whole-archive -lpthread -Wl,--no-whole-archive
>>
>> This would link the right strong symbols into binary irrespective of
>> link order
>>
>> but this will also include unused symbols into binary, probably not
>> optimal, and may using -Wl,--gc-sections could help in letting linker
>> remove the unused functions, maybe worth trying.
>
> Thanks for those details, but I'm not sure to understand what is the
> conclusion. Do you mean that it's impossible to implement this dual
> symbol definition mechanism with static libraries? Indeed, while shared
> libraries have the weak/non-weak mechanism that allows to be sure the
> pthread implementation of the symbols will be used if available, with
> static libraries, only the link order will decide if the libpthread of
> libc implementation is used.
>
> So, if this cannot be fixed in uClibc, how should this problem be
> addressed?
>

may be using with above linking options in application

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Buildroot] [PATCH 1/2] fwup: bump version to v0.8.2
  2016-08-10 22:35 [Buildroot] [PATCH 1/2] fwup: bump version to v0.8.2 Frank Hunleth
  2016-08-10 22:35 ` [Buildroot] [PATCH 2/2] fwup: link in pthreads for static builds Frank Hunleth
@ 2016-08-20 13:04 ` Thomas Petazzoni
  1 sibling, 0 replies; 10+ messages in thread
From: Thomas Petazzoni @ 2016-08-20 13:04 UTC (permalink / raw)
  To: buildroot

Hello,

On Wed, 10 Aug 2016 18:35:45 -0400, Frank Hunleth wrote:
> This version brings in several bug fixes: one of which partially
> addresses Buildroot autobuilder failures for static configurations.
> 
> Signed-off-by: Frank Hunleth <fhunleth@troodon-software.com>
> ---
>  package/fwup/fwup.hash | 2 +-
>  package/fwup/fwup.mk   | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)

Applied to master, thanks.

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Buildroot] [PATCH 2/2] fwup: link in pthreads for static builds
  2016-08-10 22:35 ` [Buildroot] [PATCH 2/2] fwup: link in pthreads for static builds Frank Hunleth
  2016-08-11 22:06   ` Thomas Petazzoni
@ 2016-10-16 14:32   ` Thomas Petazzoni
  2016-10-16 15:02     ` Frank Hunleth
  1 sibling, 1 reply; 10+ messages in thread
From: Thomas Petazzoni @ 2016-10-16 14:32 UTC (permalink / raw)
  To: buildroot

Hello,

On Wed, 10 Aug 2016 18:35:46 -0400, Frank Hunleth wrote:
> Both libconfuse (via libintl) and libarchive have references to pthreads
> but do not specify pthreads in their pkg-config descriptions. This
> change adds pthreads to the fwup linker options so that the link step
> succeeds.
> 
> Fixes autobuilder failures:
> 
> http://autobuild.buildroot.net/results/ce3793c79d5dc4e296d645c75f22e121f35030d3/
> http://autobuild.buildroot.net/results/0c5ffec5438f766dade951a64ebfa1b734a3c0aa/
> http://autobuild.buildroot.net/results/802/802b6d77e1b77b7c8fcb8f3b394cdabfd406de7a/
> 
> Signed-off-by: Frank Hunleth <fhunleth@troodon-software.com>

Thanks to the recent uClibc-ng bump, which merges libpthread into libc,
the problem that this patch was trying to fix no longer exists.

I double checked: with an older uClibc-ng toolchain, I could reproduce
the static linking issue. With the newer uClibc-ng toolchain, the built
went fine.

Therefore, I've marked your patch as Rejected in patchwork.

Thanks!

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Buildroot] [PATCH 2/2] fwup: link in pthreads for static builds
  2016-10-16 14:32   ` Thomas Petazzoni
@ 2016-10-16 15:02     ` Frank Hunleth
  0 siblings, 0 replies; 10+ messages in thread
From: Frank Hunleth @ 2016-10-16 15:02 UTC (permalink / raw)
  To: buildroot

On Sun, Oct 16, 2016 at 10:32 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Hello,
>
> On Wed, 10 Aug 2016 18:35:46 -0400, Frank Hunleth wrote:
>> Both libconfuse (via libintl) and libarchive have references to pthreads
>> but do not specify pthreads in their pkg-config descriptions. This
>> change adds pthreads to the fwup linker options so that the link step
>> succeeds.
>>
>> Fixes autobuilder failures:
>>
>> http://autobuild.buildroot.net/results/ce3793c79d5dc4e296d645c75f22e121f35030d3/
>> http://autobuild.buildroot.net/results/0c5ffec5438f766dade951a64ebfa1b734a3c0aa/
>> http://autobuild.buildroot.net/results/802/802b6d77e1b77b7c8fcb8f3b394cdabfd406de7a/
>>
>> Signed-off-by: Frank Hunleth <fhunleth@troodon-software.com>
>
> Thanks to the recent uClibc-ng bump, which merges libpthread into libc,
> the problem that this patch was trying to fix no longer exists.
>
> I double checked: with an older uClibc-ng toolchain, I could reproduce
> the static linking issue. With the newer uClibc-ng toolchain, the built
> went fine.
>
> Therefore, I've marked your patch as Rejected in patchwork.

Great! This is a much better solution.

Thanks,
Frank

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2016-10-16 15:02 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-08-10 22:35 [Buildroot] [PATCH 1/2] fwup: bump version to v0.8.2 Frank Hunleth
2016-08-10 22:35 ` [Buildroot] [PATCH 2/2] fwup: link in pthreads for static builds Frank Hunleth
2016-08-11 22:06   ` Thomas Petazzoni
2016-08-12  2:32     ` Frank Hunleth
2016-08-12  4:54     ` Khem Raj
2016-08-12 13:51       ` Thomas Petazzoni
2016-08-12 15:05         ` Khem Raj
2016-10-16 14:32   ` Thomas Petazzoni
2016-10-16 15:02     ` Frank Hunleth
2016-08-20 13:04 ` [Buildroot] [PATCH 1/2] fwup: bump version to v0.8.2 Thomas Petazzoni

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox