| Submitter | Andy Ross |
|---|---|
| Date | Aug. 15, 2012, 10:46 p.m. |
| Message ID | <1345070782-4518-2-git-send-email-andy.ross@windriver.com> |
| Download | mbox | patch |
| Permalink | /patch/34675/ |
| State | New |
| Headers | show |
Comments
On Wed, Aug 15, 2012 at 3:46 PM, Andy Ross <andy.ross@windriver.com> wrote: > diff --git a/meta/classes/insane.bbclass b/meta/classes/insane.bbclass > index 556a176..ade0616 100644 > --- a/meta/classes/insane.bbclass > +++ b/meta/classes/insane.bbclass > @@ -161,6 +161,17 @@ 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_norm(s): > + import re > + s = re.sub('[^/]+/\.\.(/|$)', '/', s) # snip ".." components > + s = re.sub('/\.(/|$)', '/', s) # snip "." components > + s = re.sub('/+', '/', s) # snip repeated slashes > + s = re.sub('/ > , '', s) # snip trailing slash > + return s > + > +def rpath_eq(a, b): > + return rpath_norm(a) == rpath_norm(b) > + Please just use os.path.normpath() rather than reinventing the wheel here.
On Wed, 2012-08-15 at 15:46 -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. Also clarify the warning text to > indicate potential link-time pollution from the host libraries. If these RPATHs are causing host pollution then that sounds like there is another bug elsewhere. They ought to be resolved relative to the sysroot during link edit. It's also tempting to say that we should, unconditionally, normalise any RPATHs which contain .. or . components before insane.bbclass runs. Having extra, redundant, components in the pathname will just cause extra work for ld.so (and hence run-time slowness) and bring no benefit. Doing that would mean that the existing check would catch things like the case you mentioned automatically. p.
Chris Larson wrote:
> Please just use os.path.normpath() rather than reinventing the wheel here.
Learn something new every day. Fixed.
Andy
On 08/16/2012 01:39 AM, Phil Blundell wrote: > If these RPATHs are causing host pollution then that sounds like there > is another bug elsewhere. They ought to be resolved relative to the > sysroot during link edit. Indeed, this is turning out to be a deeper issue than I wanted it to be. What seems to be is happening is this: an RPATH in the ELF header is interpreted relative to sysroot normally. But when linking, a -rpath argument to the ld is interpreted relative to the *host* filesystem even when there is a --sysroot argument. See the quick test script below. As it happens, both of those are ultimately produced by libtool, and it's only the second one that is fatal. The RPATH itself is probably still a warning condition, but it's not a build breaker. But neither is needed, they are happening in the case I'm looking at only because libtool (I think) is confused by the "/../" syntax in the link path into trying to add an RPATH where one isn't needed. The specific case I'm looking at (with our internal tree, I'm working on reproducing vs. poky right now) is with a pulseaudio build, where an x86-64 build on an x86_64 host hits a RPATH in libgdk-x11-2.0 that pulls in the host libXranr and fails due to version skew. I was just pointed at a discussion from last week on owl_video which looks all but identical. At least for the moment I'm going to try to track down the libtool issue (maybe sanify the path before it sees if if possible) instead of trying to fix a linker bug. Andy Reproducer for the underlying linker issue: #!/bin/sh set -ex SYSROOT=/home/andy/oe/poky/build/tmp/sysroots/qemux86-64 CC=/home/andy/oe/poky/build/tmp/sysroots/x86_64-linux/usr/bin/x86_64-poky-linux/x86_64-poky-linux-gcc # A library: echo 'int foo(){return 0;}' > foo.c $CC -shared -fPIC -o libfoo.so foo.c # Another library that references the first via RPATH=`pwd` echo 'int foo(); int bar(){return foo();}' > bar.c $CC -shared -fPIC -o libbar.so bar.c -L. -lfoo -Wl,-rpath -Wl,`pwd` # A program that uses the second library: echo 'int bar(); int main(){return bar();}' > main.c # Works (incorrectly!) because the "-rpath `pwd" argument here is # *not* interpreted relative to sysroot, so the linker sees # ./libfoo.so as a potential library. $CC --sysroot=$SYSROOT -L$SYSROOT/usr/lib64 -Wl,-rpath -Wl,`pwd` -o main2 main.c libbar.so echo THAT LINK SHOULD HAVE FAILED # Fails (correctly) because nothing on the link line tells libbar.so # how to find libfoo.so, nor does libfoo.so exist in the # sysroot-relative RPATH. $CC --sysroot=$SYSROOT -o main main.c libbar.so
On Thu, 2012-08-16 at 10:43 -0700, Andy Ross wrote: > On 08/16/2012 01:39 AM, Phil Blundell wrote: > > If these RPATHs are causing host pollution then that sounds like there > > is another bug elsewhere. They ought to be resolved relative to the > > sysroot during link edit. > > Indeed, this is turning out to be a deeper issue than I wanted it to > be. What seems to be is happening is this: an RPATH in the ELF header > is interpreted relative to sysroot normally. But when linking, a > -rpath argument to the ld is interpreted relative to the *host* > filesystem even when there is a --sysroot argument. See the quick > test script below. > > As it happens, both of those are ultimately produced by libtool, and > it's only the second one that is fatal. The RPATH itself is probably > still a warning condition, but it's not a build breaker. But neither > is needed, they are happening in the case I'm looking at only because > libtool (I think) is confused by the "/../" syntax in the link path > into trying to add an RPATH where one isn't needed. > > The specific case I'm looking at (with our internal tree, I'm working > on reproducing vs. poky right now) is with a pulseaudio build, where > an x86-64 build on an x86_64 host hits a RPATH in libgdk-x11-2.0 that > pulls in the host libXranr and fails due to version skew. I was just > pointed at a discussion from last week on owl_video which looks all > but identical. > > At least for the moment I'm going to try to track down the libtool > issue (maybe sanify the path before it sees if if possible) instead of > trying to fix a linker bug. I suspect you need to look somewhere around: http://git.yoctoproject.org/cgit.cgi/poky/tree/meta/recipes-devtools/libtool/libtool/fix-rpath.patch and normalise the rpaths being used by libtool. I think it has some kind of path normalisation function somewhere... Cheers, Richard
On 08/17/2012 03:28 AM, Richard Purdie wrote: > I suspect you need to look somewhere around: > > http://git.yoctoproject.org/cgit.cgi/poky/tree/meta/recipes-devtools/libtool/libtool/fix-rpath.patch Indeed. Found that yesterday and am currently testing a fix. Should be inbound within the hour. Andy
Patch
diff --git a/meta/classes/insane.bbclass b/meta/classes/insane.bbclass index 556a176..ade0616 100644 --- a/meta/classes/insane.bbclass +++ b/meta/classes/insane.bbclass @@ -161,6 +161,17 @@ 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_norm(s): + import re + s = re.sub('[^/]+/\.\.(/|$)', '/', s) # snip ".." components + s = re.sub('/\.(/|$)', '/', s) # snip "." components + s = re.sub('/+', '/', s) # snip repeated slashes + s = re.sub('/$', '', s) # snip trailing slash + return s + +def rpath_eq(a, b): + return rpath_norm(a) == rpath_norm(b) + QAPATHTEST[useless-rpaths] = "package_qa_check_useless_rpaths" def package_qa_check_useless_rpaths(file, name, d, elf, messages): """ @@ -181,10 +192,13 @@ 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: - # 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)) + 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. And it may + # be harmful: the RPATH may point to host directories + # (e.g. /usr/lib) which will be searched at link time, + # creating build issues. + messages.append("%s: %s contains probably-redundant, possibly host-polluted RPATH %s" % (name, package_qa_clean_path(file, d), rpath)) QAPATHTEST[dev-so] = "package_qa_check_dev" def package_qa_check_dev(path, name, d, elf, messages):
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. Also clarify the warning text to indicate potential link-time pollution from the host libraries. Signed-off-by: Andy Ross <andy.ross@windriver.com> --- meta/classes/insane.bbclass | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-)