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.
(I welcome ways to make the GUID parsing less awful. checkpatch.pl is
complaining about the line length of the sed command. The re-arranging
could be done via e.g. id=${id:6:2}{id:4:2}... since this style is
already used in the script but that turns out to be longer than the sed
command and anyway it's bash-specific. This uses a hardcoded .exe so we
could also just hardcode its GUID but I'd worry about making the tests
too inflexible.)
Signed-off-by: Nicholas Fraser <[email protected]>
---
tools/perf/tests/shell/buildid.sh | 43 +++++++++++++++++++++++++++----
1 file changed, 38 insertions(+), 5 deletions(-)
diff --git a/tools/perf/tests/shell/buildid.sh b/tools/perf/tests/shell/buildid.sh
index 416af614bbe0..55e2168ef26f 100755
--- a/tools/perf/tests/shell/buildid.sh
+++ b/tools/perf/tests/shell/buildid.sh
@@ -14,18 +14,41 @@ 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
+
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)
+ # the build id 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 +73,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
@@ -81,7 +104,7 @@ test_record()
build_id_dir=$(mktemp -d /tmp/perf.debug.XXX)
perf="perf --buildid-dir ${build_id_dir}"
- ${perf} record --buildid-all -o ${data} ${1}
+ ${perf} record --buildid-all -o ${data} ${2} ${1}
if [ $? -ne 0 ]; then
echo "failed: record ${1}"
exit 1
@@ -96,12 +119,22 @@ 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 ${ex_pe} wine
+fi
# cleanup
rm ${ex_sha1} ${ex_md5}
+if [ $add_pe -eq 0 ] || [ $run_pe -eq 0 ]; then
+ echo "WARNING: some PE tests were skipped. See previous warnings."
+fi
+
exit ${err}
--
2.30.1
On 2021-02-24 8:43 a.m., Jiri Olsa wrote:
> On Fri, Feb 19, 2021 at 11:10:34AM -0500, Nicholas Fraser wrote:
>> + # the build id 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@'`
>> + ;;
>
> wow ;-) could this have some more info on what's going on in here?
> what's the .buildid PE section format?
>
The .buildid section is in the PE debug directory format. This has a 28-byte
header:
https://docs.microsoft.com/en-us/windows/win32/debug/pe-format#the-debug-section
This contains a CodeView entry with a GUID only (no path). The GUID is 4 bytes in:
https://github.com/dotnet/runtime/blob/master/docs/design/specs/PE-COFF.md#codeview-debug-directory-entry-type-2
This means the GUID starts at byte 32 (or 33 since cut uses 1-based indexing.)
The snippet of code extracts the .buildid section with objcopy, cuts bytes
33-48, converts them to hex, and re-arranges the bytes to form a GUID (the
first three fields of a GUID are little-endian.)
Technically, bytes 20-24 contain a pointer to the data, which could be
anywhere. In practice the format of this section is fixed in order to support
reproducible builds (so the TimeDateStamp for example is always zero.) This is
something created by the MinGW (and Cygwin?) compilers only; as far as I know
it's not created by MSVC tools. So I don't think we need to do any more
complicated parsing than just pulling out those bytes.
Of course if there is a tool that can pull out the build-id directly we should
use that instead. I don't know of any at the moment though.
I'll supply another patch to fix the other issues here. I'll add a couple more
comments and a better commit message as well.
>
> I'm getting lot of wine's output, we should redirect that
No problem, I can redirect the output. I left it in because if the output is
hidden it's not clear when the command hangs or fails. I could redirect it to a
temp file instead and leave the temp file in case of failure.
>
> every other run I'm getting some small window popup saying it's
> updating wine and stuck forever.. could this be prevented?
>
Hmm. It's possible it's stuck behind a GUI prompt. For example if you don't
have wine-gecko installed it might waiting on a dialog asking to install it.
I'll unset DISPLAY so it can't pop up any dialogs; this should make it fully
non-interactive.
Other than that, it could take some time to set up the wine prefix (the
location where wine stores its runtime configuration) and it might have
something broken in the prefix, for example if it was interrupted while setting
it up. I'll change it to use a temporary wine prefix instead. This means it
will have to do the initialization on every run but it should be more reliable.
On Fri, Feb 19, 2021 at 11:10:34AM -0500, Nicholas Fraser wrote:
SNIP
> +if ! which wine > /dev/null; then
> + echo "WARNING: wine not found. PE binaries will not be run."
> + run_pe=0
> +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)
> + # the build id 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@'`
> + ;;
wow ;-) could this have some more info on what's going on in here?
what's the .buildid PE section format?
> + *)
> + 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 +73,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
> @@ -81,7 +104,7 @@ test_record()
> build_id_dir=$(mktemp -d /tmp/perf.debug.XXX)
> perf="perf --buildid-dir ${build_id_dir}"
>
> - ${perf} record --buildid-all -o ${data} ${1}
> + ${perf} record --buildid-all -o ${data} ${2} ${1}
it could be better just pass $@ and make sure test_record
args are passed in a way that record would accept them
test_record wine ${ex_pe}
> if [ $? -ne 0 ]; then
> echo "failed: record ${1}"
> exit 1
> @@ -96,12 +119,22 @@ test_record()
> # add binaries manual via perf buildid-cache -a
> test_add ${ex_sha1}
> test_add ${ex_md5}
> +if [ $add_pe -eq 1 ]; then
${add_pe}
> + 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
${run_pe}
> + test_record ${ex_pe} wine
I'm getting lot of wine's output, we should redirect that
every other run I'm getting some small window popup saying it's
updating wine and stuck forever.. could this be prevented?
> +fi
>
> # cleanup
> rm ${ex_sha1} ${ex_md5}
>
> +if [ $add_pe -eq 0 ] || [ $run_pe -eq 0 ]; then
> + echo "WARNING: some PE tests were skipped. See previous warnings."
> +fi
there's already a warning for this at the beginning,
I dont think we need another one
thanks,
jirka
> +
> exit ${err}
> --
> 2.30.1
>