diff mbox series

gitlab-ci.yml: Only run one test-case per fuzzer

Message ID 20201002143524.56930-1-alxndr@bu.edu
State New
Headers show
Series gitlab-ci.yml: Only run one test-case per fuzzer | expand

Commit Message

Alexander Bulekov Oct. 2, 2020, 2:35 p.m. UTC 
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(-)

Comments

Philippe Mathieu-Daudé Oct. 2, 2020, 2:43 p.m. UTC | #1 
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>
Thomas Huth Oct. 2, 2020, 3:15 p.m. UTC | #2 
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>
Darren Kenny Oct. 2, 2020, 3:22 p.m. UTC | #3 
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
Philippe Mathieu-Daudé Oct. 2, 2020, 3:53 p.m. UTC | #4 
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>

>
Thomas Huth Oct. 2, 2020, 3:56 p.m. UTC | #5 
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
Alexander Bulekov Oct. 2, 2020, 6:47 p.m. UTC | #6 
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>
>
Thomas Huth Oct. 12, 2020, 10 a.m. UTC | #7 
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
diff mbox series

Patch

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