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

[Tamas K Lengyel <tamas@xxxxxxxxxxxxx>]

On Wed, Jun 26, 2024 at 8:41 AM Julien Grall <julien@xxxxxxx> wrote:
>
> 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.

Sure, it turns out there is already a README.oss-fuzz in tools/fuzz
that points to the oss-fuzz so I don't think there is anything else
needed here, we can just move the build script there.

Tamas