Patchwork [CONSOLIDATED,PULL,15/32] conf, recipes: Redefine LINKER_HASH_STYLE

login
register
mail settings
Submitter Saul Wold
Date Aug. 18, 2011, 8:55 p.m.
Message ID <5e892786b427b2049ddf0e660d57877e24f89385.1313700595.git.sgw@linux.intel.com>
Download mbox | patch
Permalink /patch/10227/
State New, archived
Headers show

Comments

Saul Wold - Aug. 18, 2011, 8:55 p.m.
From: Khem Raj <raj.khem@gmail.com>

LINKER_HASH_STYLE is not set to either sysv or gnu
depending upon architecture e.g. mips does not support
gnu hash style among the supported architectures so
we make sure its set to 'sysv' form mips

Linker flags are munged to adhere to renamed variable

Third option is to set it to 'both' we do
not do that by default but user can still set it

Signed-off-by: Khem Raj <raj.khem@gmail.com>
---
 meta/conf/bitbake.conf                      |    4 ++--
 meta/conf/distro/include/tcmode-default.inc |    6 +++++-
 meta/recipes-core/uclibc/uclibc.inc         |    2 +-
 3 files changed, 8 insertions(+), 4 deletions(-)
Phil Blundell - Aug. 19, 2011, 9:24 a.m.
On Thu, 2011-08-18 at 13:55 -0700, Saul Wold wrote:
> From: Khem Raj <raj.khem@gmail.com>
> 
> LINKER_HASH_STYLE is not set to either sysv or gnu
> depending upon architecture e.g. mips does not support
> gnu hash style among the supported architectures so
> we make sure its set to 'sysv' form mips
> 
> Linker flags are munged to adhere to renamed variable
> 
> Third option is to set it to 'both' we do
> not do that by default but user can still set it

It wasn't totally obvious to me why this patch is a good thing.  Can you
explain why this change is desirable? 

Also, I found the first paragraph of the description above quite hard to
understand.  Do you think you could try to reword it to be a bit
clearer?

p.
Richard Purdie - Aug. 19, 2011, 2:26 p.m.
On Fri, 2011-08-19 at 10:24 +0100, Phil Blundell wrote:
> On Thu, 2011-08-18 at 13:55 -0700, Saul Wold wrote:
> > From: Khem Raj <raj.khem@gmail.com>
> > 
> > LINKER_HASH_STYLE is not set to either sysv or gnu
> > depending upon architecture e.g. mips does not support
> > gnu hash style among the supported architectures so
> > we make sure its set to 'sysv' form mips
> > 
> > Linker flags are munged to adhere to renamed variable
> > 
> > Third option is to set it to 'both' we do
> > not do that by default but user can still set it
> 
> It wasn't totally obvious to me why this patch is a good thing.  Can you
> explain why this change is desirable? 
> 
> Also, I found the first paragraph of the description above quite hard to
> understand.  Do you think you could try to reword it to be a bit
> clearer?

Agreed, I'm also having trouble understanding this change. I guess its
related to the options passed into gcc but I suspect we can just improve
the gcc option handling instead.

What I don't like about this patch is forcing a string into LDFLAGS and
I much prefer the current approach where its either added or its not,
its not just adding a parameter through the variable.

Also, if mips doesn't support this, how is anything at all working at
the moment?

Cheers,

Richard
Khem Raj - Aug. 19, 2011, 3:27 p.m.
On 8/19/2011 7:26 AM, Richard Purdie wrote:
> On Fri, 2011-08-19 at 10:24 +0100, Phil Blundell wrote:
>> On Thu, 2011-08-18 at 13:55 -0700, Saul Wold wrote:
>>> From: Khem Raj<raj.khem@gmail.com>
>>>
>>> LINKER_HASH_STYLE is not set to either sysv or gnu
>>> depending upon architecture e.g. mips does not support
>>> gnu hash style among the supported architectures so
>>> we make sure its set to 'sysv' form mips
>>>
>>> Linker flags are munged to adhere to renamed variable
>>>
>>> Third option is to set it to 'both' we do
>>> not do that by default but user can still set it
>>
>> It wasn't totally obvious to me why this patch is a good thing.  Can you
>> explain why this change is desirable?

when we build libraries within gcc e.g. libgcc or libstdc++ etc. then
it does not respect injection of LDFLAGS as we do so if we set hash 
style to gnu then it does not get passed to build environment of those
runtime libraries. Now one may argue that its a bug in gcc  but I would 
rather like to use provisions gcc provides.

This patch makes ways for us to specify linker style options to gcc 
configure.
but the bigger problem is that I forgot to attach the gcc patch that 
adds this options to gcc configury:) that however does not impact this
patch so much but the next one where I enable the gcc options.



>>
>> Also, I found the first paragraph of the description above quite hard to
>> understand.  Do you think you could try to reword it to be a bit
>> clearer?

Yes I had a typo in there

  LINKER_HASH_STYLE is not set to either sysv or gnu

should have been

  LINKER_HASH_STYLE is now set to either sysv or gnu
>
> Agreed, I'm also having trouble understanding this change. I guess its
> related to the options passed into gcc but I suspect we can just improve
> the gcc option handling instead.
>
> What I don't like about this patch is forcing a string into LDFLAGS and
> I much prefer the current approach where its either added or its not,
> its not just adding a parameter through the variable.
>

