[PATCH] insane: Rationalise phdrs-based QA checks

[PATCH] insane: Rationalise phdrs-based QA checks

[Phil Blundell]

Various different QA checks are based on essentially the same data from
the ELF program headers.  Calling objdump to extract it repeatedly is
inefficient, particularly if the shell is involved.  Instead, let's
cache the output from objdump inside the qa.elf object and allow it to
be reused by multiple tests.

Also, using objdump instead of scanelf to check for bad RPATHs (in the
same way that the useless-rpaths check was doing already) allows the
dependency on pax-utils-native to be dropped.

Signed-off-by: Phil Blundell <philb@gnu.org>
---
 meta/classes/insane.bbclass |   41 ++++++++++++++++++-----------------------
 meta/lib/oe/qa.py           |   17 +++++++++++++++++
 2 files changed, 35 insertions(+), 23 deletions(-)

diff --git a/meta/classes/insane.bbclass b/meta/classes/insane.bbclass
index 1fb8970..3705fe9 100644
--- a/meta/classes/insane.bbclass
+++ b/meta/classes/insane.bbclass
@@ -17,13 +17,8 @@
 #   files under exec_prefix
 
 
-#
-# We need to have the scanelf utility as soon as
-# possible and this is contained within the pax-utils-native.
-# The package.bbclass can help us here.
-#
 inherit package
-PACKAGE_DEPENDS += "pax-utils-native ${QADEPENDS}"
+PACKAGE_DEPENDS += "${QADEPENDS}"
 PACKAGEFUNCS += " do_package_qa "
 
 # unsafe-references-in-binaries requires prelink-rtld from
@@ -147,21 +142,23 @@ def package_qa_check_rpath(file,name, d, elf, messages):
     if not elf:
         return
 
-    scanelf = os.path.join(d.getVar('STAGING_BINDIR_NATIVE',True),'scanelf')
     bad_dirs = [d.getVar('TMPDIR', True) + "/work", d.getVar('STAGING_DIR_TARGET', True)]
     bad_dir_test = d.getVar('TMPDIR', True)
-    if not os.path.exists(scanelf):
-        bb.fatal("Can not check RPATH, scanelf (part of pax-utils-native) not found")
 
     if not bad_dirs[0] in d.getVar('WORKDIR', True):
         bb.fatal("This class assumed that WORKDIR is ${TMPDIR}/work... Not doing any check")
 
-    output = os.popen("%s -B -F%%r#F '%s'" % (scanelf,file))
-    txt    = output.readline().split()
-    for line in txt:
-        for dir in bad_dirs:
-            if dir in line:
-                messages.append("package %s contains bad RPATH %s in file %s" % (name, line, file))
+    phdrs = elf.run_objdump("-p", d)
+
+    import re
+    rpath_re = re.compile("\s+RPATH\s+(.*)")
+    for line in phdrs.split("\n"):
+    	m = rpath_re.match(line)
+	if m:
+	    rpath = m.group(1)
+	    for dir in bad_dirs:
+                if dir in rpath:
+                    messages.append("package %s contains bad RPATH %s in file %s" % (name, rpath, file))
 
 QAPATHTEST[useless-rpaths] = "package_qa_check_useless_rpaths"
 def package_qa_check_useless_rpaths(file, name, d, elf, messages):
@@ -174,15 +171,14 @@ def package_qa_check_useless_rpaths(file, name, d, elf, messages):
     if not elf:
         return
 
-    objdump = d.getVar('OBJDUMP', True)
-    env_path = d.getVar('PATH', True)
-
     libdir = d.getVar("libdir", True)
     base_libdir = d.getVar("base_libdir", True)
 
+    phdrs = elf.run_objdump("-p", d)
+
     import re
     rpath_re = re.compile("\s+RPATH\s+(.*)")
-    for line in os.popen("LC_ALL=C PATH=%s %s -p '%s' 2> /dev/null" % (env_path, objdump, file), "r"):
+    for line in phdrs.split("\n"):
     	m = rpath_re.match(line)
 	if m:
 	   rpath = m.group(1)
@@ -443,14 +439,13 @@ def package_qa_hash_style(path, name, d, elf, messages):
     if not gnu_hash:
         return
 
-    objdump = d.getVar('OBJDUMP', True)
-    env_path = d.getVar('PATH', True)
-
     sane = False
     has_syms = False
 
+    phdrs = elf.run_objdump("-p", d)
+
     # If this binary has symbols, we expect it to have GNU_HASH too.
-    for line in os.popen("LC_ALL=C PATH=%s %s -p '%s' 2> /dev/null" % (env_path, objdump, path), "r"):
+    for line in phdrs.split("\n"):
         if "SYMTAB" in line:
             has_syms = True
         if "GNU_HASH" in line:
