Re: [PULL 00/34] s390x update

[Cornelia Huck]

On Fri, 20 Sep 2019 13:59:12 +0200
David Hildenbrand <address@hidden> wrote:

> On 20.09.19 13:51, David Hildenbrand wrote:
> > On 20.09.19 13:00, Cornelia Huck wrote:  
> >> On Fri, 20 Sep 2019 11:45:18 +0100
> >> Peter Maydell <address@hidden> wrote:
> >>  
> >>> On Thu, 19 Sep 2019 at 13:41, Cornelia Huck <address@hidden> wrote:  
> >>>>
> >>>> The following changes since commit 
> >>>> f8c3db33a5e863291182f8862ddf81618a7c6194:
> >>>>
> >>>>   target/sparc: Switch to do_transaction_failed() hook (2019-09-17 
> >>>> 12:01:00 +0100)
> >>>>
> >>>> are available in the Git repository at:
> >>>>
> >>>>   https://github.com/cohuck/qemu tags/s390x-20190919
> >>>>
> >>>> for you to fetch changes up to 37105adebeb28e60da3cb1ef82231d7ed8d23589:
> >>>>
> >>>>   Merge tag 'tags/s390-ccw-bios-2019-09-18' into s390-next-staging 
> >>>> (2019-09-19 12:04:01 +0200)
> >>>>
> >>>> ----------------------------------------------------------------
> >>>> - bugfixes in tcg and the ccw bios
> >>>> - gen15a is called z15
> >>>> - officially require a 3.15 kernel or later for kvm
> >>>>
> >>>> ----------------------------------------------------------------    
> >>>
> >>> Hi -- I'm afraid this pullreq results in new warnings from
> >>> the runtime-sanitizer build when 'make check' is run:
> >>> MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
> >>> QTEST_QEMU_BINARY=s390x-softmmu/qemu-system-s390x
> >>> QTEST_QEMU_IMG=qemu-img tests
> >>> /boot-serial-test -m=quick -k --tap < /dev/null |
> >>> ./scripts/tap-driver.pl --test-name="boot-serial-test"
> >>> /home/petmay01/linaro/qemu-for-merges/target/s390x/mem_helper.c:293:17:
> >>> runtime error: null pointer passed as argument 1, which is declared to
> >>> never be null
> >>> /usr/include/string.h:47:14: note: nonnull attribute specified here
> >>> /home/petmay01/linaro/qemu-for-merges/target/s390x/mem_helper.c:293:32:
> >>> runtime error: null pointer passed as argument 2, which is declared to
> >>> never be null
> >>>
> >>> (and the same warnings for a few other tests).
> >>>
> >>> Looks like you sometimes can pass NULL pointers to the source
> >>> and destination of memmove(). This isn't permitted by the
> >>> standard even in the case where the size argument is zero.
> >>>
> >>> thanks
> >>> -- PMM  
> >>
> >> David, can you take a look?
> >>  
> > 
> > I would have assumed these pointers are ignored in case the length is
> > zero, too (the only way this can happen). Easy to fix.
> >   
> 
> 
> diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
> index e50cec9263..ef8e0c20a7 100644
> --- a/target/s390x/mem_helper.c
> +++ b/target/s390x/mem_helper.c
> @@ -290,17 +290,23 @@ static void access_memmove(CPUS390XState *env, 
> S390Access *desta,
>  
>      if (srca->size1 == desta->size1) {
>          memmove(desta->haddr1, srca->haddr1, srca->size1);
> -        memmove(desta->haddr2, srca->haddr2, srca->size2);
> +        if (likely(srca->size2)) {
> +            memmove(desta->haddr2, srca->haddr2, srca->size2);
> +        }
>      } else if (srca->size1 < desta->size1) {
>          diff = desta->size1 - srca->size1;
>          memmove(desta->haddr1, srca->haddr1, srca->size1);
>          memmove(desta->haddr1 + srca->size1, srca->haddr2, diff);
> -        memmove(desta->haddr2, srca->haddr2 + diff, desta->size2);
> +        if (likely(desta->size2)) {
> +            memmove(desta->haddr2, srca->haddr2 + diff, desta->size2);
> +        }
>      } else {
>          diff = srca->size1 - desta->size1;
>          memmove(desta->haddr1, srca->haddr1, desta->size1);
>          memmove(desta->haddr2, srca->haddr1 + desta->size1, diff);
> -        memmove(desta->haddr2 + diff, srca->haddr2, srca->size2);
> +        if (likely(srca->size2)) {
> +            memmove(desta->haddr2 + diff, srca->haddr2, srca->size2);
> +        }
>      }
>  }
> 
> For "s390x/tcg: Fault-safe memmove" should do the trick. Will test.

Ok, great.

Peter, FYI: I'll be on vacation for two weeks (starting later today),
so David/Thomas/Christian will probably handle any s390x-related things
including pull requests for that time (I don't think I want to put a v2
together in a hurry...)