Re: [Qemu-devel] [PATCH] qht: do not segfault when gathering stats from an uninitialized qht

From: Peter Maydell <peter.maydell@linaro.org>
To: "Emilio G. Cota" <cota@braap.org>
Cc: QEMU Developers <qemu-devel@nongnu.org>,
	Changlong Xie <xiecl.fnst@cn.fujitsu.com>,
	Richard Henderson <rth@twiddle.net>
Subject: Re: [Qemu-devel] [PATCH] qht: do not segfault when gathering stats from an uninitialized qht
Date: Sat, 23 Jul 2016 11:01:34 +0100	[thread overview]
Message-ID: <CAFEAcA_doX3DFP4Ce=nFAGPgQj=N3CA3B05BP1ViiZjcbWun=Q@mail.gmail.com> (raw)
In-Reply-To: <1469205390-14369-1-git-send-email-cota@braap.org>

On 22 July 2016 at 17:36, Emilio G. Cota <cota@braap.org> wrote:
> So far, QHT functions assume that the passed qht has previously been
> initialized--otherwise they segfault.
>
> This patch makes an exception for qht_statistics_init, with the goal
> of simplifying calling code. For instance, qht_statistics_init is
> called from the 'info jit' dump, and given that under KVM the TB qht
> is never initialized, we get a segfault. Thus, instead of complicating
> the 'info jit' code with additional checks, let's allow passing an
> uninitialized qht to qht_statistics_init.
>
> While at it, add a test for this to test-qht.
>
> Before the patch (for $ qemu -enable-kvm [...]):
> (qemu) info jit
> [...]
> direct jump count   0 (0%) (2 jumps=0 0%)
> Program received signal SIGSEGV, Segmentation fault.
>
> After the patch:
> (qemu) info jit
> [...]
> direct jump count   0 (0%) (2 jumps=0 0%)
> TB hash buckets     0/0 (-nan% head buckets used)
> TB hash occupancy   nan% avg chain occ. Histogram: (null)
> TB hash avg chain   nan buckets. Histogram: (null)

This looks like we're passing NULL pointers to
printf %s specifiers. This is undefined behaviour at least
for POSIX printf, and I can't see anything in the glib
printf-alike function documentation that gives an extra
guarantee for this, so it's probably a bad idea.

Printing 'nan' also looks a bit odd, though it's not UB.

thanks
-- PMM