diff --git a/meta/lib/oe/qa.py b/meta/lib/oe/qa.py
index d380012..9e5ab58 100644
--- a/meta/lib/oe/qa.py
+++ b/meta/lib/oe/qa.py
@@ -28,6 +28,7 @@ class ELFFile:
     def __init__(self, name, bits = 0):
         self.name = name
         self.bits = bits
+        self.objdump_output = {}
 
     def open(self):
         self.file = file(self.name, "r")
@@ -87,3 +88,19 @@ class ELFFile:
         import struct
         (a,) = struct.unpack(self.sex+"H", self.data[18:20])
         return a
+
+    def run_objdump(self, cmd, d):
+        import bb.process
+        import sys
+
+        if self.objdump_output.has_key(cmd):
+            return self.objdump_output[cmd]
+
+        objdump = d.getVar('OBJDUMP', True)
+        staging_dir = d.getVar('STAGING_BINDIR_TOOLCHAIN', True)
+
+        env = os.environ
+        env["LC_ALL"] = "C"
+
+        self.objdump_output[cmd] = bb.process.run([ os.path.join(staging_dir, objdump), cmd, self.name ], env=env, shell=False)[0]
+        return self.objdump_output[cmd]
-- 
1.7.10.4

Re: [PATCH] insane: Rationalise phdrs-based QA checks

[Saul Wold]

On 10/01/2012 10:29 AM, Phil Blundell wrote:
> Various different QA checks are based on essentially the same data from
> the ELF program headers.  Calling objdump to extract it repeatedly is
> inefficient, particularly if the shell is involved.  Instead, let's
> cache the output from objdump inside the qa.elf object and allow it to
> be reused by multiple tests.
>
> Also, using objdump instead of scanelf to check for bad RPATHs (in the
> same way that the useless-rpaths check was doing already) allows the
> dependency on pax-utils-native to be dropped.
>
This seems to be failing for a QemuArm build of world, specifically 
lsbsetup, quilt, sysvinit, and foomatic-filters seems like its failing 
on symlinks.



> Signed-off-by: Phil Blundell <philb@gnu.org>
> ---
>   meta/classes/insane.bbclass |   41 ++++++++++++++++++-----------------------
>   meta/lib/oe/qa.py           |   17 +++++++++++++++++
>   2 files changed, 35 insertions(+), 23 deletions(-)
>
> diff --git a/meta/classes/insane.bbclass b/meta/classes/insane.bbclass
> index 1fb8970..3705fe9 100644
> --- a/meta/classes/insane.bbclass
> +++ b/meta/classes/insane.bbclass
> @@ -17,13 +17,8 @@
>   #   files under exec_prefix
>
>
> -#
> -# We need to have the scanelf utility as soon as
> -# possible and this is contained within the pax-utils-native.
> -# The package.bbclass can help us here.
> -#
>   inherit package
> -PACKAGE_DEPENDS += "pax-utils-native ${QADEPENDS}"
> +PACKAGE_DEPENDS += "${QADEPENDS}"
>   PACKAGEFUNCS += " do_package_qa "
>
>   # unsafe-references-in-binaries requires prelink-rtld from
> @@ -147,21 +142,23 @@ def package_qa_check_rpath(file,name, d, elf, messages):
>       if not elf:
>           return
>
> -    scanelf = os.path.join(d.getVar('STAGING_BINDIR_NATIVE',True),'scanelf')
>       bad_dirs = [d.getVar('TMPDIR', True) + "/work", d.getVar('STAGING_DIR_TARGET', True)]
>       bad_dir_test = d.getVar('TMPDIR', True)
> -    if not os.path.exists(scanelf):
> -        bb.fatal("Can not check RPATH, scanelf (part of pax-utils-native) not found")
>
>       if not bad_dirs[0] in d.getVar('WORKDIR', True):
>           bb.fatal("This class assumed that WORKDIR is ${TMPDIR}/work... Not doing any check")
>
> -    output = os.popen("%s -B -F%%r#F '%s'" % (scanelf,file))
> -    txt    = output.readline().split()
> -    for line in txt:
> -        for dir in bad_dirs:
> -            if dir in line:
> -                messages.append("package %s contains bad RPATH %s in file %s" % (name, line, file))
> +    phdrs = elf.run_objdump("-p", d)
> +
> +    import re
> +    rpath_re = re.compile("\s+RPATH\s+(.*)")
> +    for line in phdrs.split("\n"):
> +    	m = rpath_re.match(line)
> +	if m:
> +	    rpath = m.group(1)
> +	    for dir in bad_dirs:
> +                if dir in rpath:
> +                    messages.append("package %s contains bad RPATH %s in file %s" % (name, rpath, file))
>
>   QAPATHTEST[useless-rpaths] = "package_qa_check_useless_rpaths"
>   def package_qa_check_useless_rpaths(file, name, d, elf, messages):
> @@ -174,15 +171,14 @@ def package_qa_check_useless_rpaths(file, name, d, elf, messages):
>       if not elf:
>           return
>
> -    objdump = d.getVar('OBJDUMP', True)
> -    env_path = d.getVar('PATH', True)
> -
>       libdir = d.getVar("libdir", True)
>       base_libdir = d.getVar("base_libdir", True)
>
> +    phdrs = elf.run_objdump("-p", d)
> +
>       import re
>       rpath_re = re.compile("\s+RPATH\s+(.*)")
> -    for line in os.popen("LC_ALL=C PATH=%s %s -p '%s' 2> /dev/null" % (env_path, objdump, file), "r"):
> +    for line in phdrs.split("\n"):
>       	m = rpath_re.match(line)
>   	if m:
>   	   rpath = m.group(1)
> @@ -443,14 +439,13 @@ def package_qa_hash_style(path, name, d, elf, messages):
>       if not gnu_hash:
>           return
>
> -    objdump = d.getVar('OBJDUMP', True)
> -    env_path = d.getVar('PATH', True)
> -
>       sane = False
>       has_syms = False
>
> +    phdrs = elf.run_objdump("-p", d)
> +
>       # If this binary has symbols, we expect it to have GNU_HASH too.
> -    for line in os.popen("LC_ALL=C PATH=%s %s -p '%s' 2> /dev/null" % (env_path, objdump, path), "r"):
> +    for line in phdrs.split("\n"):
>           if "SYMTAB" in line:
>               has_syms = True
>           if "GNU_HASH" in line:
> diff --git a/meta/lib/oe/qa.py b/meta/lib/oe/qa.py
> index d380012..9e5ab58 100644
> --- a/meta/lib/oe/qa.py
> +++ b/meta/lib/oe/qa.py
> @@ -28,6 +28,7 @@ class ELFFile:
>       def __init__(self, name, bits = 0):
>           self.name = name
>           self.bits = bits
> +        self.objdump_output = {}
>
>       def open(self):
>           self.file = file(self.name, "r")
> @@ -87,3 +88,19 @@ class ELFFile:
>           import struct
>           (a,) = struct.unpack(self.sex+"H", self.data[18:20])
>           return a
> +
> +    def run_objdump(self, cmd, d):
> +        import bb.process
> +        import sys
> +
> +        if self.objdump_output.has_key(cmd):
> +            return self.objdump_output[cmd]
> +
> +        objdump = d.getVar('OBJDUMP', True)
> +        staging_dir = d.getVar('STAGING_BINDIR_TOOLCHAIN', True)
> +
> +        env = os.environ
> +        env["LC_ALL"] = "C"
> +
> +        self.objdump_output[cmd] = bb.process.run([ os.path.join(staging_dir, objdump), cmd, self.name ], env=env, shell=False)[0]
> +        return self.objdump_output[cmd]
>

