Re: [PATCH 2/2] perf buildid-cache: Add test for PE executable

[Jiri Olsa]

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
>