Yes default is sysv so if you do not specify anything then linker 
assumes sysv but since barring mips we pass this option to all 
architecture there should be not much impact on other architectures than 
mips.

> Also, if mips doesn't support this, how is anything at all working at
> the moment?

see last line in distro/include/tcmode-default.inc

TARGET_LINK_HASH_STYLE ?= 
"${@['-Wl,--hash-style=gnu',''][bb.data.getVar('TARGET_ARCH', d, True) 
in ['mips', 'mipsel', 'mips64', 'mips64el']]}"


>
> Cheers,
>
> Richard
>
>
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
Richard Purdie - Aug. 19, 2011, 4:13 p.m.
On Fri, 2011-08-19 at 08:27 -0700, Khem Raj wrote:
> > Also, if mips doesn't support this, how is anything at all working at
> > the moment?
> 
> see last line in distro/include/tcmode-default.inc
> 
> TARGET_LINK_HASH_STYLE ?= 
> "${@['-Wl,--hash-style=gnu',''][bb.data.getVar('TARGET_ARCH', d, True) 
> in ['mips', 'mipsel', 'mips64', 'mips64el']]}"

So given that, why can't we just add TARGET_LINK_HASH_STYLE to gcc's
EXTRA_OECONF?

Cheers,

Richard
Khem Raj - Aug. 19, 2011, 4:35 p.m.
On Fri, Aug 19, 2011 at 9:13 AM, Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
> On Fri, 2011-08-19 at 08:27 -0700, Khem Raj wrote:
>> > Also, if mips doesn't support this, how is anything at all working at
>> > the moment?
>>
>> see last line in distro/include/tcmode-default.inc
>>
>> TARGET_LINK_HASH_STYLE ?=
>> "${@['-Wl,--hash-style=gnu',''][bb.data.getVar('TARGET_ARCH', d, True)
>> in ['mips', 'mipsel', 'mips64', 'mips64el']]}"
>
> So given that, why can't we just add TARGET_LINK_HASH_STYLE to gcc's
> EXTRA_OECONF?

if you mean the configure option --with-linker-hash-style then
yes we can and certainly we can get rid of TARGET_LINK_HASH_STYLE
altogether then however
the patch I backported only for gcc 4.6 certainly can be done for 4.5
and I left it in there mainly for
prebuilt external toolchains. since we don't control how those
prebuilt toolchains are configured

>
> Cheers,
>
> Richard
>
>
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
>

Patch

diff --git a/meta/conf/bitbake.conf b/meta/conf/bitbake.conf
index b309516..58a604a 100644
--- a/meta/conf/bitbake.conf
+++ b/meta/conf/bitbake.conf
@@ -475,9 +475,9 @@  BUILDSDK_LDFLAGS = "-L${STAGING_LIBDIR} \
                     -Wl,-rpath-link,${STAGING_DIR_HOST}${base_libdir} \
                     -Wl,-rpath,${base_libdir} -Wl,-O1"
 
-TARGET_LINK_HASH_STYLE ??= ""
+LINKER_HASH_STYLE ??= "gnu"
 export LDFLAGS = "${TARGET_LDFLAGS}"
-export TARGET_LDFLAGS = "-Wl,-O1 ${TARGET_LINK_HASH_STYLE}"
+export TARGET_LDFLAGS = "-Wl,-O1 -Wl,--hash-style=${LINKER_HASH_STYLE}"
 #export TARGET_LDFLAGS = "-L${STAGING_DIR_TARGET}${libdir} \
 #                         -Wl,-rpath-link,${STAGING_DIR_TARGET}${libdir} \
 #                         -Wl,-O1"
diff --git a/meta/conf/distro/include/tcmode-default.inc b/meta/conf/distro/include/tcmode-default.inc
index 0d0af38..60f99ac 100644
--- a/meta/conf/distro/include/tcmode-default.inc
+++ b/meta/conf/distro/include/tcmode-default.inc
@@ -63,5 +63,9 @@  PREFERRED_VERSION_gzip-native ?= "1.4"
 
 # Setup suitable toolchain flags
 require conf/distro/include/as-needed.inc
-TARGET_LINK_HASH_STYLE ?= "${@['-Wl,--hash-style=gnu',''][bb.data.getVar('TARGET_ARCH', d, True) in ['mips', 'mipsel', 'mips64', 'mips64el']]}"
 
+# mips does not support GNU hash style therefore we hard assign them
+LINKER_HASH_STYLE_mips = "sysv"
+LINKER_HASH_STYLE_mipsel = "sysv"
+LINKER_HASH_STYLE_mips64 = "sysv"
+LINKER_HASH_STYLE_mips64el = "sysv"
diff --git a/meta/recipes-core/uclibc/uclibc.inc b/meta/recipes-core/uclibc/uclibc.inc
index 222c34f..ab10f1b 100644
--- a/meta/recipes-core/uclibc/uclibc.inc
+++ b/meta/recipes-core/uclibc/uclibc.inc
@@ -201,5 +201,5 @@  do_configure() {
 
 do_install() {
         oe_runmake PREFIX=${D} install
-        oe_runmake PREFIX=${D} "SSP_ALL_CFLAGS=${TARGET_LINK_HASH_STYLE}" install_utils
+        oe_runmake PREFIX=${D} "SSP_ALL_CFLAGS=-Wl,--hash-style=${LINKER_HASH_STYLE}" install_utils
 }