Re: [Qemu-devel] [PATCH 0/9] valgrind/i386/s390x: memcheck false positives

[Paolo Bonzini]

On 30/10/2014 14:20, Christian Borntraeger wrote:
> Am 30.10.2014 14:03, schrieb Paolo Bonzini:
>> On 10/30/2014 10:36 AM, Christian Borntraeger wrote:
>>> Some of these things could
>>> also be fixed in valgrind, but it will take a while until these changes
>>> hit a release or distros.
>>
>> Ok, it's sensible to have it fixed in QEMU if it's temporary.  Which
>> could not be fixed in valgrind?
> 
> This is a tricky question. A typical annotation in valgrind for an more 
> complex ioctl looks like
> 
>    case VKI_SIOCGMIIREG:         /* get hardware entry registers */
>       PRE_MEM_RASCIIZ( "ioctl(SIOCGIFMIIREG)",
>                      (Addr)((struct vki_ifreq *)ARG3)->vki_ifr_name );
>       PRE_MEM_READ( "ioctl(SIOCGIFMIIREG)",
>                      (Addr)&((struct vki_mii_ioctl_data *)&((struct vki_ifreq 
> *)ARG3)->vki_ifr_data)->phy_id,
>                      sizeof(((struct vki_mii_ioctl_data *)&((struct vki_ifreq 
> *)ARG3)->vki_ifr_data)->phy_id) );
>       PRE_MEM_READ( "ioctl(SIOCGIFMIIREG)",
>                      (Addr)&((struct vki_mii_ioctl_data *)&((struct vki_ifreq 
> *)ARG3)->vki_ifr_data)->reg_num,
>                      sizeof(((struct vki_mii_ioctl_data *)&((struct vki_ifreq 
> *)ARG3)->vki_ifr_data)->reg_num) );
>       PRE_MEM_WRITE( "ioctl(SIOCGIFMIIREG)", ARG3, 
>                      sizeof(struct vki_ifreq));
>       break;
> 
> This scheme works fine as long as the ioctl is unchanged.
> So any ioctl that has padding and no flags this should be doable.
> 
> For all KVM ioctls with reserved fields that might become used on certain 
> flags, we have two options:
> 
> a: we would instruct valgrind to not check the reserved fields
> Whenever we start using them, we would still not check those field
> 
> b: we would instruct valgrind to not check the reserved fields if flags has a 
> certain value (e.g. 0), otherwise all reserved fields would be checked.
> Whenever we start using the reserved fields, valgrind would complain unless 
> we write all. So in that case we have to modify valgrind again
> 
> In essence a will cause false negatives, b will cause false positives
> 
> I think b is preferred

I agree.

Paolo