Re: [PATCH] insane: Rationalise phdrs-based QA checks

[Phil Blundell]

[-- Attachment #1: Type: text/plain, Size: 1042 bytes --]

On Sun, 2012-10-14 at 14:45 -0700, Saul Wold wrote:
> On 10/01/2012 10:29 AM, Phil Blundell wrote:
> > Various different QA checks are based on essentially the same data from
> > the ELF program headers.  Calling objdump to extract it repeatedly is
> > inefficient, particularly if the shell is involved.  Instead, let's
> > cache the output from objdump inside the qa.elf object and allow it to
> > be reused by multiple tests.
> >
> > Also, using objdump instead of scanelf to check for bad RPATHs (in the
> > same way that the useless-rpaths check was doing already) allows the
> > dependency on pax-utils-native to be dropped.
> >
> This seems to be failing for a QemuArm build of world, specifically 
> lsbsetup, quilt, sysvinit, and foomatic-filters seems like its failing 
> on symlinks.

I wasn't able to complete a build of world successfully due to some
unrelated-looking breakage in xserver-xorg, but I did reproduce this
problem by building quilt by hand.  The attached patch fixes it for me.

thanks

p.


[-- Attachment #2: 0001-insane-Don-t-try-to-run-objdump-on-symlinks.patch --]
[-- Type: text/x-patch, Size: 1478 bytes --]

From 0aa4c262ded3e9d9da8293d899cd6ec28b4f60bb Mon Sep 17 00:00:00 2001
From: Phil Blundell <philb@gnu.org>
Date: Mon, 15 Oct 2012 11:28:00 +0100
Subject: [PATCH] insane: Don't try to run objdump on symlinks

If the link is absolute then we might end up reading from a host binary
or a nonexistent path, neither of which will produce useful results and
may result in objdump failure and python backtrace spew.  If the link
does point to a binary within the installation root then we will scan the
pointed-to file at some point anyway so there is no need to do it again.

Signed-off-by: Phil Blundell <pb@pbcl.net>
---
 meta/classes/insane.bbclass |    6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/meta/classes/insane.bbclass b/meta/classes/insane.bbclass
index 2b48419..71a9a58 100644
--- a/meta/classes/insane.bbclass
+++ b/meta/classes/insane.bbclass
@@ -157,6 +157,9 @@ def package_qa_check_rpath(file,name, d, elf, messages):
     if not elf:
         return
 
+    if os.path.islink(file):
+        return
+
     bad_dirs = [d.getVar('TMPDIR', True) + "/work", d.getVar('STAGING_DIR_TARGET', True)]
     bad_dir_test = d.getVar('TMPDIR', True)
 
@@ -186,6 +189,9 @@ def package_qa_check_useless_rpaths(file, name, d, elf, messages):
     if not elf:
         return
 
+    if os.path.islink(file):
+        return
+
     libdir = d.getVar("libdir", True)
     base_libdir = d.getVar("base_libdir", True)
 
-- 
1.7.10.4

Re: [PATCH] insane: Rationalise phdrs-based QA checks

[Saul Wold]

On 10/15/2012 03:32 AM, Phil Blundell wrote:
> On Sun, 2012-10-14 at 14:45 -0700, Saul Wold wrote:
>> On 10/01/2012 10:29 AM, Phil Blundell wrote:
>>> Various different QA checks are based on essentially the same data from
>>> the ELF program headers.  Calling objdump to extract it repeatedly is
>>> inefficient, particularly if the shell is involved.  Instead, let's
>>> cache the output from objdump inside the qa.elf object and allow it to
>>> be reused by multiple tests.
>>>
>>> Also, using objdump instead of scanelf to check for bad RPATHs (in the
>>> same way that the useless-rpaths check was doing already) allows the
>>> dependency on pax-utils-native to be dropped.
>>>
>> This seems to be failing for a QemuArm build of world, specifically
>> lsbsetup, quilt, sysvinit, and foomatic-filters seems like its failing
>> on symlinks.
>
> I wasn't able to complete a build of world successfully due to some
> unrelated-looking breakage in xserver-xorg, but I did reproduce this
> problem by building quilt by hand.  The attached patch fixes it for me.
>
This is better, but I found another failure:

> ERROR: Error executing a python function in /intel/distro/meta/recipes-devtools/qemu/qemu_1.2.0.bb:
> ExecutionError: Execution of '/intel/poky/builds/world/tmp/sysroots/x86_64-linux/usr/bin/armv5te-poky-linux-gnueabi/arm-poky-linux-gnueabi-objdump -p /intel/poky/builds/world/tmp/work/armv5te-poky-linux-gnueabi/qemu-1.2.0-r3/packages-split/qemu/usr/share/qemu/palcode-clipper' failed with exit code 1:
> /intel/poky/builds/world/tmp/sysroots/x86_64-linux/usr/bin/armv5te-poky-linux-gnueabi/arm-poky-linux-gnueabi-objdump: /intel/poky/builds/world/tmp/work/armv5te-poky-linux-gnueabi/qemu-1.2.0-r3/packages-split/qemu/usr/share/qemu/palcode-clipper: File format not recognized
>

When I run file:
/intel/poky/builds/world/tmp/work/armv5te-poky-linux-gnueabi/qemu-1.2.0-r3/packages-split/qemu/usr/share/qemu/palcode-clipper: 
ELF 64-bit LSB executable, Alpha (unofficial), version 1 (SYSV), 
statically linked, not stripped

I was building qemuarm, but I had a done a qemuppc build earlier also.

Sau!

> thanks
>
> p.
>

Re: [PATCH] insane: Rationalise phdrs-based QA checks

[Phil Blundell]

On Tue, 2012-10-16 at 11:29 -0700, Saul Wold wrote:
> On 10/15/2012 03:32 AM, Phil Blundell wrote:
> > On Sun, 2012-10-14 at 14:45 -0700, Saul Wold wrote:
> >> On 10/01/2012 10:29 AM, Phil Blundell wrote:
> >>> Various different QA checks are based on essentially the same data from
> >>> the ELF program headers.  Calling objdump to extract it repeatedly is
> >>> inefficient, particularly if the shell is involved.  Instead, let's
> >>> cache the output from objdump inside the qa.elf object and allow it to
> >>> be reused by multiple tests.
> >>>
> >>> Also, using objdump instead of scanelf to check for bad RPATHs (in the
> >>> same way that the useless-rpaths check was doing already) allows the
> >>> dependency on pax-utils-native to be dropped.
> >>>
> >> This seems to be failing for a QemuArm build of world, specifically
> >> lsbsetup, quilt, sysvinit, and foomatic-filters seems like its failing
> >> on symlinks.
> >
> > I wasn't able to complete a build of world successfully due to some
> > unrelated-looking breakage in xserver-xorg, but I did reproduce this
> > problem by building quilt by hand.  The attached patch fixes it for me.
> >
> This is better, but I found another failure:
> 
> > ERROR: Error executing a python function in /intel/distro/meta/recipes-devtools/qemu/qemu_1.2.0.bb:
> > ExecutionError: Execution of '/intel/poky/builds/world/tmp/sysroots/x86_64-linux/usr/bin/armv5te-poky-linux-gnueabi/arm-poky-linux-gnueabi-objdump -p /intel/poky/builds/world/tmp/work/armv5te-poky-linux-gnueabi/qemu-1.2.0-r3/packages-split/qemu/usr/share/qemu/palcode-clipper' failed with exit code 1:
> > /intel/poky/builds/world/tmp/sysroots/x86_64-linux/usr/bin/armv5te-poky-linux-gnueabi/arm-poky-linux-gnueabi-objdump: /intel/poky/builds/world/tmp/work/armv5te-poky-linux-gnueabi/qemu-1.2.0-r3/packages-split/qemu/usr/share/qemu/palcode-clipper: File format not recognized
> >
> 
> When I run file:
> /intel/poky/builds/world/tmp/work/armv5te-poky-linux-gnueabi/qemu-1.2.0-r3/packages-split/qemu/usr/share/qemu/palcode-clipper: 
> ELF 64-bit LSB executable, Alpha (unofficial), version 1 (SYSV), 
> statically linked, not stripped
> 
> I was building qemuarm, but I had a done a qemuppc build earlier also.

Well, that's a bit weird.  It seems as though you have somehow gotten an
Alpha binary installed, which isn't appropriate for either qemuarm or
qemuppc.  In fact I don't think we support alpha in oe-core at all so
it's a bit of a mystery how it would have been built in the first place.

On the one hand, this is a genuine problem and it's good that the QA
check is diagnosing it.  On the other hand, it probably oughtn't to be
diagnosing it by means of an objdump failure (and I think there is
already a dedicated check for wrong ELF arch anyway).  So probably the
right thing to do is just trap exceptions around running objdump and
ignore any files that it doesn't understand.

p.

Re: [PATCH] insane: Rationalise phdrs-based QA checks

[Phil Blundell]

[-- Attachment #1: Type: text/plain, Size: 3870 bytes --]

On Tue, 2012-10-16 at 20:14 +0100, Phil Blundell wrote:
> On Tue, 2012-10-16 at 11:29 -0700, Saul Wold wrote:
> > On 10/15/2012 03:32 AM, Phil Blundell wrote:
> > > On Sun, 2012-10-14 at 14:45 -0700, Saul Wold wrote:
> > >> On 10/01/2012 10:29 AM, Phil Blundell wrote:
> > >>> Various different QA checks are based on essentially the same data from
> > >>> the ELF program headers.  Calling objdump to extract it repeatedly is
> > >>> inefficient, particularly if the shell is involved.  Instead, let's
> > >>> cache the output from objdump inside the qa.elf object and allow it to
> > >>> be reused by multiple tests.
> > >>>
> > >>> Also, using objdump instead of scanelf to check for bad RPATHs (in the
> > >>> same way that the useless-rpaths check was doing already) allows the
> > >>> dependency on pax-utils-native to be dropped.
> > >>>
> > >> This seems to be failing for a QemuArm build of world, specifically
> > >> lsbsetup, quilt, sysvinit, and foomatic-filters seems like its failing
> > >> on symlinks.
> > >
> > > I wasn't able to complete a build of world successfully due to some
> > > unrelated-looking breakage in xserver-xorg, but I did reproduce this
> > > problem by building quilt by hand.  The attached patch fixes it for me.
> > >
> > This is better, but I found another failure:
> > 
> > > ERROR: Error executing a python function in /intel/distro/meta/recipes-devtools/qemu/qemu_1.2.0.bb:
> > > ExecutionError: Execution of '/intel/poky/builds/world/tmp/sysroots/x86_64-linux/usr/bin/armv5te-poky-linux-gnueabi/arm-poky-linux-gnueabi-objdump -p /intel/poky/builds/world/tmp/work/armv5te-poky-linux-gnueabi/qemu-1.2.0-r3/packages-split/qemu/usr/share/qemu/palcode-clipper' failed with exit code 1:
> > > /intel/poky/builds/world/tmp/sysroots/x86_64-linux/usr/bin/armv5te-poky-linux-gnueabi/arm-poky-linux-gnueabi-objdump: /intel/poky/builds/world/tmp/work/armv5te-poky-linux-gnueabi/qemu-1.2.0-r3/packages-split/qemu/usr/share/qemu/palcode-clipper: File format not recognized
> > >
> > 
> > When I run file:
> > /intel/poky/builds/world/tmp/work/armv5te-poky-linux-gnueabi/qemu-1.2.0-r3/packages-split/qemu/usr/share/qemu/palcode-clipper: 
> > ELF 64-bit LSB executable, Alpha (unofficial), version 1 (SYSV), 
> > statically linked, not stripped
> > 
> > I was building qemuarm, but I had a done a qemuppc build earlier also.
> 
> Well, that's a bit weird.  It seems as though you have somehow gotten an
> Alpha binary installed, which isn't appropriate for either qemuarm or
> qemuppc.  In fact I don't think we support alpha in oe-core at all so
> it's a bit of a mystery how it would have been built in the first place.
> 
> On the one hand, this is a genuine problem and it's good that the QA
> check is diagnosing it.  On the other hand, it probably oughtn't to be
> diagnosing it by means of an objdump failure (and I think there is
> already a dedicated check for wrong ELF arch anyway).  So probably the
> right thing to do is just trap exceptions around running objdump and
> ignore any files that it doesn't understand.

So, it turns out that:

a) qemu is deliberately turning off the "wrong ELF architecture" check,
which implies that it wants to ship these foreign-architecture binaries.
It's not clear to me why this is a useful thing to do, but still.

b) if binutils is configured for qemux86 then it is quite happy to let
you run "objdump -p" on alpha and sparc binaries, which is what I would
have expected.  If it is configured for qemuarm then, for reasons which
are mysterious to me, you get "File format not recognised" in that
situation.

Anyway, we clearly shouldn't get a failure in that situation.  The
attached patch causes qa.py to trap errors from objdump and just return
a null output, which is pretty much what the calling code will expect.

p.


[-- Attachment #2: 0001-qa-Trap-exceptions-when-running-objdump.patch --]
[-- Type: text/x-patch, Size: 1373 bytes --]

From ec8a41e464ae0fba8ad1d72f529a38b5ae30bfcf Mon Sep 17 00:00:00 2001
From: Phil Blundell <philb@gnu.org>
Date: Wed, 17 Oct 2012 21:34:58 +0100
Subject: [PATCH] qa: Trap exceptions when running objdump

This avoids propagating a failure if we encounter an ELF file
that objdump can't parse for any reason.  Some versions and/or
configurations of objdump will refuse to read files for "the
wrong" architecture.

Signed-off-by: Phil Blundell <pb@pbcl.net>
---
 meta/lib/oe/qa.py |    9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/meta/lib/oe/qa.py b/meta/lib/oe/qa.py
index 9e5ab58..12dcd1f 100644
--- a/meta/lib/oe/qa.py
+++ b/meta/lib/oe/qa.py
@@ -102,5 +102,10 @@ class ELFFile:
         env = os.environ
         env["LC_ALL"] = "C"
 
-        self.objdump_output[cmd] = bb.process.run([ os.path.join(staging_dir, objdump), cmd, self.name ], env=env, shell=False)[0]
-        return self.objdump_output[cmd]
+        try:
+            bb.note("%s %s %s" % (objdump, cmd, self.name))
+            self.objdump_output[cmd] = bb.process.run([ os.path.join(staging_dir, objdump), cmd, self.name ], env=env, shell=False)[0]
+            return self.objdump_output[cmd]
+        except Exception, e:
+            bb.note("%s %s %s failed: %s" % (objdump, cmd, self.name, e))
+            return ""
-- 
1.7.10.4

Re: [PATCH] insane: Rationalise phdrs-based QA checks

[Khem Raj]

On Oct 17, 2012, at 1:39 PM, Phil Blundell <philb@gnu.org> wrote:

> On Tue, 2012-10-16 at 20:14 +0100, Phil Blundell wrote:
>> On Tue, 2012-10-16 at 11:29 -0700, Saul Wold wrote:
>>> On 10/15/2012 03:32 AM, Phil Blundell wrote:
>>>> On Sun, 2012-10-14 at 14:45 -0700, Saul Wold wrote:
>>>>> On 10/01/2012 10:29 AM, Phil Blundell wrote:
>>>>>> Various different QA checks are based on essentially the same data from
>>>>>> the ELF program headers.  Calling objdump to extract it repeatedly is
>>>>>> inefficient, particularly if the shell is involved.  Instead, let's
>>>>>> cache the output from objdump inside the qa.elf object and allow it to
>>>>>> be reused by multiple tests.
>>>>>> 
>>>>>> Also, using objdump instead of scanelf to check for bad RPATHs (in the
>>>>>> same way that the useless-rpaths check was doing already) allows the
>>>>>> dependency on pax-utils-native to be dropped.
>>>>>> 
>>>>> This seems to be failing for a QemuArm build of world, specifically
>>>>> lsbsetup, quilt, sysvinit, and foomatic-filters seems like its failing
>>>>> on symlinks.
>>>> 
>>>> I wasn't able to complete a build of world successfully due to some
>>>> unrelated-looking breakage in xserver-xorg, but I did reproduce this
>>>> problem by building quilt by hand.  The attached patch fixes it for me.
>>>> 
>>> This is better, but I found another failure:
>>> 
>>>> ERROR: Error executing a python function in /intel/distro/meta/recipes-devtools/qemu/qemu_1.2.0.bb:
>>>> ExecutionError: Execution of '/intel/poky/builds/world/tmp/sysroots/x86_64-linux/usr/bin/armv5te-poky-linux-gnueabi/arm-poky-linux-gnueabi-objdump -p /intel/poky/builds/world/tmp/work/armv5te-poky-linux-gnueabi/qemu-1.2.0-r3/packages-split/qemu/usr/share/qemu/palcode-clipper' failed with exit code 1:
>>>> /intel/poky/builds/world/tmp/sysroots/x86_64-linux/usr/bin/armv5te-poky-linux-gnueabi/arm-poky-linux-gnueabi-objdump: /intel/poky/builds/world/tmp/work/armv5te-poky-linux-gnueabi/qemu-1.2.0-r3/packages-split/qemu/usr/share/qemu/palcode-clipper: File format not recognized
>>>> 
>>> 
>>> When I run file:
>>> /intel/poky/builds/world/tmp/work/armv5te-poky-linux-gnueabi/qemu-1.2.0-r3/packages-split/qemu/usr/share/qemu/palcode-clipper: 
>>> ELF 64-bit LSB executable, Alpha (unofficial), version 1 (SYSV), 
>>> statically linked, not stripped
>>> 
>>> I was building qemuarm, but I had a done a qemuppc build earlier also.
>> 
>> Well, that's a bit weird.  It seems as though you have somehow gotten an
>> Alpha binary installed, which isn't appropriate for either qemuarm or
>> qemuppc.  In fact I don't think we support alpha in oe-core at all so
>> it's a bit of a mystery how it would have been built in the first place.
>> 
>> On the one hand, this is a genuine problem and it's good that the QA
>> check is diagnosing it.  On the other hand, it probably oughtn't to be
>> diagnosing it by means of an objdump failure (and I think there is
>> already a dedicated check for wrong ELF arch anyway).  So probably the
>> right thing to do is just trap exceptions around running objdump and
>> ignore any files that it doesn't understand.
> 
> So, it turns out that:
> 
> a) qemu is deliberately turning off the "wrong ELF architecture" check,
> which implies that it wants to ship these foreign-architecture binaries.
> It's not clear to me why this is a useful thing to do, but still.
> 
> b) if binutils is configured for qemux86 then it is quite happy to let
> you run "objdump -p" on alpha and sparc binaries, which is what I would
> have expected.  If it is configured for qemuarm then, for reasons which
> are mysterious to me, you get "File format not recognised" in that
> situation.

