* Re: [PATCH] gitlab-ci.yml: Only run one test-case per fuzzer
2020-10-02 14:35 [PATCH] gitlab-ci.yml: Only run one test-case per fuzzer Alexander Bulekov
@ 2020-10-02 14:43 ` Philippe Mathieu-Daudé
2020-10-02 15:15 ` Thomas Huth
` (2 subsequent siblings)
3 siblings, 0 replies; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-10-02 14:43 UTC (permalink / raw)
To: Alexander Bulekov, qemu-devel
Cc: qemu-trivial, Thomas Huth, Alex Bennée,
Wainer dos Santos Moschetta
On 10/2/20 4:35 PM, Alexander Bulekov wrote:
> With 1000 runs, there is a non-negligible chance that the fuzzer can
> trigger a crash. With this CI job, we care about catching build/runtime
> issues in the core fuzzing code. Actual device fuzzing takes place on
> oss-fuzz. For these purposes, only running one input should be
> sufficient.
>
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
> Suggested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> .gitlab-ci.yml | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> index a51c89554f..075c15d45c 100644
> --- a/.gitlab-ci.yml
> +++ b/.gitlab-ci.yml
> @@ -303,7 +303,7 @@ build-oss-fuzz:
> | grep -v slirp); do
> grep "LLVMFuzzerTestOneInput" ${fuzzer} > /dev/null 2>&1 || continue ;
> echo Testing ${fuzzer} ... ;
> - "${fuzzer}" -runs=1000 -seed=1 || exit 1 ;
> + "${fuzzer}" -runs=1 -seed=1 || exit 1 ;
> done
> # Unrelated to fuzzer: run some tests with -fsanitize=address
> - cd build-oss-fuzz && make check-qtest-i386 check-unit
>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] gitlab-ci.yml: Only run one test-case per fuzzer
2020-10-02 14:35 [PATCH] gitlab-ci.yml: Only run one test-case per fuzzer Alexander Bulekov
2020-10-02 14:43 ` Philippe Mathieu-Daudé
@ 2020-10-02 15:15 ` Thomas Huth
2020-10-02 15:53 ` Philippe Mathieu-Daudé
2020-10-02 18:47 ` Alexander Bulekov
2020-10-02 15:22 ` Darren Kenny
2020-10-12 10:00 ` Thomas Huth
3 siblings, 2 replies; 8+ messages in thread
From: Thomas Huth @ 2020-10-02 15:15 UTC (permalink / raw)
To: Alexander Bulekov, qemu-devel
Cc: qemu-trivial, Alex Bennée, Philippe Mathieu-Daudé,
Wainer dos Santos Moschetta
On 02/10/2020 16.35, Alexander Bulekov wrote:
> With 1000 runs, there is a non-negligible chance that the fuzzer can
> trigger a crash. With this CI job, we care about catching build/runtime
> issues in the core fuzzing code. Actual device fuzzing takes place on
> oss-fuzz. For these purposes, only running one input should be
> sufficient.
>
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
> Suggested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> .gitlab-ci.yml | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> index a51c89554f..075c15d45c 100644
> --- a/.gitlab-ci.yml
> +++ b/.gitlab-ci.yml
> @@ -303,7 +303,7 @@ build-oss-fuzz:
> | grep -v slirp); do
> grep "LLVMFuzzerTestOneInput" ${fuzzer} > /dev/null 2>&1 || continue ;
> echo Testing ${fuzzer} ... ;
> - "${fuzzer}" -runs=1000 -seed=1 || exit 1 ;
> + "${fuzzer}" -runs=1 -seed=1 || exit 1 ;
... but we're apparently already using a fixed seed for running the
test, so it should be pretty much deterministic, shouldn't it? So the
chance that the fuzzer hits a crash here for a pre-existing problem
should be close to zero? ... so I'm not quite sure whether we really
need this? Anyway, I certainly also won't object this patch, so in case
anybody wants to merge it:
Acked-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] gitlab-ci.yml: Only run one test-case per fuzzer
2020-10-02 15:15 ` Thomas Huth
@ 2020-10-02 15:53 ` Philippe Mathieu-Daudé
2020-10-02 15:56 ` Thomas Huth
2020-10-02 18:47 ` Alexander Bulekov
1 sibling, 1 reply; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-10-02 15:53 UTC (permalink / raw)
To: Thomas Huth, Alexander Bulekov, qemu-devel
Cc: qemu-trivial, Alex Bennée, Wainer dos Santos Moschetta
On 10/2/20 5:15 PM, Thomas Huth wrote:
> On 02/10/2020 16.35, Alexander Bulekov wrote:
>> With 1000 runs, there is a non-negligible chance that the fuzzer can
>> trigger a crash. With this CI job, we care about catching build/runtime
>> issues in the core fuzzing code. Actual device fuzzing takes place on
>> oss-fuzz. For these purposes, only running one input should be
>> sufficient.
>>
>> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
>> Suggested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>> .gitlab-ci.yml | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
>> index a51c89554f..075c15d45c 100644
>> --- a/.gitlab-ci.yml
>> +++ b/.gitlab-ci.yml
>> @@ -303,7 +303,7 @@ build-oss-fuzz:
>> | grep -v slirp); do
>> grep "LLVMFuzzerTestOneInput" ${fuzzer} > /dev/null 2>&1 || continue ;
>> echo Testing ${fuzzer} ... ;
>> - "${fuzzer}" -runs=1000 -seed=1 || exit 1 ;
>> + "${fuzzer}" -runs=1 -seed=1 || exit 1 ;
>
> ... but we're apparently already using a fixed seed for running the
> test, so it should be pretty much deterministic, shouldn't it? So the
> chance that the fuzzer hits a crash here for a pre-existing problem
> should be close to zero? ... so I'm not quite sure whether we really
> need this?
You are right, "non-negligible chance that the fuzzer can trigger a
crash" shouldn't be a problem. What matters is we don't waste CI
resources, 1 run is enough to test the fuzzer is working.
> Anyway, I certainly also won't object this patch, so in case
> anybody wants to merge it:
>
> Acked-by: Thomas Huth <thuth@redhat.com>
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] gitlab-ci.yml: Only run one test-case per fuzzer
2020-10-02 15:53 ` Philippe Mathieu-Daudé
@ 2020-10-02 15:56 ` Thomas Huth
0 siblings, 0 replies; 8+ messages in thread
From: Thomas Huth @ 2020-10-02 15:56 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, Alexander Bulekov, qemu-devel
Cc: qemu-trivial, Alex Bennée, Wainer dos Santos Moschetta
On 02/10/2020 17.53, Philippe Mathieu-Daudé wrote:
> On 10/2/20 5:15 PM, Thomas Huth wrote:
>> On 02/10/2020 16.35, Alexander Bulekov wrote:
>>> With 1000 runs, there is a non-negligible chance that the fuzzer can
>>> trigger a crash. With this CI job, we care about catching build/runtime
>>> issues in the core fuzzing code. Actual device fuzzing takes place on
>>> oss-fuzz. For these purposes, only running one input should be
>>> sufficient.
>>>
>>> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
>>> Suggested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>> ---
>>> .gitlab-ci.yml | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
>>> index a51c89554f..075c15d45c 100644
>>> --- a/.gitlab-ci.yml
>>> +++ b/.gitlab-ci.yml
>>> @@ -303,7 +303,7 @@ build-oss-fuzz:
>>> | grep -v slirp); do
>>> grep "LLVMFuzzerTestOneInput" ${fuzzer} > /dev/null 2>&1 || continue ;
>>> echo Testing ${fuzzer} ... ;
>>> - "${fuzzer}" -runs=1000 -seed=1 || exit 1 ;
>>> + "${fuzzer}" -runs=1 -seed=1 || exit 1 ;
>>
>> ... but we're apparently already using a fixed seed for running the
>> test, so it should be pretty much deterministic, shouldn't it? So the
>> chance that the fuzzer hits a crash here for a pre-existing problem
>> should be close to zero? ... so I'm not quite sure whether we really
>> need this?
>
> You are right, "non-negligible chance that the fuzzer can trigger a
> crash" shouldn't be a problem. What matters is we don't waste CI
> resources, 1 run is enough to test the fuzzer is working.
Ok, considering that gitlab is currently thinking about limiting the
free CI minutes, that's a valid reason, indeed.
Thomas
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] gitlab-ci.yml: Only run one test-case per fuzzer
2020-10-02 15:15 ` Thomas Huth
2020-10-02 15:53 ` Philippe Mathieu-Daudé
@ 2020-10-02 18:47 ` Alexander Bulekov
1 sibling, 0 replies; 8+ messages in thread
From: Alexander Bulekov @ 2020-10-02 18:47 UTC (permalink / raw)
To: Thomas Huth
Cc: qemu-trivial, Alex Bennée, Philippe Mathieu-Daudé,
qemu-devel, Wainer dos Santos Moschetta
On 201002 1715, Thomas Huth wrote:
> On 02/10/2020 16.35, Alexander Bulekov wrote:
> > With 1000 runs, there is a non-negligible chance that the fuzzer can
> > trigger a crash. With this CI job, we care about catching build/runtime
> > issues in the core fuzzing code. Actual device fuzzing takes place on
> > oss-fuzz. For these purposes, only running one input should be
> > sufficient.
> >
> > Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
> > Suggested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> > ---
> > .gitlab-ci.yml | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> > index a51c89554f..075c15d45c 100644
> > --- a/.gitlab-ci.yml
> > +++ b/.gitlab-ci.yml
> > @@ -303,7 +303,7 @@ build-oss-fuzz:
> > | grep -v slirp); do
> > grep "LLVMFuzzerTestOneInput" ${fuzzer} > /dev/null 2>&1 || continue ;
> > echo Testing ${fuzzer} ... ;
> > - "${fuzzer}" -runs=1000 -seed=1 || exit 1 ;
> > + "${fuzzer}" -runs=1 -seed=1 || exit 1 ;
>
> ... but we're apparently already using a fixed seed for running the
> test, so it should be pretty much deterministic, shouldn't it? So the
> chance that the fuzzer hits a crash here for a pre-existing problem
> should be close to zero? ... so I'm not quite sure whether we really
> need this? Anyway, I certainly also won't object this patch, so in case
> anybody wants to merge it:
In addition to using an RNG+seed, libfuzzer also uses coverage
information to guide mutations. My guess is that as QEMU changes, this
coverage can change as well, so I wouldn't assume that using the same
seed will result in the same inputs generated, in the longer term.
Its true that the main benefit will probably be a few minutes shaved off
the 400 minute limit...
Thanks
-Alex
>
> Acked-by: Thomas Huth <thuth@redhat.com>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] gitlab-ci.yml: Only run one test-case per fuzzer
2020-10-02 14:35 [PATCH] gitlab-ci.yml: Only run one test-case per fuzzer Alexander Bulekov
2020-10-02 14:43 ` Philippe Mathieu-Daudé
2020-10-02 15:15 ` Thomas Huth
@ 2020-10-02 15:22 ` Darren Kenny
2020-10-12 10:00 ` Thomas Huth
3 siblings, 0 replies; 8+ messages in thread
From: Darren Kenny @ 2020-10-02 15:22 UTC (permalink / raw)
To: Alexander Bulekov, qemu-devel
Cc: Thomas Huth, qemu-trivial, Philippe Mathieu-Daudé,
Wainer dos Santos Moschetta, Alexander Bulekov, Alex Bennée
On Friday, 2020-10-02 at 10:35:24 -04, Alexander Bulekov wrote:
> With 1000 runs, there is a non-negligible chance that the fuzzer can
> trigger a crash. With this CI job, we care about catching build/runtime
> issues in the core fuzzing code. Actual device fuzzing takes place on
> oss-fuzz. For these purposes, only running one input should be
> sufficient.
>
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
> Suggested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
> ---
> .gitlab-ci.yml | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> index a51c89554f..075c15d45c 100644
> --- a/.gitlab-ci.yml
> +++ b/.gitlab-ci.yml
> @@ -303,7 +303,7 @@ build-oss-fuzz:
> | grep -v slirp); do
> grep "LLVMFuzzerTestOneInput" ${fuzzer} > /dev/null 2>&1 || continue ;
> echo Testing ${fuzzer} ... ;
> - "${fuzzer}" -runs=1000 -seed=1 || exit 1 ;
> + "${fuzzer}" -runs=1 -seed=1 || exit 1 ;
> done
> # Unrelated to fuzzer: run some tests with -fsanitize=address
> - cd build-oss-fuzz && make check-qtest-i386 check-unit
> --
> 2.28.0
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] gitlab-ci.yml: Only run one test-case per fuzzer
2020-10-02 14:35 [PATCH] gitlab-ci.yml: Only run one test-case per fuzzer Alexander Bulekov
` (2 preceding siblings ...)
2020-10-02 15:22 ` Darren Kenny
@ 2020-10-12 10:00 ` Thomas Huth
3 siblings, 0 replies; 8+ messages in thread
From: Thomas Huth @ 2020-10-12 10:00 UTC (permalink / raw)
To: Alexander Bulekov, qemu-devel
Cc: qemu-trivial, Philippe Mathieu-Daudé, Alex Bennée,
Wainer dos Santos Moschetta
On 02/10/2020 16.35, Alexander Bulekov wrote:
> With 1000 runs, there is a non-negligible chance that the fuzzer can
> trigger a crash. With this CI job, we care about catching build/runtime
> issues in the core fuzzing code. Actual device fuzzing takes place on
> oss-fuzz. For these purposes, only running one input should be
> sufficient.
>
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
> Suggested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> .gitlab-ci.yml | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> index a51c89554f..075c15d45c 100644
> --- a/.gitlab-ci.yml
> +++ b/.gitlab-ci.yml
> @@ -303,7 +303,7 @@ build-oss-fuzz:
> | grep -v slirp); do
> grep "LLVMFuzzerTestOneInput" ${fuzzer} > /dev/null 2>&1 || continue ;
> echo Testing ${fuzzer} ... ;
> - "${fuzzer}" -runs=1000 -seed=1 || exit 1 ;
> + "${fuzzer}" -runs=1 -seed=1 || exit 1 ;
> done
> # Unrelated to fuzzer: run some tests with -fsanitize=address
> - cd build-oss-fuzz && make check-qtest-i386 check-unit
Thanks, queued to:
https://gitlab.com/huth/qemu/-/commits/qtest-next/
Thomas
^ permalink raw reply [flat|nested] 8+ messages in thread