* [PATCH 0/2] RPATH host pollution fixes
@ 2012-08-17 15:53 Andy Ross
2012-08-17 15:53 ` [PATCH 1/2] insane.bbclass: Fix RPATH warning in the face of funny path strings Andy Ross
0 siblings, 1 reply; 9+ messages in thread
From: Andy Ross @ 2012-08-17 15:53 UTC (permalink / raw
To: openembedded-core
Results from chasing the RPATH pollution I was seeing:
I don't have a fix for the underlying linker issue, but that is
captured here:
https://bugzilla.yoctoproject.org/show_bug.cgi?id=2965
Patch 1 is the warning fix from earlier, which is still a fix to the
existing logic and should be applied. I've removed the
"host-polluted" text though, as AFAICT the actual RPATH is not a
problem, only the use of -rpath with the linker that produced it.
Patch 2 is a fix to libtool on top of the existing fix-rpath.patch to
normalize the strings. With this applied, all the link failures we
have reproduced (including the one reported vs. owl_video last week)
work correctly. No libraries in the resulting sysroot have a RPATH
set to a default link directory.
A few runnable binaries still trigger the warning though (but
otherwise build successfully), I'm looking at that now -- I suspect
these simply aren't using libtool, or else we've missed a spot where
normalization and/or default directory pruning needs to happen.
Andy
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] insane.bbclass: Fix RPATH warning in the face of funny path strings
2012-08-17 15:53 [PATCH 0/2] RPATH host pollution fixes Andy Ross
@ 2012-08-17 15:53 ` Andy Ross
2012-08-17 15:53 ` [PATCH 2/2] libtool: normalize link paths before considering for RPATH Andy Ross
2012-08-17 16:20 ` [PATCH 1/2] insane.bbclass: Fix RPATH warning in the face of funny path strings Richard Purdie
0 siblings, 2 replies; 9+ messages in thread
From: Andy Ross @ 2012-08-17 15:53 UTC (permalink / raw
To: openembedded-core
In toolchain edge cases it's possible for the RPATH of a library to be
set to something like "/usr/lib/../lib". This should be detected as
"/usr/lib" and generate a warning.
Signed-off-by: Andy Ross <andy.ross@windriver.com>
---
meta/classes/insane.bbclass | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/meta/classes/insane.bbclass b/meta/classes/insane.bbclass
index 556a176..b84a89f 100644
--- a/meta/classes/insane.bbclass
+++ b/meta/classes/insane.bbclass
@@ -161,6 +161,10 @@ def package_qa_check_rpath(file,name, d, elf, messages):
if dir in line:
messages.append("package %s contains bad RPATH %s in file %s" % (name, line, file))
+def rpath_eq(a, b):
+ import os.path
+ return os.path.normpath(a) == os.path.normpath(b)
+
QAPATHTEST[useless-rpaths] = "package_qa_check_useless_rpaths"
def package_qa_check_useless_rpaths(file, name, d, elf, messages):
"""
@@ -181,7 +185,7 @@ def package_qa_check_useless_rpaths(file, name, d, elf, messages):
m = rpath_re.match(line)
if m:
rpath = m.group(1)
- if rpath == libdir or rpath == base_libdir:
+ if rpath_eq(rpath, libdir) or rpath_eq(rpath, base_libdir):
# The dynamic linker searches both these places anyway. There is no point in
# looking there again.
messages.append("%s: %s contains probably-redundant RPATH %s" % (name, package_qa_clean_path(file, d), rpath))
--
1.7.11.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/2] libtool: normalize link paths before considering for RPATH
2012-08-17 15:53 ` [PATCH 1/2] insane.bbclass: Fix RPATH warning in the face of funny path strings Andy Ross
@ 2012-08-17 15:53 ` Andy Ross
2012-08-19 10:06 ` Richard Purdie
2012-08-17 16:20 ` [PATCH 1/2] insane.bbclass: Fix RPATH warning in the face of funny path strings Richard Purdie
1 sibling, 1 reply; 9+ messages in thread
From: Andy Ross @ 2012-08-17 15:53 UTC (permalink / raw
To: openembedded-core
Libtool may be passed link paths of the form "/usr/lib/../lib", which fool
its detection code into thinking it should be included as an RPATH in
the generated binary. Normalize before comparision.
Signed-off-by: Andy Ross <andy.ross@windriver.com>
---
meta/recipes-devtools/libtool/libtool-2.4.2.inc | 1 +
.../libtool/libtool/norm-rpath.patch | 42 ++++++++++++++++++++++
2 files changed, 43 insertions(+)
create mode 100644 meta/recipes-devtools/libtool/libtool/norm-rpath.patch
diff --git a/meta/recipes-devtools/libtool/libtool-2.4.2.inc b/meta/recipes-devtools/libtool/libtool-2.4.2.inc
index 5b9557e..691427e 100644
--- a/meta/recipes-devtools/libtool/libtool-2.4.2.inc
+++ b/meta/recipes-devtools/libtool/libtool-2.4.2.inc
@@ -19,6 +19,7 @@ SRC_URI = "${GNU_MIRROR}/libtool/libtool-${PV}.tar.gz \
file://avoid_absolute_paths_for_general_utils.patch \
file://fix-rpath.patch \
file://respect-fstack-protector.patch \
+ file://norm-rpath.patch \
"
SRC_URI[md5sum] = "d2f3b7d4627e69e13514a40e72a24d50"
diff --git a/meta/recipes-devtools/libtool/libtool/norm-rpath.patch b/meta/recipes-devtools/libtool/libtool/norm-rpath.patch
new file mode 100644
index 0000000..03a7667
--- /dev/null
+++ b/meta/recipes-devtools/libtool/libtool/norm-rpath.patch
@@ -0,0 +1,42 @@
+libtool: normalize link paths before considering for RPATH
+
+Libtool may be passed link paths of the form "/usr/lib/../lib", which
+fool its detection code into thinking it should be included as an
+RPATH in the generated binary. Normalize before comparision.
+
+Signed-off-by: Andy Ross <andy.ross@windriver.com>
+Upstream-Status: Pending
+
+diff -ru a/libltdl/config/ltmain.m4sh b/libltdl/config/ltmain.m4sh
+--- a/libltdl/config/ltmain.m4sh 2012-08-16 13:58:55.058900363 -0700
++++ b/libltdl/config/ltmain.m4sh 2012-08-16 16:34:54.616627821 -0700
+@@ -7288,8 +7288,13 @@
+ else
+ # We only want to hardcode in an rpath if it isn't in the
+ # default dlsearch path.
++ libdir_norm=`echo $libdir \
++ | sed 's/\/\+\.\(\/\+\|$\)/\//g' \
++ | sed 's/[^\/]\+\/\+\.\.\(\/\+\|$\)//g' \
++ | sed 's/\/\+/\//g' \
++ | sed 's/\(.\)\/$/\1/g'`
+ case " $sys_lib_dlsearch_path " in
+- *" $libdir "*) ;;
++ *" $libdir_norm "*) ;;
+ *) eval flag=\"$hardcode_libdir_flag_spec\"
+ func_append dep_rpath " $flag"
+ ;;
+@@ -8027,8 +8032,13 @@
+ else
+ # We only want to hardcode in an rpath if it isn't in the
+ # default dlsearch path.
++ libdir_norm=`echo $libdir \
++ | sed 's/\/\+\.\(\/\+\|$\)/\//g' \
++ | sed 's/[^\/]\+\/\+\.\.\(\/\+\|$\)//g' \
++ | sed 's/\/\+/\//g' \
++ | sed 's/\(.\)\/$/\1/g'`
+ case " $sys_lib_dlsearch_path " in
+- *" $libdir "*) ;;
++ *" $libdir_norm "*) ;;
+ *) eval flag=\"$hardcode_libdir_flag_spec\"
+ rpath+=" $flag"
+ ;;
--
1.7.11.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] insane.bbclass: Fix RPATH warning in the face of funny path strings
2012-08-17 15:53 ` [PATCH 1/2] insane.bbclass: Fix RPATH warning in the face of funny path strings Andy Ross
2012-08-17 15:53 ` [PATCH 2/2] libtool: normalize link paths before considering for RPATH Andy Ross
@ 2012-08-17 16:20 ` Richard Purdie
2012-08-20 21:05 ` [PATCH] " Andy Ross
1 sibling, 1 reply; 9+ messages in thread
From: Richard Purdie @ 2012-08-17 16:20 UTC (permalink / raw
To: Patches and discussions about the oe-core layer; +Cc: openembedded-core
On Fri, 2012-08-17 at 08:53 -0700, Andy Ross wrote:
> In toolchain edge cases it's possible for the RPATH of a library to be
> set to something like "/usr/lib/../lib". This should be detected as
> "/usr/lib" and generate a warning.
>
> Signed-off-by: Andy Ross <andy.ross@windriver.com>
> ---
> meta/classes/insane.bbclass | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/meta/classes/insane.bbclass b/meta/classes/insane.bbclass
> index 556a176..b84a89f 100644
> --- a/meta/classes/insane.bbclass
> +++ b/meta/classes/insane.bbclass
> @@ -161,6 +161,10 @@ def package_qa_check_rpath(file,name, d, elf, messages):
> if dir in line:
> messages.append("package %s contains bad RPATH %s in file %s" % (name, line, file))
>
> +def rpath_eq(a, b):
> + import os.path
You don't need that import, os is always assumed to be present for our
code since it gets used so often. I'd suggest defining this within the
package_qa_check_useless_rpaths() function too since its slightly less
effort for the parser.
Cheers,
Richard
> + return os.path.normpath(a) == os.path.normpath(b)
> +
> QAPATHTEST[useless-rpaths] = "package_qa_check_useless_rpaths"
> def package_qa_check_useless_rpaths(file, name, d, elf, messages):
> """
> @@ -181,7 +185,7 @@ def package_qa_check_useless_rpaths(file, name, d, elf, messages):
> m = rpath_re.match(line)
> if m:
> rpath = m.group(1)
> - if rpath == libdir or rpath == base_libdir:
> + if rpath_eq(rpath, libdir) or rpath_eq(rpath, base_libdir):
> # The dynamic linker searches both these places anyway. There is no point in
> # looking there again.
> messages.append("%s: %s contains probably-redundant RPATH %s" % (name, package_qa_clean_path(file, d), rpath))
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] libtool: normalize link paths before considering for RPATH
2012-08-17 15:53 ` [PATCH 2/2] libtool: normalize link paths before considering for RPATH Andy Ross
@ 2012-08-19 10:06 ` Richard Purdie
2012-08-20 16:59 ` Andy Ross
0 siblings, 1 reply; 9+ messages in thread
From: Richard Purdie @ 2012-08-19 10:06 UTC (permalink / raw
To: Patches and discussions about the oe-core layer; +Cc: openembedded-core
On Fri, 2012-08-17 at 08:53 -0700, Andy Ross wrote:
> Libtool may be passed link paths of the form "/usr/lib/../lib", which fool
> its detection code into thinking it should be included as an RPATH in
> the generated binary. Normalize before comparision.
>
> Signed-off-by: Andy Ross <andy.ross@windriver.com>
> ---
> meta/recipes-devtools/libtool/libtool-2.4.2.inc | 1 +
> .../libtool/libtool/norm-rpath.patch | 42 ++++++++++++++++++++++
> 2 files changed, 43 insertions(+)
> create mode 100644 meta/recipes-devtools/libtool/libtool/norm-rpath.patch
>
> diff --git a/meta/recipes-devtools/libtool/libtool-2.4.2.inc b/meta/recipes-devtools/libtool/libtool-2.4.2.inc
> index 5b9557e..691427e 100644
> --- a/meta/recipes-devtools/libtool/libtool-2.4.2.inc
> +++ b/meta/recipes-devtools/libtool/libtool-2.4.2.inc
> @@ -19,6 +19,7 @@ SRC_URI = "${GNU_MIRROR}/libtool/libtool-${PV}.tar.gz \
> file://avoid_absolute_paths_for_general_utils.patch \
> file://fix-rpath.patch \
> file://respect-fstack-protector.patch \
> + file://norm-rpath.patch \
> "
>
> SRC_URI[md5sum] = "d2f3b7d4627e69e13514a40e72a24d50"
> diff --git a/meta/recipes-devtools/libtool/libtool/norm-rpath.patch b/meta/recipes-devtools/libtool/libtool/norm-rpath.patch
> new file mode 100644
> index 0000000..03a7667
> --- /dev/null
> +++ b/meta/recipes-devtools/libtool/libtool/norm-rpath.patch
> @@ -0,0 +1,42 @@
> +libtool: normalize link paths before considering for RPATH
> +
> +Libtool may be passed link paths of the form "/usr/lib/../lib", which
> +fool its detection code into thinking it should be included as an
> +RPATH in the generated binary. Normalize before comparision.
> +
> +Signed-off-by: Andy Ross <andy.ross@windriver.com>
> +Upstream-Status: Pending
> +
> +diff -ru a/libltdl/config/ltmain.m4sh b/libltdl/config/ltmain.m4sh
> +--- a/libltdl/config/ltmain.m4sh 2012-08-16 13:58:55.058900363 -0700
> ++++ b/libltdl/config/ltmain.m4sh 2012-08-16 16:34:54.616627821 -0700
> +@@ -7288,8 +7288,13 @@
> + else
> + # We only want to hardcode in an rpath if it isn't in the
> + # default dlsearch path.
> ++ libdir_norm=`echo $libdir \
> ++ | sed 's/\/\+\.\(\/\+\|$\)/\//g' \
> ++ | sed 's/[^\/]\+\/\+\.\.\(\/\+\|$\)//g' \
> ++ | sed 's/\/\+/\//g' \
> ++ | sed 's/\(.\)\/$/\1/g'`
> + case " $sys_lib_dlsearch_path " in
> +- *" $libdir "*) ;;
> ++ *" $libdir_norm "*) ;;
> + *) eval flag=\"$hardcode_libdir_flag_spec\"
> + func_append dep_rpath " $flag"
> + ;;
Can't we use func_norm_abspath here?
Cheers,
Richard
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] libtool: normalize link paths before considering for RPATH
2012-08-19 10:06 ` Richard Purdie
@ 2012-08-20 16:59 ` Andy Ross
2012-08-20 21:55 ` Richard Purdie
0 siblings, 1 reply; 9+ messages in thread
From: Andy Ross @ 2012-08-20 16:59 UTC (permalink / raw
To: Richard Purdie; +Cc: openembedded-core
On 08/19/2012 03:06 AM, Richard Purdie wrote:
> On Fri, 2012-08-17 at 08:53 -0700, Andy Ross wrote:
>> ++ libdir_norm=`echo $libdir \
>> ++ | sed 's/\/\+\.\(\/\+\|$\)/\//g' \
>> ++ | sed 's/[^\/]\+\/\+\.\.\(\/\+\|$\)//g' \
>> ++ | sed 's/\/\+/\//g' \
>> ++ | sed 's/\(.\)\/$/\1/g'`
>
> Can't we use func_norm_abspath here?
I have to admit I got a little confused reading that code (not that my
sed mess is significantly better, but at least I trust it because I
wrote it); but it looks to me like it's an abspath implementation on
the host filesystem (not the use of `pwd` in a few places). That will
work for pruning in this case, since the problem case is already an
absolute path to a host directory. But I don't see how it won't in
principle break things by expanding host paths.
Andy
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] insane.bbclass: Fix RPATH warning in the face of funny path strings
2012-08-17 16:20 ` [PATCH 1/2] insane.bbclass: Fix RPATH warning in the face of funny path strings Richard Purdie
@ 2012-08-20 21:05 ` Andy Ross
2012-08-21 15:49 ` Saul Wold
0 siblings, 1 reply; 9+ messages in thread
From: Andy Ross @ 2012-08-20 21:05 UTC (permalink / raw
To: openembedded-core, Richard Purdie
In toolchain edge cases it's possible for the RPATH of a library to be
set to something like "/usr/lib/../lib". This should be detected as
"/usr/lib" and generate a warning.
Signed-off-by: Andy Ross <andy.ross@windriver.com>
---
meta/classes/insane.bbclass | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/meta/classes/insane.bbclass b/meta/classes/insane.bbclass
index 556a176..9d085a4 100644
--- a/meta/classes/insane.bbclass
+++ b/meta/classes/insane.bbclass
@@ -166,6 +166,9 @@ def package_qa_check_useless_rpaths(file, name, d, elf, messages):
"""
Check for RPATHs that are useless but not dangerous
"""
+ def rpath_eq(a, b):
+ return os.path.normpath(a) == os.path.normpath(b)
+
if not elf:
return
@@ -181,7 +184,7 @@ def package_qa_check_useless_rpaths(file, name, d, elf, messages):
m = rpath_re.match(line)
if m:
rpath = m.group(1)
- if rpath == libdir or rpath == base_libdir:
+ if rpath_eq(rpath, libdir) or rpath_eq(rpath, base_libdir):
# The dynamic linker searches both these places anyway. There is no point in
# looking there again.
messages.append("%s: %s contains probably-redundant RPATH %s" % (name, package_qa_clean_path(file, d), rpath))
--
1.7.11.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] libtool: normalize link paths before considering for RPATH
2012-08-20 16:59 ` Andy Ross
@ 2012-08-20 21:55 ` Richard Purdie
0 siblings, 0 replies; 9+ messages in thread
From: Richard Purdie @ 2012-08-20 21:55 UTC (permalink / raw
To: Andy Ross; +Cc: openembedded-core
On Mon, 2012-08-20 at 09:59 -0700, Andy Ross wrote:
> On 08/19/2012 03:06 AM, Richard Purdie wrote:
> > On Fri, 2012-08-17 at 08:53 -0700, Andy Ross wrote:
> >> ++ libdir_norm=`echo $libdir \
> >> ++ | sed 's/\/\+\.\(\/\+\|$\)/\//g' \
> >> ++ | sed 's/[^\/]\+\/\+\.\.\(\/\+\|$\)//g' \
> >> ++ | sed 's/\/\+/\//g' \
> >> ++ | sed 's/\(.\)\/$/\1/g'`
> >
> > Can't we use func_norm_abspath here?
>
> I have to admit I got a little confused reading that code (not that my
> sed mess is significantly better, but at least I trust it because I
> wrote it); but it looks to me like it's an abspath implementation on
> the host filesystem (not the use of `pwd` in a few places). That will
> work for pruning in this case, since the problem case is already an
> absolute path to a host directory. But I don't see how it won't in
> principle break things by expanding host paths.
As I read it, it will only use pwd if the path is relative or empty. I
can't think of a case where you'd encode something like that into an
rpath...
Libtool can't expect the directory to exist on the filesystem when it
makes these calls so we should be safe from that perspective.
Cheers,
Richard
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] insane.bbclass: Fix RPATH warning in the face of funny path strings
2012-08-20 21:05 ` [PATCH] " Andy Ross
@ 2012-08-21 15:49 ` Saul Wold
0 siblings, 0 replies; 9+ messages in thread
From: Saul Wold @ 2012-08-21 15:49 UTC (permalink / raw
To: Andy Ross; +Cc: openembedded-core
On 08/20/2012 02:05 PM, Andy Ross wrote:
> In toolchain edge cases it's possible for the RPATH of a library to be
> set to something like "/usr/lib/../lib". This should be detected as
> "/usr/lib" and generate a warning.
>
> Signed-off-by: Andy Ross <andy.ross@windriver.com>
> ---
> meta/classes/insane.bbclass | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/meta/classes/insane.bbclass b/meta/classes/insane.bbclass
> index 556a176..9d085a4 100644
> --- a/meta/classes/insane.bbclass
> +++ b/meta/classes/insane.bbclass
> @@ -166,6 +166,9 @@ def package_qa_check_useless_rpaths(file, name, d, elf, messages):
> """
> Check for RPATHs that are useless but not dangerous
> """
> + def rpath_eq(a, b):
> + return os.path.normpath(a) == os.path.normpath(b)
> +
> if not elf:
> return
>
> @@ -181,7 +184,7 @@ def package_qa_check_useless_rpaths(file, name, d, elf, messages):
> m = rpath_re.match(line)
> if m:
> rpath = m.group(1)
> - if rpath == libdir or rpath == base_libdir:
> + if rpath_eq(rpath, libdir) or rpath_eq(rpath, base_libdir):
> # The dynamic linker searches both these places anyway. There is no point in
> # looking there again.
> messages.append("%s: %s contains probably-redundant RPATH %s" % (name, package_qa_clean_path(file, d), rpath))
>
Merged this to OE-Core
Thanks
Sau!
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2012-08-21 16:02 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-08-17 15:53 [PATCH 0/2] RPATH host pollution fixes Andy Ross
2012-08-17 15:53 ` [PATCH 1/2] insane.bbclass: Fix RPATH warning in the face of funny path strings Andy Ross
2012-08-17 15:53 ` [PATCH 2/2] libtool: normalize link paths before considering for RPATH Andy Ross
2012-08-19 10:06 ` Richard Purdie
2012-08-20 16:59 ` Andy Ross
2012-08-20 21:55 ` Richard Purdie
2012-08-17 16:20 ` [PATCH 1/2] insane.bbclass: Fix RPATH warning in the face of funny path strings Richard Purdie
2012-08-20 21:05 ` [PATCH] " Andy Ross
2012-08-21 15:49 ` Saul Wold
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.