we should add --enable-targets=all --with-64-bit-bfd and that should build all possible BFDs into binutils
and then it will be able to dump different architectures. 

> 
> Anyway, we clearly shouldn't get a failure in that situation.  The
> attached patch causes qa.py to trap errors from objdump and just return
> a null output, which is pretty much what the calling code will expect.
> 
> p.
> 
> <0001-qa-Trap-exceptions-when-running-objdump.patch>_______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core

Re: [PATCH] insane: Rationalise phdrs-based QA checks

[Saul Wold]

On 10/01/2012 10:29 AM, Phil Blundell wrote:
> Various different QA checks are based on essentially the same data from
> the ELF program headers.  Calling objdump to extract it repeatedly is
> inefficient, particularly if the shell is involved.  Instead, let's
> cache the output from objdump inside the qa.elf object and allow it to
> be reused by multiple tests.
>
> Also, using objdump instead of scanelf to check for bad RPATHs (in the
> same way that the useless-rpaths check was doing already) allows the
> dependency on pax-utils-native to be dropped.
>
> Signed-off-by: Phil Blundell <philb@gnu.org>
> ---
>   meta/classes/insane.bbclass |   41 ++++++++++++++++++-----------------------
>   meta/lib/oe/qa.py           |   17 +++++++++++++++++
>   2 files changed, 35 insertions(+), 23 deletions(-)
>

