Re: [PATCH] Ignore random output from Asan tests in dg-cmp-results

[Thomas Preudhomme <thomas dot preudhomme at foss dot arm dot com>]

[adding Mike Stump who approved my last patch to dg-cmp-results]

Hi Yuri,

On 09/07/17 20:06, Yuri Gribov wrote:
> Hi,
>
> Currently dg-cmp-results fails to compare fails in Asan tests because
> their output includes failing process id:
> NA->FAIL: c-c++-common/asan/bitfield-1.c   -O0  output pattern test,
> is ==18161==ERROR
> FAIL->NA: c-c++-common/asan/bitfield-1.c   -O0  output pattern test,
> is ==26667==ERROR
>
> This patch fixes this. I also did some fairly standard refactoring to
> improve stability and get rid of code dup.

While I welcome the hardening of dg-cmp-results you made, I think this would be 
better split out in a separate patch. Note though that I'm not maintainer so 
feel free to ignore me.

Other comments inline.

> Ok for trunk?
>
> -Y
>
>
> dg-cmp-results-asan-fix-1.patch
>
>
> 2017-07-09  Yury Gribov<tetra2005@gmail.com>
>
> contrib/
>          * dg-cmp-results.sh: Ignore random output from Asan tests.
>
> diff -rupN gcc/contrib/dg-cmp-results.sh gcc-compare-checks/contrib/dg-cmp-results.sh
> --- gcc/contrib/dg-cmp-results.sh	2017-07-09 07:41:13.000000000 +0200
> +++ gcc-compare-checks/contrib/dg-cmp-results.sh	2017-07-09 18:58:52.000000000 +0200
> @@ -1,5 +1,5 @@
> -#!/bin/bash
> -# Copyright (C) 2006, 2008 Free Software Foundation
> +#!/bin/bash -eu
> +# Copyright (C) 2006, 2008, 2017 Free Software Foundation
>   #
>   # Analyze changes in GCC DejaGNU test logs for binutils, gcc, gdb, etc.
>   # Original version written in 2005 by James Lemke<jwlemke@wasabisystems.com>.
> @@ -89,25 +89,29 @@ sed $E -e '/^[[:space:]]+===/,$d' $OFILE
>   echo "Newer log file: $NFILE"
>   sed $E -e '/^[[:space:]]+===/,$d' $NFILE
>   
> -# Create a temporary file from the old file's interesting section.
> -sed $E -e "/$header/,/^[[:space:]]+===.*Summary ===/!d" \
> -  -e '/^[A-Z]+:/!d' \
> -  -e '/^(WARNING|ERROR):/d' \
> -  -e 's/\r$//' \
> -  -e 's/^/O:/' \
> -  $OFILE |
> -  sort -s -t : -k 3b - \
> -  >/tmp/o$$-$OBASE
> -
> -# Create a temporary file from the new file's interesting section.
> -sed $E -e "/$header/,/^[[:space:]]+===.*Summary ===/!d" \
> -  -e '/^[A-Z]+:/!d' \
> -  -e '/^(WARNING|ERROR):/d' \
> -  -e 's/\r$//' \
> -  -e 's/^/N:/' \
> -  $NFILE |
> -  sort -s -t : -k 3b - \
> -  >/tmp/n$$-$NBASE
> +O=/tmp/o$$-$OBASE
> +N=/tmp/n$$-$NBASE
> +
> +trap "rm -f compare-$$.awk /tmp/o$$-$OBASE /tmp/n$$-$NBASE" EXIT

Why not use $O and $N here?

> +
> +# Create a temporary file from file's interesting section.
> +get_interesting_lines() {
> +  sed $E -e "/$header/,/^[[:space:]]+===.*Summary ===/!d" \
> +      -e '/^[A-Z]+:/!d' \
> +      -e '/^(WARNING|ERROR):/d' \
> +      -e 's/\r$//' \
> +      -e 's/^/'$1':/' \
> +      | sort -s -t : -k 3b -
> +}
> +
> +# Preprocess lines to avoid spurious differences
> +canonize_lines() {
> +  # For now just strip pid numbers from Asan messages
> +  sed $E -e 's/(\/asan\/.* is ==)[0-9]+==/\1PID==/'
> +}

I'd suggest to use @ as a separator for sed which will allow you to avoid the 
escaping of '/', ie. sed $E -e 's@(/asan/.* is ==)[0-9]+==@\1PID==@'

> +
> +get_interesting_lines O < $OFILE | canonize_lines > $O
> +get_interesting_lines N < $NFILE | canonize_lines > $N
>   
>   # Merge the two files, then compare adjacent lines.
>   # Comparison is complicated by tests that may be run multiple times.
> @@ -202,8 +206,3 @@ END {
>   EOF
>   sort -m -s -t : -k 3b /tmp/o$$-$OBASE /tmp/n$$-$NBASE |
>    awk -v verbose=$verbose -f compare-$$.awk /dev/stdin
> -
> -# Delete the temporary files.
> -rm -f compare-$$.awk /tmp/o$$-$OBASE /tmp/n$$-$NBASE
> -
> -exit 0

LGTM otherwise, but as I said this is not an approval to commit.

Best regards,

Thomas