Re: [PATCH] hw/misc/npcm_clk: fix buffer-overflow

[Peter Maydell <peter.maydell@linaro.org>]

(edited cc list since it's moved away from a discussion of this
particular patch and on to a testing/ci coverage issue)

On Wed, 26 Feb 2025 at 19:03, Pierrick Bouvier
<pierrick.bouvier@linaro.org> wrote:
>
> On 2/26/25 03:50, Peter Maydell wrote:
> > On Tue, 25 Feb 2025 at 20:57, Pierrick Bouvier
> > <pierrick.bouvier@linaro.org> wrote:
> >>
> >> On 2/25/25 05:41, Peter Maydell wrote:
> >>> (Looking more closely at the cold_reset_values handling
> >>> in npcm_gcr.c, that looks not quite right in a different
> >>> way; I'll send a reply to that patch email about that.)
> >>>
> >>
> >> It may be a hole in our CI right now.
> >> Would that be interesting for CI to run all tests (check-functional +
> >> check w/o functional) with both ubsan and asan?
> >
> > We do have at least some ubsan tests in our CI right now
> > (eg the "clang-system" job). The problem with ubsan coverage
> > is the usual one that we already have too much CI going on,
> > and it takes forever and we don't have that much headroom
> > for adding more jobs.
>
> I understand the problem behind spending more minutes on this.
>
> However, looking at our CI, we already duplicate functional testing a lot:
> buildtest.yml:functional-system-alpine:
> buildtest.yml:functional-system-ubuntu:
> buildtest.yml:functional-system-debian:
> buildtest.yml:functional-system-fedora:
> buildtest.yml:functional-system-centos:
> buildtest.yml:functional-system-opensuse:

I think that these are mostly testing different target
architectures, e.g. functional-system-alpine tests what
build-system-alpine built, which covers avr, loongarch64,
mips64 and mipsel targets; functional-system-ubuntu
tests what build-system-ubuntu bulit, which is alpha,
microblazeel, mips64el, and so on. So there is less overlap
than it might appear.

(Some of them complete pretty quickly because we have very few
functional tests for some archs; some are slower where we're
running more tests. e.g.
https://gitlab.com/qemu-project/qemu/-/jobs/9213571833
is functional-system-fedora completing in 7 mins because we
only have a few ppc tests, but this is functional-system-opensuse
https://gitlab.com/qemu-project/qemu/-/jobs/9213571852
taking 27 mins because it's testing x86 and aarch64. The
corresponding build jobs take about 30 mins each.)

> Would that hurt so much to have one configuration enabled with ubsan and
> asan, which catches *real* bugs, and potential security issues?
> Yes, it adds overhead, but it should not be x10. Around x2 to x3.

You'd need to have a duplicate of all of the above
functional-system-* test jobs if you wanted
to test all the guest architectures, I think. So it's
30 mins build * six configs plus 60 mins total for testing.
Or we could convert (some of?) the existing jobs to use the
sanitisers if we needed to economise on CI time.

> I guess CI minutes are cheaper than engineer ones those days.

You could make an argument that it's the other way around:
from the project's point of view engineer minutes are cheap
because we never pay engineers, whereas CI minutes are
expensive because we must pay for them out of our project
donations :-)

-- PMM