[Buildroot] [PATCH] support/script/check-bin-arch: ignore /usr/share

[Buildroot] [PATCH] support/script/check-bin-arch: ignore /usr/share

[Thomas Petazzoni]

/usr/share normally should not contain binaries executable for the
target platform. However, it might contain ELF binaries for other
platforms, such as firmware files installed by Qemu or
pru-software-support.

Instead of special-casing each package, let's simply ignore /usr/share.

Fixes:

  http://autobuild.buildroot.net/results/6f3fea9f6adaef1573fbb0dd6903b5d99e470610/
  (pru-software-support)

  http://autobuild.buildroot.net/results/fe8892bc22a03299fc41e30bfea5e42166838f88/
  (qemu)

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 support/scripts/check-bin-arch | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/support/scripts/check-bin-arch b/support/scripts/check-bin-arch
index 2c619ad..be2f371 100755
--- a/support/scripts/check-bin-arch
+++ b/support/scripts/check-bin-arch
@@ -27,6 +27,13 @@ for f in ${pkg_files} ; do
 		continue
 	fi
 
+	# Skip files in /usr/share, several packages (qemu,
+	# pru-software-support) legitimately install ELF binaries that
+	# are not for the target architecture
+	if [[ "${f}" =~ ^\./usr/share/.* ]]; then
+		continue
+	fi
+
 	# Get architecture using readelf. We pipe through 'head -1' so
 	# that when the file is a static library (.a), we only take
 	# into account the architecture of the first object file.
-- 
2.7.4

[Buildroot] [PATCH] support/script/check-bin-arch: ignore /usr/share

[Yann E. MORIN]

Thomas, All,

On 2017-03-21 21:35 +0100, Thomas Petazzoni spake thusly:
> /usr/share normally should not contain binaries executable for the
> target platform. However, it might contain ELF binaries for other
> platforms, such as firmware files installed by Qemu or
> pru-software-support.
> 
> Instead of special-casing each package, let's simply ignore /usr/share.
> 
> Fixes:
> 
>   http://autobuild.buildroot.net/results/6f3fea9f6adaef1573fbb0dd6903b5d99e470610/
>   (pru-software-support)
> 
>   http://autobuild.buildroot.net/results/fe8892bc22a03299fc41e30bfea5e42166838f88/
>   (qemu)
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

Regards,
Yann E. MORIN.

> ---
>  support/scripts/check-bin-arch | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/support/scripts/check-bin-arch b/support/scripts/check-bin-arch
> index 2c619ad..be2f371 100755
> --- a/support/scripts/check-bin-arch
> +++ b/support/scripts/check-bin-arch
> @@ -27,6 +27,13 @@ for f in ${pkg_files} ; do
>  		continue
>  	fi
>  
> +	# Skip files in /usr/share, several packages (qemu,
> +	# pru-software-support) legitimately install ELF binaries that
> +	# are not for the target architecture
> +	if [[ "${f}" =~ ^\./usr/share/.* ]]; then
> +		continue
> +	fi
> +
>  	# Get architecture using readelf. We pipe through 'head -1' so
>  	# that when the file is a static library (.a), we only take
>  	# into account the architecture of the first object file.
> -- 
> 2.7.4
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH] support/script/check-bin-arch: ignore /usr/share

[Peter Korsgaard]

>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes:

 > /usr/share normally should not contain binaries executable for the
 > target platform. However, it might contain ELF binaries for other
 > platforms, such as firmware files installed by Qemu or
 > pru-software-support.

 > Instead of special-casing each package, let's simply ignore /usr/share.

 > Fixes:

 >   http://autobuild.buildroot.net/results/6f3fea9f6adaef1573fbb0dd6903b5d99e470610/
 >   (pru-software-support)

 >   http://autobuild.buildroot.net/results/fe8892bc22a03299fc41e30bfea5e42166838f88/
 >   (qemu)

 > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

Committed, thanks.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH] support/script/check-bin-arch: ignore /usr/share

[Peter Korsgaard]

>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes:

 > /usr/share normally should not contain binaries executable for the
 > target platform. However, it might contain ELF binaries for other
 > platforms, such as firmware files installed by Qemu or
 > pru-software-support.

 > Instead of special-casing each package, let's simply ignore /usr/share.

 > Fixes:

 >   http://autobuild.buildroot.net/results/6f3fea9f6adaef1573fbb0dd6903b5d99e470610/
 >   (pru-software-support)

 >   http://autobuild.buildroot.net/results/fe8892bc22a03299fc41e30bfea5e42166838f88/
 >   (qemu)

Hmm, this doesn't seem to work?

http://autobuild.buildroot.net/results/4e2/4e27559827f3ed75a12f13bd595998bf661b2aaf/build-end.log
http://autobuild.buildroot.net/results/99e/99e4ed21116c721faf9f1d0349f312b357d333ee/build-end.log

I wonder if we shouldn't turn the tests around, E.G. instead of
searching for elf files with a machine different from target, search for
a files with machine == host.

