From 9fd0b3889f00ad13662879767d833309d8a035b6 Mon Sep 17 00:00:00 2001
From: Nicholas Fraser <[email protected]>
Date: Thu, 18 Feb 2021 13:24:03 -0500
Subject: [PATCH] perf buildid-cache: Add test for PE executable
This builds on the previous changes to tests/shell/buildid.sh, adding
tests for a PE file. It adds it to the build-id cache manually and, if
Wine is available, runs it under "perf record" and verifies that it was
added automatically.
If wine is not installed, only warnings are printed; the test can still
exit 0.
Signed-off-by: Nicholas Fraser <[email protected]>
---
tools/perf/tests/shell/buildid.sh | 65 +++++++++++++++++++++++++++----
1 file changed, 58 insertions(+), 7 deletions(-)
diff --git a/tools/perf/tests/shell/buildid.sh b/tools/perf/tests/shell/buildid.sh
index 416af614bbe0..f05670d1e39e 100755
--- a/tools/perf/tests/shell/buildid.sh
+++ b/tools/perf/tests/shell/buildid.sh
@@ -14,18 +14,56 @@ if ! [ -x "$(command -v cc)" ]; then
exit 2
fi
+# check what we need to test windows binaries
+add_pe=1
+run_pe=1
+if ! perf version --build-options | grep -q 'libbfd: .* on '; then
+ echo "WARNING: perf not built with libbfd. PE binaries will not be tested."
+ add_pe=0
+ run_pe=0
+fi
+if ! which wine > /dev/null; then
+ echo "WARNING: wine not found. PE binaries will not be run."
+ run_pe=0
+fi
+
+# set up wine
+if [ ${run_pe} -eq 1 ]; then
+ wineprefix=$(mktemp -d /tmp/perf.wineprefix.XXX)
+ export WINEPREFIX=${wineprefix}
+ # clear display variables to prevent wine from popping up dialogs
+ unset DISPLAY
+ unset WAYLAND_DISPLAY
+fi
+
ex_md5=$(mktemp /tmp/perf.ex.MD5.XXX)
ex_sha1=$(mktemp /tmp/perf.ex.SHA1.XXX)
+ex_pe=$(dirname $0)/../pe-file.exe
echo 'int main(void) { return 0; }' | cc -Wl,--build-id=sha1 -o ${ex_sha1} -x c -
echo 'int main(void) { return 0; }' | cc -Wl,--build-id=md5 -o ${ex_md5} -x c -
-echo "test binaries: ${ex_sha1} ${ex_md5}"
+echo "test binaries: ${ex_sha1} ${ex_md5} ${ex_pe}"
check()
{
- id=`readelf -n ${1} 2>/dev/null | grep 'Build ID' | awk '{print $3}'`
-
+ case $1 in
+ *.exe)
+ # We don't have a tool that can pull a nicely formatted build-id out of
+ # a PE file, but we can extract the whole section with objcopy and
+ # format it ourselves. The .buildid section is a Debug Directory
+ # containing a CodeView entry:
+ # https://docs.microsoft.com/en-us/windows/win32/debug/pe-format#debug-directory-image-only
+ # https://github.com/dotnet/runtime/blob/da94c022576a5c3bbc0e896f006565905eb137f9/docs/design/specs/PE-COFF.md
+ # The build-id starts at byte 33 and must be rearranged into a GUID.
+ id=`objcopy -O binary --only-section=.buildid $1 /dev/stdout | \
+ cut -c 33-48 | hexdump -ve '/1 "%02x"' | \
+ sed 's@^\(..\)\(..\)\(..\)\(..\)\(..\)\(..\)\(..\)\(..\)\(.*\)0a$@\4\3\2\1\6\5\8\7\9@'`
+ ;;
+ *)
+ id=`readelf -n ${1} 2>/dev/null | grep 'Build ID' | awk '{print $3}'`
+ ;;
+ esac
echo "build id: ${id}"
link=${build_id_dir}/.build-id/${id:0:2}/${id:2}
@@ -50,7 +88,7 @@ check()
exit 1
fi
- ${perf} buildid-cache -l | grep $id
+ ${perf} buildid-cache -l | grep ${id}
if [ $? -ne 0 ]; then
echo "failed: ${id} is not reported by \"perf buildid-cache -l\""
exit 1
@@ -79,16 +117,20 @@ test_record()
{
data=$(mktemp /tmp/perf.data.XXX)
build_id_dir=$(mktemp -d /tmp/perf.debug.XXX)
+ log=$(mktemp /tmp/perf.log.XXX)
perf="perf --buildid-dir ${build_id_dir}"
- ${perf} record --buildid-all -o ${data} ${1}
+ echo "running: perf record $@"
+ ${perf} record --buildid-all -o ${data} $@ &> ${log}
if [ $? -ne 0 ]; then
- echo "failed: record ${1}"
+ echo "failed: record $@"
+ echo "see log: ${log}"
exit 1
fi
- check ${1}
+ check ${@: -1}
+ rm -f ${log}
rm -rf ${build_id_dir}
rm -rf ${data}
}
@@ -96,12 +138,21 @@ test_record()
# add binaries manual via perf buildid-cache -a
test_add ${ex_sha1}
test_add ${ex_md5}
+if [ ${add_pe} -eq 1 ]; then
+ test_add ${ex_pe}
+fi
# add binaries via perf record post processing
test_record ${ex_sha1}
test_record ${ex_md5}
+if [ ${run_pe} -eq 1 ]; then
+ test_record wine ${ex_pe}
+fi
# cleanup
rm ${ex_sha1} ${ex_md5}
+if [ ${run_pe} -eq 1 ]; then
+ rm -r ${wineprefix}
+fi
exit ${err}
--
2.30.1
On Wed, Feb 24, 2021 at 02:59:16PM -0500, Nicholas Fraser wrote:
> From 9fd0b3889f00ad13662879767d833309d8a035b6 Mon Sep 17 00:00:00 2001
> From: Nicholas Fraser <[email protected]>
> Date: Thu, 18 Feb 2021 13:24:03 -0500
> Subject: [PATCH] perf buildid-cache: Add test for PE executable
>
> This builds on the previous changes to tests/shell/buildid.sh, adding
> tests for a PE file. It adds it to the build-id cache manually and, if
> Wine is available, runs it under "perf record" and verifies that it was
> added automatically.
>
> If wine is not installed, only warnings are printed; the test can still
> exit 0.
>
> Signed-off-by: Nicholas Fraser <[email protected]>
works nicely now, thanks
Acked-by: Jiri Olsa <[email protected]>
jirka
> ---
> tools/perf/tests/shell/buildid.sh | 65 +++++++++++++++++++++++++++----
> 1 file changed, 58 insertions(+), 7 deletions(-)
>
> diff --git a/tools/perf/tests/shell/buildid.sh b/tools/perf/tests/shell/buildid.sh
> index 416af614bbe0..f05670d1e39e 100755
> --- a/tools/perf/tests/shell/buildid.sh
> +++ b/tools/perf/tests/shell/buildid.sh
> @@ -14,18 +14,56 @@ if ! [ -x "$(command -v cc)" ]; then
> exit 2
> fi
>
> +# check what we need to test windows binaries
> +add_pe=1
> +run_pe=1
> +if ! perf version --build-options | grep -q 'libbfd: .* on '; then
> + echo "WARNING: perf not built with libbfd. PE binaries will not be tested."
> + add_pe=0
> + run_pe=0
> +fi
> +if ! which wine > /dev/null; then
> + echo "WARNING: wine not found. PE binaries will not be run."
> + run_pe=0
> +fi
> +
> +# set up wine
> +if [ ${run_pe} -eq 1 ]; then
> + wineprefix=$(mktemp -d /tmp/perf.wineprefix.XXX)
> + export WINEPREFIX=${wineprefix}
> + # clear display variables to prevent wine from popping up dialogs
> + unset DISPLAY
> + unset WAYLAND_DISPLAY
> +fi
> +
> ex_md5=$(mktemp /tmp/perf.ex.MD5.XXX)
> ex_sha1=$(mktemp /tmp/perf.ex.SHA1.XXX)
> +ex_pe=$(dirname $0)/../pe-file.exe
>
> echo 'int main(void) { return 0; }' | cc -Wl,--build-id=sha1 -o ${ex_sha1} -x c -
> echo 'int main(void) { return 0; }' | cc -Wl,--build-id=md5 -o ${ex_md5} -x c -
>
> -echo "test binaries: ${ex_sha1} ${ex_md5}"
> +echo "test binaries: ${ex_sha1} ${ex_md5} ${ex_pe}"
>
> check()
> {
> - id=`readelf -n ${1} 2>/dev/null | grep 'Build ID' | awk '{print $3}'`
> -
> + case $1 in
> + *.exe)
> + # We don't have a tool that can pull a nicely formatted build-id out of
> + # a PE file, but we can extract the whole section with objcopy and
> + # format it ourselves. The .buildid section is a Debug Directory
> + # containing a CodeView entry:
> + # https://docs.microsoft.com/en-us/windows/win32/debug/pe-format#debug-directory-image-only
> + # https://github.com/dotnet/runtime/blob/da94c022576a5c3bbc0e896f006565905eb137f9/docs/design/specs/PE-COFF.md
> + # The build-id starts at byte 33 and must be rearranged into a GUID.
> + id=`objcopy -O binary --only-section=.buildid $1 /dev/stdout | \
> + cut -c 33-48 | hexdump -ve '/1 "%02x"' | \
> + sed 's@^\(..\)\(..\)\(..\)\(..\)\(..\)\(..\)\(..\)\(..\)\(.*\)0a$@\4\3\2\1\6\5\8\7\9@'`
> + ;;
> + *)
> + id=`readelf -n ${1} 2>/dev/null | grep 'Build ID' | awk '{print $3}'`
> + ;;
> + esac
> echo "build id: ${id}"
>
> link=${build_id_dir}/.build-id/${id:0:2}/${id:2}
> @@ -50,7 +88,7 @@ check()
> exit 1
> fi
>
> - ${perf} buildid-cache -l | grep $id
> + ${perf} buildid-cache -l | grep ${id}
> if [ $? -ne 0 ]; then
> echo "failed: ${id} is not reported by \"perf buildid-cache -l\""
> exit 1
> @@ -79,16 +117,20 @@ test_record()
> {
> data=$(mktemp /tmp/perf.data.XXX)
> build_id_dir=$(mktemp -d /tmp/perf.debug.XXX)
> + log=$(mktemp /tmp/perf.log.XXX)
> perf="perf --buildid-dir ${build_id_dir}"
>
> - ${perf} record --buildid-all -o ${data} ${1}
> + echo "running: perf record $@"
> + ${perf} record --buildid-all -o ${data} $@ &> ${log}
> if [ $? -ne 0 ]; then
> - echo "failed: record ${1}"
> + echo "failed: record $@"
> + echo "see log: ${log}"
> exit 1
> fi
>
> - check ${1}
> + check ${@: -1}
>
> + rm -f ${log}
> rm -rf ${build_id_dir}
> rm -rf ${data}
> }
> @@ -96,12 +138,21 @@ test_record()
> # add binaries manual via perf buildid-cache -a
> test_add ${ex_sha1}
> test_add ${ex_md5}
> +if [ ${add_pe} -eq 1 ]; then
> + test_add ${ex_pe}
> +fi
>
> # add binaries via perf record post processing
> test_record ${ex_sha1}
> test_record ${ex_md5}
> +if [ ${run_pe} -eq 1 ]; then
> + test_record wine ${ex_pe}
> +fi
>
> # cleanup
> rm ${ex_sha1} ${ex_md5}
> +if [ ${run_pe} -eq 1 ]; then
> + rm -r ${wineprefix}
> +fi
>
> exit ${err}
> --
> 2.30.1
>
>
Em Thu, Feb 25, 2021 at 09:35:04PM +0100, Jiri Olsa escreveu:
> On Wed, Feb 24, 2021 at 02:59:16PM -0500, Nicholas Fraser wrote:
> > From 9fd0b3889f00ad13662879767d833309d8a035b6 Mon Sep 17 00:00:00 2001
> > From: Nicholas Fraser <[email protected]>
> > Date: Thu, 18 Feb 2021 13:24:03 -0500
> > Subject: [PATCH] perf buildid-cache: Add test for PE executable
> >
> > This builds on the previous changes to tests/shell/buildid.sh, adding
> > tests for a PE file. It adds it to the build-id cache manually and, if
> > Wine is available, runs it under "perf record" and verifies that it was
> > added automatically.
> >
> > If wine is not installed, only warnings are printed; the test can still
> > exit 0.
> >
> > Signed-off-by: Nicholas Fraser <[email protected]>
>
> works nicely now, thanks
>
> Acked-by: Jiri Olsa <[email protected]>
Thanks for checking it, but if you did a review, i.e. if you looked at
the code, made suggestions, the submitter acted upon those changes, you
looked again, etc, shouldn't this be a more appropriate:
Reviewed-by: Jiri Olsa <[email protected]>
?
I think we need to make these tags reflect more what really happened,
i.e. if you just glanced over and thought, quickly, that it seems
okayish, then Acked-by is what we should use, but if you gone thru the
trouble of actually _looking hard_ at it, sometimes multiple times, then
we should really use Reviewed-by and not take that lightly.
- Arnaldo
> jirka
>
> > ---
> > tools/perf/tests/shell/buildid.sh | 65 +++++++++++++++++++++++++++----
> > 1 file changed, 58 insertions(+), 7 deletions(-)
> >
> > diff --git a/tools/perf/tests/shell/buildid.sh b/tools/perf/tests/shell/buildid.sh
> > index 416af614bbe0..f05670d1e39e 100755
> > --- a/tools/perf/tests/shell/buildid.sh
> > +++ b/tools/perf/tests/shell/buildid.sh
> > @@ -14,18 +14,56 @@ if ! [ -x "$(command -v cc)" ]; then
> > exit 2
> > fi
> >
> > +# check what we need to test windows binaries
> > +add_pe=1
> > +run_pe=1
> > +if ! perf version --build-options | grep -q 'libbfd: .* on '; then
> > + echo "WARNING: perf not built with libbfd. PE binaries will not be tested."
> > + add_pe=0
> > + run_pe=0
> > +fi
> > +if ! which wine > /dev/null; then
> > + echo "WARNING: wine not found. PE binaries will not be run."
> > + run_pe=0
> > +fi
> > +
> > +# set up wine
> > +if [ ${run_pe} -eq 1 ]; then
> > + wineprefix=$(mktemp -d /tmp/perf.wineprefix.XXX)
> > + export WINEPREFIX=${wineprefix}
> > + # clear display variables to prevent wine from popping up dialogs
> > + unset DISPLAY
> > + unset WAYLAND_DISPLAY
> > +fi
> > +
> > ex_md5=$(mktemp /tmp/perf.ex.MD5.XXX)
> > ex_sha1=$(mktemp /tmp/perf.ex.SHA1.XXX)
> > +ex_pe=$(dirname $0)/../pe-file.exe
> >
> > echo 'int main(void) { return 0; }' | cc -Wl,--build-id=sha1 -o ${ex_sha1} -x c -
> > echo 'int main(void) { return 0; }' | cc -Wl,--build-id=md5 -o ${ex_md5} -x c -
> >
> > -echo "test binaries: ${ex_sha1} ${ex_md5}"
> > +echo "test binaries: ${ex_sha1} ${ex_md5} ${ex_pe}"
> >
> > check()
> > {
> > - id=`readelf -n ${1} 2>/dev/null | grep 'Build ID' | awk '{print $3}'`
> > -
> > + case $1 in
> > + *.exe)
> > + # We don't have a tool that can pull a nicely formatted build-id out of
> > + # a PE file, but we can extract the whole section with objcopy and
> > + # format it ourselves. The .buildid section is a Debug Directory
> > + # containing a CodeView entry:
> > + # https://docs.microsoft.com/en-us/windows/win32/debug/pe-format#debug-directory-image-only
> > + # https://github.com/dotnet/runtime/blob/da94c022576a5c3bbc0e896f006565905eb137f9/docs/design/specs/PE-COFF.md
> > + # The build-id starts at byte 33 and must be rearranged into a GUID.
> > + id=`objcopy -O binary --only-section=.buildid $1 /dev/stdout | \
> > + cut -c 33-48 | hexdump -ve '/1 "%02x"' | \
> > + sed 's@^\(..\)\(..\)\(..\)\(..\)\(..\)\(..\)\(..\)\(..\)\(.*\)0a$@\4\3\2\1\6\5\8\7\9@'`
> > + ;;
> > + *)
> > + id=`readelf -n ${1} 2>/dev/null | grep 'Build ID' | awk '{print $3}'`
> > + ;;
> > + esac
> > echo "build id: ${id}"
> >
> > link=${build_id_dir}/.build-id/${id:0:2}/${id:2}
> > @@ -50,7 +88,7 @@ check()
> > exit 1
> > fi
> >
> > - ${perf} buildid-cache -l | grep $id
> > + ${perf} buildid-cache -l | grep ${id}
> > if [ $? -ne 0 ]; then
> > echo "failed: ${id} is not reported by \"perf buildid-cache -l\""
> > exit 1
> > @@ -79,16 +117,20 @@ test_record()
> > {
> > data=$(mktemp /tmp/perf.data.XXX)
> > build_id_dir=$(mktemp -d /tmp/perf.debug.XXX)
> > + log=$(mktemp /tmp/perf.log.XXX)
> > perf="perf --buildid-dir ${build_id_dir}"
> >
> > - ${perf} record --buildid-all -o ${data} ${1}
> > + echo "running: perf record $@"
> > + ${perf} record --buildid-all -o ${data} $@ &> ${log}
> > if [ $? -ne 0 ]; then
> > - echo "failed: record ${1}"
> > + echo "failed: record $@"
> > + echo "see log: ${log}"
> > exit 1
> > fi
> >
> > - check ${1}
> > + check ${@: -1}
> >
> > + rm -f ${log}
> > rm -rf ${build_id_dir}
> > rm -rf ${data}
> > }
> > @@ -96,12 +138,21 @@ test_record()
> > # add binaries manual via perf buildid-cache -a
> > test_add ${ex_sha1}
> > test_add ${ex_md5}
> > +if [ ${add_pe} -eq 1 ]; then
> > + test_add ${ex_pe}
> > +fi
> >
> > # add binaries via perf record post processing
> > test_record ${ex_sha1}
> > test_record ${ex_md5}
> > +if [ ${run_pe} -eq 1 ]; then
> > + test_record wine ${ex_pe}
> > +fi
> >
> > # cleanup
> > rm ${ex_sha1} ${ex_md5}
> > +if [ ${run_pe} -eq 1 ]; then
> > + rm -r ${wineprefix}
> > +fi
> >
> > exit ${err}
> > --
> > 2.30.1
> >
> >
>
--
- Arnaldo
On Fri, Feb 26, 2021 at 08:47:36PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Thu, Feb 25, 2021 at 09:35:04PM +0100, Jiri Olsa escreveu:
> > On Wed, Feb 24, 2021 at 02:59:16PM -0500, Nicholas Fraser wrote:
> > > From 9fd0b3889f00ad13662879767d833309d8a035b6 Mon Sep 17 00:00:00 2001
> > > From: Nicholas Fraser <[email protected]>
> > > Date: Thu, 18 Feb 2021 13:24:03 -0500
> > > Subject: [PATCH] perf buildid-cache: Add test for PE executable
> > >
> > > This builds on the previous changes to tests/shell/buildid.sh, adding
> > > tests for a PE file. It adds it to the build-id cache manually and, if
> > > Wine is available, runs it under "perf record" and verifies that it was
> > > added automatically.
> > >
> > > If wine is not installed, only warnings are printed; the test can still
> > > exit 0.
> > >
> > > Signed-off-by: Nicholas Fraser <[email protected]>
> >
> > works nicely now, thanks
> >
> > Acked-by: Jiri Olsa <[email protected]>
>
> Thanks for checking it, but if you did a review, i.e. if you looked at
> the code, made suggestions, the submitter acted upon those changes, you
> looked again, etc, shouldn't this be a more appropriate:
>
> Reviewed-by: Jiri Olsa <[email protected]>
>
> ?
>
> I think we need to make these tags reflect more what really happened,
> i.e. if you just glanced over and thought, quickly, that it seems
> okayish, then Acked-by is what we should use, but if you gone thru the
> trouble of actually _looking hard_ at it, sometimes multiple times, then
> we should really use Reviewed-by and not take that lightly.
ah right, I slipped to using ack regardles the effort ;-)
I'll try to kick myself to use reviewed where appropriate
for this one:
Reviewed-by: Jiri Olsa <[email protected]>
thanks,
jirka
Em Sat, Feb 27, 2021 at 11:17:01AM +0100, Jiri Olsa escreveu:
> On Fri, Feb 26, 2021 at 08:47:36PM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Thu, Feb 25, 2021 at 09:35:04PM +0100, Jiri Olsa escreveu:
> > > On Wed, Feb 24, 2021 at 02:59:16PM -0500, Nicholas Fraser wrote:
> > > > From 9fd0b3889f00ad13662879767d833309d8a035b6 Mon Sep 17 00:00:00 2001
> > > > From: Nicholas Fraser <[email protected]>
> > > > Date: Thu, 18 Feb 2021 13:24:03 -0500
> > > > Subject: [PATCH] perf buildid-cache: Add test for PE executable
> > > >
> > > > This builds on the previous changes to tests/shell/buildid.sh, adding
> > > > tests for a PE file. It adds it to the build-id cache manually and, if
> > > > Wine is available, runs it under "perf record" and verifies that it was
> > > > added automatically.
> > > >
> > > > If wine is not installed, only warnings are printed; the test can still
> > > > exit 0.
> > > >
> > > > Signed-off-by: Nicholas Fraser <[email protected]>
> > >
> > > works nicely now, thanks
> > >
> > > Acked-by: Jiri Olsa <[email protected]>
> >
> > Thanks for checking it, but if you did a review, i.e. if you looked at
> > the code, made suggestions, the submitter acted upon those changes, you
> > looked again, etc, shouldn't this be a more appropriate:
> >
> > Reviewed-by: Jiri Olsa <[email protected]>
> >
> > ?
> >
> > I think we need to make these tags reflect more what really happened,
> > i.e. if you just glanced over and thought, quickly, that it seems
> > okayish, then Acked-by is what we should use, but if you gone thru the
> > trouble of actually _looking hard_ at it, sometimes multiple times, then
> > we should really use Reviewed-by and not take that lightly.
>
> ah right, I slipped to using ack regardles the effort ;-)
> I'll try to kick myself to use reviewed where appropriate
>
> for this one:
>
> Reviewed-by: Jiri Olsa <[email protected]>
Thanks, applied to perf/core.
- Arnaldo