Merged this along with associated patches into 1.4

Thanks for your patience

	Sau!

(Yes, we are taking patches again, RP and I are vetting patches on the list)


> diff --git a/meta/classes/insane.bbclass b/meta/classes/insane.bbclass
> index 1fb8970..3705fe9 100644
> --- a/meta/classes/insane.bbclass
> +++ b/meta/classes/insane.bbclass
> @@ -17,13 +17,8 @@
>   #   files under exec_prefix
>
>
> -#
> -# We need to have the scanelf utility as soon as
> -# possible and this is contained within the pax-utils-native.
> -# The package.bbclass can help us here.
> -#
>   inherit package
> -PACKAGE_DEPENDS += "pax-utils-native ${QADEPENDS}"
> +PACKAGE_DEPENDS += "${QADEPENDS}"
>   PACKAGEFUNCS += " do_package_qa "
>
>   # unsafe-references-in-binaries requires prelink-rtld from
> @@ -147,21 +142,23 @@ def package_qa_check_rpath(file,name, d, elf, messages):
>       if not elf:
>           return
>
> -    scanelf = os.path.join(d.getVar('STAGING_BINDIR_NATIVE',True),'scanelf')
>       bad_dirs = [d.getVar('TMPDIR', True) + "/work", d.getVar('STAGING_DIR_TARGET', True)]
>       bad_dir_test = d.getVar('TMPDIR', True)
> -    if not os.path.exists(scanelf):
> -        bb.fatal("Can not check RPATH, scanelf (part of pax-utils-native) not found")
>
>       if not bad_dirs[0] in d.getVar('WORKDIR', True):
>           bb.fatal("This class assumed that WORKDIR is ${TMPDIR}/work... Not doing any check")
>
> -    output = os.popen("%s -B -F%%r#F '%s'" % (scanelf,file))
> -    txt    = output.readline().split()
> -    for line in txt:
> -        for dir in bad_dirs:
> -            if dir in line:
> -                messages.append("package %s contains bad RPATH %s in file %s" % (name, line, file))
> +    phdrs = elf.run_objdump("-p", d)
> +
> +    import re
> +    rpath_re = re.compile("\s+RPATH\s+(.*)")
> +    for line in phdrs.split("\n"):
> +    	m = rpath_re.match(line)
> +	if m:
> +	    rpath = m.group(1)
> +	    for dir in bad_dirs:
> +                if dir in rpath:
> +                    messages.append("package %s contains bad RPATH %s in file %s" % (name, rpath, file))
>
>   QAPATHTEST[useless-rpaths] = "package_qa_check_useless_rpaths"
>   def package_qa_check_useless_rpaths(file, name, d, elf, messages):
> @@ -174,15 +171,14 @@ def package_qa_check_useless_rpaths(file, name, d, elf, messages):
>       if not elf:
>           return
>
> -    objdump = d.getVar('OBJDUMP', True)
> -    env_path = d.getVar('PATH', True)
> -
>       libdir = d.getVar("libdir", True)
>       base_libdir = d.getVar("base_libdir", True)
>
> +    phdrs = elf.run_objdump("-p", d)
> +
>       import re
>       rpath_re = re.compile("\s+RPATH\s+(.*)")
> -    for line in os.popen("LC_ALL=C PATH=%s %s -p '%s' 2> /dev/null" % (env_path, objdump, file), "r"):
> +    for line in phdrs.split("\n"):
>       	m = rpath_re.match(line)
>   	if m:
>   	   rpath = m.group(1)
> @@ -443,14 +439,13 @@ def package_qa_hash_style(path, name, d, elf, messages):
>       if not gnu_hash:
>           return
>
> -    objdump = d.getVar('OBJDUMP', True)
> -    env_path = d.getVar('PATH', True)
> -
>       sane = False
>       has_syms = False
>
> +    phdrs = elf.run_objdump("-p", d)
> +
>       # If this binary has symbols, we expect it to have GNU_HASH too.
> -    for line in os.popen("LC_ALL=C PATH=%s %s -p '%s' 2> /dev/null" % (env_path, objdump, path), "r"):
> +    for line in phdrs.split("\n"):
>           if "SYMTAB" in line:
>               has_syms = True
>           if "GNU_HASH" in line:
> diff --git a/meta/lib/oe/qa.py b/meta/lib/oe/qa.py
> index d380012..9e5ab58 100644
> --- a/meta/lib/oe/qa.py
> +++ b/meta/lib/oe/qa.py
> @@ -28,6 +28,7 @@ class ELFFile:
>       def __init__(self, name, bits = 0):
>           self.name = name
>           self.bits = bits
> +        self.objdump_output = {}
>
>       def open(self):
>           self.file = file(self.name, "r")
> @@ -87,3 +88,19 @@ class ELFFile:
>           import struct
>           (a,) = struct.unpack(self.sex+"H", self.data[18:20])
>           return a
> +
> +    def run_objdump(self, cmd, d):
> +        import bb.process
> +        import sys
> +
> +        if self.objdump_output.has_key(cmd):
> +            return self.objdump_output[cmd]
> +
> +        objdump = d.getVar('OBJDUMP', True)
> +        staging_dir = d.getVar('STAGING_BINDIR_TOOLCHAIN', True)
> +
> +        env = os.environ
> +        env["LC_ALL"] = "C"
> +
> +        self.objdump_output[cmd] = bb.process.run([ os.path.join(staging_dir, objdump), cmd, self.name ], env=env, shell=False)[0]
> +        return self.objdump_output[cmd]
>