If we do this then we need to disable the test when host == target (but
the current tests aren't really helpful either for such setups).

This can naturally still fail if somebody adds a package (E.G. in
br2-external) that installs a i386/x86-64 binary. Presumably this could
happen if you want to include a PC application to inside the firmware
(E.G. downloadable through the web interface and used for controlling
the firmware or similar), but it doesn't get caught up in all these
build issues about various other firmware files or slightly different
machine strings (sparc / sparcv9, arcompat / arcv2)..

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH] support/script/check-bin-arch: ignore /usr/share

[Thomas Petazzoni]

Hello,

On Wed, 22 Mar 2017 09:24:56 +0100, Peter Korsgaard wrote:

> http://autobuild.buildroot.net/results/4e2/4e27559827f3ed75a12f13bd595998bf661b2aaf/build-end.log
> http://autobuild.buildroot.net/results/99e/99e4ed21116c721faf9f1d0349f312b357d333ee/build-end.log
> 
> I wonder if we shouldn't turn the tests around, E.G. instead of
> searching for elf files with a machine different from target, search for
> a files with machine == host.

Indeed, that's an option. We would need to have only the readelf
machine string for the most common build architectures (x86, x86-64),
and if Buildroot is used on a different architecture, we simply don't
do the check.

> If we do this then we need to disable the test when host == target (but
> the current tests aren't really helpful either for such setups).

Correct.

> This can naturally still fail if somebody adds a package (E.G. in
> br2-external) that installs a i386/x86-64 binary. Presumably this could
> happen if you want to include a PC application to inside the firmware
> (E.G. downloadable through the web interface and used for controlling
> the firmware or similar), but it doesn't get caught up in all these
> build issues about various other firmware files or slightly different
> machine strings (sparc / sparcv9, arcompat / arcv2)..

I must say I still like the fact that we detect sparc vs. sparcv9,
arcompat vs arcv2. I.e why do we have some binaries that have a
different machine number than most of the binaries being produced?

But I agree it's of limited usefulness, so if you want me to change the
mechanism by inverting the logic, I can cook some patches.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[Buildroot] [PATCH] support/script/check-bin-arch: ignore /usr/share

[Peter Korsgaard]

>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes:

 > Hello,
 > On Wed, 22 Mar 2017 09:24:56 +0100, Peter Korsgaard wrote:

 >> http://autobuild.buildroot.net/results/4e2/4e27559827f3ed75a12f13bd595998bf661b2aaf/build-end.log
 >> http://autobuild.buildroot.net/results/99e/99e4ed21116c721faf9f1d0349f312b357d333ee/build-end.log
 >> 
 >> I wonder if we shouldn't turn the tests around, E.G. instead of
 >> searching for elf files with a machine different from target, search for
 >> a files with machine == host.

 > Indeed, that's an option. We would need to have only the readelf
 > machine string for the most common build architectures (x86, x86-64),
 > and if Buildroot is used on a different architecture, we simply don't
 > do the check.

Exactly, or alternatively we run readelf on a host utility (E.G. HOSTCC)
to detect the machine string.


 >> This can naturally still fail if somebody adds a package (E.G. in
 >> br2-external) that installs a i386/x86-64 binary. Presumably this could
 >> happen if you want to include a PC application to inside the firmware
 >> (E.G. downloadable through the web interface and used for controlling
 >> the firmware or similar), but it doesn't get caught up in all these
 >> build issues about various other firmware files or slightly different
 >> machine strings (sparc / sparcv9, arcompat / arcv2)..

 > I must say I still like the fact that we detect sparc vs. sparcv9,
 > arcompat vs arcv2. I.e why do we have some binaries that have a
 > different machine number than most of the binaries being produced?

Sorry, I don't know enough details about Sparc and Arc. It indeed seems
strange.


 > But I agree it's of limited usefulness, so if you want me to change the
 > mechanism by inverting the logic, I can cook some patches.

Well, just like for all other BR features I want to ensure that the
complexity/gain relation is OK. If we need to add more and more
exceptions and perhaps more complicated/fuzzy matching to handle these
machine string variantions then I think it would make sense to invert
the test to simplify - But lets see how it goes.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH] support/script/check-bin-arch: ignore /usr/share

[Thomas Petazzoni]

Hello,

On Wed, 22 Mar 2017 22:43:48 +0100, Peter Korsgaard wrote:

> Exactly, or alternatively we run readelf on a host utility (E.G. HOSTCC)
> to detect the machine string.

Indeed.

>  > I must say I still like the fact that we detect sparc vs. sparcv9,
>  > arcompat vs arcv2. I.e why do we have some binaries that have a
>  > different machine number than most of the binaries being produced?  
> 
> Sorry, I don't know enough details about Sparc and Arc. It indeed seems
> strange.

The ARC is resolved. I'll look into the SPARC one.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[Buildroot] [PATCH] support/script/check-bin-arch: ignore /usr/share

