| Message ID | 1446420326-29281-1-git-send-email-ps.report@gmx.net |
|---|---|
| State | Accepted |
| Headers | show |
Dear Peter Seiderer, On Mon, 2 Nov 2015 00:25:26 +0100, Peter Seiderer wrote: > Tested with example program from [1] with qemu_x86_64. > > [1] http://valgrind.10908.n7.nabble.com/Thread-local-storage-TLS-support-td40815.html > > Signed-off-by: Peter Seiderer <ps.report@gmx.net> > --- > package/valgrind/valgrind.mk | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) I've applied your patch (after fixing the typo in the commit title). However, it is not a fully correct solution: BR2_GCC_ENABLE_TLS is only valid for internal toolchains. For external toolchains, it will always be false, so you will never have TLS support enabled for external toolchains. I think it would really be easier if Valgrind had a compile-time way of determining whether TLS support is available or not, because it is the only package for which we need to know if TLS support is available or not. I've nonetheless applied your patch because it doesn't make things really worse than they are. But I would really prefer if you could do some research at adjusting valgrind configure.ac to check TLS availability at compile time. Generally speaking, I think we should drop BR2_GCC_ENABLE_TLS altogether, and simply enable TLS whenever we have NPTL threads being used. Best regards, Thomas
Hello Thomas, On Mon, 2 Nov 2015 15:33:09 +0100, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote: > Dear Peter Seiderer, > > On Mon, 2 Nov 2015 00:25:26 +0100, Peter Seiderer wrote: > > Tested with example program from [1] with qemu_x86_64. > > > > [1] http://valgrind.10908.n7.nabble.com/Thread-local-storage-TLS-support-td40815.html > > > > Signed-off-by: Peter Seiderer <ps.report@gmx.net> > > --- > > package/valgrind/valgrind.mk | 8 +++++++- > > 1 file changed, 7 insertions(+), 1 deletion(-) > > I've applied your patch (after fixing the typo in the commit title). > Many thanks for fixing it... > However, it is not a fully correct solution: BR2_GCC_ENABLE_TLS is only > valid for internal toolchains. For external toolchains, it will always > be false, so you will never have TLS support enabled for external > toolchains. I think it would really be easier if Valgrind had a I can live with it.... ;-) > compile-time way of determining whether TLS support is available or > not, because it is the only package for which we need to know if TLS > support is available or not. > > I've nonetheless applied your patch because it doesn't make things > really worse than they are. But I would really prefer if you could do > some research at adjusting valgrind configure.ac to check TLS > availability at compile time. > O.k, I will take a look... Regards, Peter > Generally speaking, I think we should drop BR2_GCC_ENABLE_TLS > altogether, and simply enable TLS whenever we have NPTL threads being > used. > > Best regards, > > Thomas
diff --git a/package/valgrind/valgrind.mk b/package/valgrind/valgrind.mk index a630125..dc126ca 100644 --- a/package/valgrind/valgrind.mk +++ b/package/valgrind/valgrind.mk @@ -9,12 +9,18 @@ VALGRIND_SITE = http://valgrind.org/downloads VALGRIND_SOURCE = valgrind-$(VALGRIND_VERSION).tar.bz2 VALGRIND_LICENSE = GPLv2 GFDLv1.2 VALGRIND_LICENSE_FILES = COPYING COPYING.DOCS -VALGRIND_CONF_OPTS = --disable-tls --disable-ubsan +VALGRIND_CONF_OPTS = --disable-ubsan VALGRIND_INSTALL_STAGING = YES # patch touching configure.ac VALGRIND_AUTORECONF = YES +ifeq ($(BR2_GCC_ENABLE_TLS),y) +VALGRIND_CONF_OPTS += --enable-tls +else +VALGRIND_CONF_OPTS += --disable-tls +endif + # When Valgrind detects a 32-bit MIPS architecture, it forcibly adds # -march=mips32 to CFLAGS; when it detects a 64-bit MIPS architecture, # it forcibly adds -march=mips64. This causes Valgrind to be built
Tested with example program from [1] with qemu_x86_64. [1] http://valgrind.10908.n7.nabble.com/Thread-local-storage-TLS-support-td40815.html Signed-off-by: Peter Seiderer <ps.report@gmx.net> --- package/valgrind/valgrind.mk | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)