Re: [PATCH 4/4] check-block: Enable iotests with SafeStack

[Daniele Buono <dbuono@linux.vnet.ibm.com>]

On 5/21/2020 5:59 AM, Stefan Hajnoczi wrote:
> On Wed, Apr 29, 2020 at 03:44:20PM -0400, Daniele Buono wrote:
>> SafeStack is a stack protection technique implemented in llvm. It is
>> enabled with a -fsanitize flag.
>> iotests are currently disabled when any -fsanitize option is used.
>> Since SafeStack is useful on production environments, and its
>> implementation may break the binary, filter it out when the check is
>> performed, so that if SafeStack was the only -fsanitize option, iotests
>> are still performed.
> 
> I can't parse this sentence. What does "its implementation may break the
> binary" mean? Do you mean it's worth running tests with SafeStack
> enabled because it exposes failures that go unnoticed without SafeStack?

What I meant is that, without proper changes, SafeStack breaks 
co-routines. Since they are heavily used in the io subsystem, this is 
probably the best class of tests to make sure co-routines are working 
fine with SafeStack.

I initially re-enabled the iotests for my internal testing.

Since I wasn't seeing any issue, I thought it would be useful to keep 
running this to make sure future implementations of SafeStack won't 
break co-routines again.

> 
>>
>> Signed-off-by: Daniele Buono <dbuono@linux.vnet.ibm.com>
>> ---
>>   tests/check-block.sh | 12 +++++++++++-
>>   1 file changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/check-block.sh b/tests/check-block.sh
>> index ad320c21ba..8e29c868e5 100755
>> --- a/tests/check-block.sh
>> +++ b/tests/check-block.sh
>> @@ -21,7 +21,17 @@ if grep -q "CONFIG_GPROF=y" config-host.mak 2>/dev/null ; then
>>       exit 0
>>   fi
>>   
>> -if grep -q "CFLAGS.*-fsanitize" config-host.mak 2>/dev/null ; then
>> +# Disable tests with any sanitizer except for SafeStack
>> +CFLAGS=$( grep "CFLAGS.*-fsanitize" config-host.mak 2>/dev/null )
>> +SANITIZE_FLAGS=""
>> +#Remove all occurrencies of -fsanitize=safe-stack
>> +for i in ${CFLAGS}; do
>> +        if [ "${i}" != "-fsanitize=safe-stack" ]; then
>> +                SANITIZE_FLAGS="${SANITIZE_FLAGS} ${i}"
>> +        fi
>> +done
>> +if echo ${SANITIZE_FLAGS} | grep -q "\-fsanitize" 2>/dev/null; then
>> +    # Have a sanitize flag that is not allowed, stop
>>       echo "Sanitizers are enabled ==> Not running the qemu-iotests."
>>       exit 0
>>   fi
> 
> The commit that disabled check-block.sh with sanitizers said:
> 
>    The sanitizers (especially the address sanitizer from Clang) are
>    sometimes printing out warnings or false positives - this spoils
>    the output of the iotests, causing some of the tests to fail.
> 
> It seems fine to allow SafeStack if check-block.sh currently passes with
> it enabled. Does it pass and produce no extra output?
> 
Yes, that was the idea. SafeStack should pass the tests without extra 
output.

It did (pass) on my testing machine. However I don't remember if I did 
the full (slow) check or only the partial one.

Will check again before I submit v2
> Stefan
>