Re: [Qemu-devel] [PATCH v2 2/7] iotests: exclude killed processes from running under Valgrind

[Roman Kagan]

On Mon, Jun 17, 2019 at 02:53:55PM +0200, Kevin Wolf wrote:
> Am 17.06.2019 um 14:18 hat Roman Kagan geschrieben:
> > On Mon, Jun 17, 2019 at 01:15:04PM +0200, Kevin Wolf wrote:
> > > Am 11.06.2019 um 20:02 hat Andrey Shinkevich geschrieben:
> > > > The Valgrind tool fails to manage its termination when QEMU raises the
> > > > signal SIGKILL. Lets exclude such test cases from running under the
> > > > Valgrind because there is no sense to check memory issues that way.
> > > > 
> > > > Signed-off-by: Andrey Shinkevich <address@hidden>
> > > 
> > > I don't fully understand the reasoning here. Most interesting memory
> > > access errors happen before a process terminates. (I'm not talking about
> > > leaks here, but use-after-free, buffer overflows, uninitialised memory
> > > etc.)
> > 
> > Nothing of the above, and nothing in general, happens in the usermode
> > process upon SIGKILL delivery.
> 
> My point is, the interesting part is what the program does before
> SIGKILL happens. There is value in reporting memory errors as long as we
> can, even if the final check doesn't happen because of SIGKILL.

Agreed in general, but here the testcases that include 'sigraise 9' only
do simple operations before that which are covered elsewhere too.  So
the extra effort on making valgrind work with these testcases arguably
isn't worth the extra value to be gained.

> > > However, I do see that running these test cases with -valgrind ends in a
> > > hang because the valgrind process keeps hanging around as a zombie
> > > process and the test case doesn't reap it. I'm not exactly sure why that
> > > is, but it looks more like a problem with the parent process (i.e. the
> > > bash script).
> > 
> > It rather looks like valgrind getting confused about what to do with
> > raise(SIGKILL) in the multithreaded case.
> 
> Well, valgrind can't do anything with SIGKILL, obviously, because it's
> killed immediately.

Right, but it can do whatever it wants with raise(SIGKILL).  I haven't
looked at valgrind sources, but

  # strace -ff valgind qemu-io -c 'sigraise 9'

shows SIGKILL neither sent nor received by any thread; it just shows the
main thread exit and the second thread getting stuck waiting on a futex.

> But maybe the kernel does get confused for some
> reason. I get the main threads as a zombie, but a second is still
> running. Sending SIGKILL to the second thread, too, makes the test case
> complete successfully.
> 
> So I guess the main question is why the second thread isn't
> automatically killed when the main thread receives SIGKILL.

I don't see any thread receive SIGKILL.  So I tend to think this is
valgrind's bug/feature.

Anyway the problem is outside of QEMU, so I think we need to weigh the
costs of investigating it and implementing a workaround with the
potential benefit.

Roman.