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

[Yuri Gribov <tetra2005 at gmail dot com>]

On Mon, Jul 10, 2017 at 1:13 PM, Thomas Preudhomme
<thomas.preudhomme@foss.arm.com> wrote:
> [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?

Ah, will do.

>> +
>> +# 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==@'

Right.

>> +
>> +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