[Arnout Vandecappelle]

On 22-03-17 22:43, Peter Korsgaard wrote:
>>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes:
> 
>  > Hello,
>  > On Wed, 22 Mar 2017 09:24:56 +0100, Peter Korsgaard wrote:
> 
>  >> http://autobuild.buildroot.net/results/4e2/4e27559827f3ed75a12f13bd595998bf661b2aaf/build-end.log
>  >> http://autobuild.buildroot.net/results/99e/99e4ed21116c721faf9f1d0349f312b357d333ee/build-end.log

 Looks like the regexes in the script are all wrong: they match against
./something, but the file names are /something.

 Ah, that's Yann's doing: when moving the pkg_files to the while read loop, he
sneaked in the removal  of the initial . - without mentioning it in the commit
log. Tsk tsk.


>  >> I wonder if we shouldn't turn the tests around, E.G. instead of
>  >> searching for elf files with a machine different from target, search for
>  >> a files with machine == host.

 I don't agree. We don't only want to detect host-target confusion, we also want
to detect wrong compilation, e.g. missing -EB.

 By the way, we shouldn't be afraid to exclude files and directories. Take for
example /usr/share: some packages may actually install binaries there (e.g.
gstreamer installs a bash-completion helper there). However, all packages also
install something somewhere else, and it's very likely that if it does something
wrong, it's wrong everywhere.

 BTW, we should also manually check if there are no problems with the boot
loaders, which don't get triggered in the autobuilders...


>  > Indeed, that's an option. We would need to have only the readelf
>  > machine string for the most common build architectures (x86, x86-64),
>  > and if Buildroot is used on a different architecture, we simply don't
>  > do the check.
> 
> Exactly, or alternatively we run readelf on a host utility (E.G. HOSTCC)
> to detect the machine string.
> 
> 
>  >> This can naturally still fail if somebody adds a package (E.G. in
>  >> br2-external) that installs a i386/x86-64 binary. Presumably this could
>  >> happen if you want to include a PC application to inside the firmware
>  >> (E.G. downloadable through the web interface and used for controlling
>  >> the firmware or similar), but it doesn't get caught up in all these
>  >> build issues about various other firmware files or slightly different
>  >> machine strings (sparc / sparcv9, arcompat / arcv2)..
> 
>  > I must say I still like the fact that we detect sparc vs. sparcv9,
>  > arcompat vs arcv2. I.e why do we have some binaries that have a
>  > different machine number than most of the binaries being produced?
> 
> Sorry, I don't know enough details about Sparc and Arc. It indeed seems
> strange.
> 
> 
>  > But I agree it's of limited usefulness, so if you want me to change the
>  > mechanism by inverting the logic, I can cook some patches.
> 
> Well, just like for all other BR features I want to ensure that the
> complexity/gain relation is OK. If we need to add more and more
> exceptions and perhaps more complicated/fuzzy matching to handle these
> machine string variantions then I think it would make sense to invert
> the test to simplify - But lets see how it goes.

 Indeed, if it becomes too complicated we have to find a simpler approach.

 Regards,
 Arnout


-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH] support/script/check-bin-arch: ignore /usr/share

[Thomas Petazzoni]

Hello,

On Thu, 23 Mar 2017 00:03:57 +0100, Arnout Vandecappelle wrote:

> >  >> I wonder if we shouldn't turn the tests around, E.G. instead of
> >  >> searching for elf files with a machine different from target, search for
> >  >> a files with machine == host.  
> 
>  I don't agree. We don't only want to detect host-target confusion, we also want
> to detect wrong compilation, e.g. missing -EB.

This was exactly the case of the gst-ffmpeg package, for which I've
sent a fix. Everything was built for Sparc, but two libraries were
built for a slightly different version of Sparc, which was actually
incorrect.

Since for now, autobuilders issue caused by this seems to have all been
handled, I don't think there's much need to find a different solution.
Let's give it some more time and see if we need to change our mind.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[Buildroot] [PATCH] support/script/check-bin-arch: ignore /usr/share

[Peter Korsgaard]

>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes:

 > Hello,
 > On Thu, 23 Mar 2017 00:03:57 +0100, Arnout Vandecappelle wrote:

 >> >  >> I wonder if we shouldn't turn the tests around, E.G. instead of
 >> >  >> searching for elf files with a machine different from target, search for
 >> >  >> a files with machine == host.  
 >> 
 >> I don't agree. We don't only want to detect host-target confusion, we also want
 >> to detect wrong compilation, e.g. missing -EB.

 > This was exactly the case of the gst-ffmpeg package, for which I've
 > sent a fix. Everything was built for Sparc, but two libraries were
 > built for a slightly different version of Sparc, which was actually
 > incorrect.

 > Since for now, autobuilders issue caused by this seems to have all been
 > handled, I don't think there's much need to find a different solution.
 > Let's give it some more time and see if we need to change our mind.

Ok, sounds good to me. Thanks!

-- 
Bye, Peter Korsgaard