Re: [PATCH] perf buildid-cache: Add test for PE executable
From: Arnaldo Carvalho de Melo
Date: Fri Feb 26 2021 - 18:48:25 EST
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 <nfraser@xxxxxxxxxxxxxxx>
>
> Date: 2021年2月18日 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 <nfraser@xxxxxxxxxxxxxxx>
>
>
works nicely now, thanks
>
>
Acked-by: Jiri Olsa <jolsa@xxxxxxxxxx>
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 <jolsa@xxxxxxxxxx>
?
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