Re: [PATCH 2/2] Add scripts/oss-fuzz/build.sh

[Julien Grall <julien@xxxxxxx>]

Hi Tamas,

On 24/06/2024 23:18, Tamas K Lengyel wrote:
> On Mon, Jun 24, 2024 at 5:58 PM Julien Grall <julien@xxxxxxx> wrote:
> > Hi,
> >
> > On 21/06/2024 20:14, Tamas K Lengyel wrote:
> > > The build integration script for oss-fuzz targets.
> > Do you have any details how this is meant and/or will be used?
> https://google.github.io/oss-fuzz/getting-started/new-project-guide/#buildsh
>
> > I also couldn't find a cover letter. For series with more than one
> > patch, it is recommended to have one as it help threading and could also
> > give some insight on what you are aiming to do.
> >
> > > Signed-off-by: Tamas K Lengyel <tamas@xxxxxxxxxxxxx>
> > > ---
> > >    scripts/oss-fuzz/build.sh | 22 ++++++++++++++++++++++
> > >    1 file changed, 22 insertions(+)
> > >    create mode 100755 scripts/oss-fuzz/build.sh
> > >
> > > diff --git a/scripts/oss-fuzz/build.sh b/scripts/oss-fuzz/build.sh
> > > new file mode 100755
> > > index 0000000000..48528bbfc2
> > > --- /dev/null
> > > +++ b/scripts/oss-fuzz/build.sh
> > Depending on the answer above, we may want to consider to create the
> > directory oss-fuzz under automation or maybe tools/fuzz/.
> I'm fine with moving it wherever.
What about tools/fuzz then? This is where are all the tooling for the 
fuzzing.
> > > @@ -0,0 +1,22 @@
> > > +#!/bin/bash -eu
> > > +# Copyright 2024 Google LLC
> > I am a bit confused with this copyright. Is this script taken from
> > somewhere?
> Yes, I took an existing build.sh from oss-fuzz,
It is unclear to me what is left from that "existing" build.sh. At least 
everything below seems to be Xen specific.
Anyway, if you want to give the copyright to Google then fair enough, 
but I think you want to use an Origin tag (or similar) to indicate the 
original copy.
>  it is recommended to
> have the more complex part of build.sh as part of the upstream
> repository so that additional targets/fixes can be merged there
> instead of opening PRs on oss-fuzz directly. With this setup the
> build.sh I merge to oss-fuzz will just just this build.sh in the Xen
> repository. See
> https://github.com/tklengyel/oss-fuzz/commit/552317ae9d24ef1c00d87595516cc364bc33b662.
>
> > > +#
> > > +# Licensed under the Apache License, Version 2.0 (the "License");
> > > +# you may not use this file except in compliance with the License.
> > > +# You may obtain a copy of the License at
> > > +#
> > > +#      http://www.apache.org/licenses/LICENSE-2.0
> > > +#
> > > +# Unless required by applicable law or agreed to in writing, software
> > > +# distributed under the License is distributed on an "AS IS" BASIS,
> > > +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> > > +# See the License for the specific language governing permissions and
> > > +# limitations under the License.
> > > +#
> > > +################################################################################
> > > +
> > > +cd xen
> > > +./configure clang=y --disable-stubdom --disable-pvshim --disable-docs 
> > > --disable-xen
> > Looking at the help from ./configure, 'clang=y' is not mentioned and it
> > doesn't make any difference in the config.log. Can you clarify why this
> > was added?
> Just throwing stuff at the wall till I was able to get a clang build.
> If it's indeed not needed I can remove it.
>
> > > +make clang=y -C tools/include
> > > +make clang=y -C tools/fuzz/x86_instruction_emulator libfuzzer-harness
> > > +cp tools/fuzz/x86_instruction_emulator/libfuzzer-harness 
> > > $OUT/x86_instruction_emulator
> > Who will be defining $OUT?
> oss-fuzz
Ok. Can you add a link to the documentation in build.sh? This would be 
helpful for the future reader to understand what's $OUT really mean.
Cheers,

--
Julien Grall