[Qemu-devel] [PATCH] migration/rdma: clang compilation fix

[Qemu-devel] [PATCH] migration/rdma: clang compilation fix

[Marcel Apfelbaum]

Configuring QEMU with:
        ../configure --cc=clang --enable-rdma

Leads to compilation error:

  CC      migration/rdma.o
  CC      migration/block.o
  qemu/migration/rdma.c:3615:58: error: taking address of packed member 'rkey' of class or structure
      'RDMARegisterResult' may result in an unaligned pointer value [-Werror,-Waddress-of-packed-member]
                            (uintptr_t)host_addr, NULL, &reg_result->rkey,
                                                         ^~~~~~~~~~~~~~~~

This is a false warning; even if RDMARegisterResult is "packed", rkey
is the first field so is guaranteed to be aligned.

Fix it by disabling the warning only for this instance.

Signed-off-by: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
---
 migration/rdma.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/migration/rdma.c b/migration/rdma.c
index 54a3c11540..e0f66a4717 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -3611,6 +3611,8 @@ static int qemu_rdma_registration_handle(QEMUFile *f, void *opaque)
                 }
                 chunk_start = ram_chunk_start(block, chunk);
                 chunk_end = ram_chunk_end(block, chunk + reg->chunks);
+                #pragma clang diagnostic push
+                #pragma clang diagnostic ignored "-Waddress-of-packed-member"
                 if (qemu_rdma_register_and_get_keys(rdma, block,
                             (uintptr_t)host_addr, NULL, &reg_result->rkey,
                             chunk, chunk_start, chunk_end)) {
@@ -3618,6 +3620,7 @@ static int qemu_rdma_registration_handle(QEMUFile *f, void *opaque)
                     ret = -EINVAL;
                     goto out;
                 }
+                #pragma clang diagnostic pop
 
                 reg_result->host_addr = (uintptr_t)block->local_host_addr;
 
-- 
2.17.1

Re: [Qemu-devel] [PATCH] migration/rdma: clang compilation fix

[Eric Blake]

On 3/4/19 2:42 AM, Marcel Apfelbaum wrote:
> Configuring QEMU with:
>          ../configure --cc=clang --enable-rdma
> 
> Leads to compilation error:
> 
>    CC      migration/rdma.o
>    CC      migration/block.o
>    qemu/migration/rdma.c:3615:58: error: taking address of packed member 'rkey' of class or structure
>        'RDMARegisterResult' may result in an unaligned pointer value [-Werror,-Waddress-of-packed-member]
>                              (uintptr_t)host_addr, NULL, &reg_result->rkey,
>                                                           ^~~~~~~~~~~~~~~~
> 
> This is a false warning; even if RDMARegisterResult is "packed", rkey
> is the first field so is guaranteed to be aligned.

Not so. If you packed struct, gcc is free to abut that struct next to 
some other unaligned field in a larger struct:

https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg06743.html

> 
> Fix it by disabling the warning only for this instance.

Ignoring the bug is not the same as fixing the bug.  You need to rework 
this, as the compiler warning is not a false negative.

> 
> Signed-off-by: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> ---
>   migration/rdma.c | 3 +++
>   1 file changed, 3 insertions(+)
-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH] migration/rdma: clang compilation fix

[Dr. David Alan Gilbert]

* Eric Blake (eblake@redhat.com) wrote:
> On 3/4/19 2:42 AM, Marcel Apfelbaum wrote:
> > Configuring QEMU with:
> >          ../configure --cc=clang --enable-rdma
> > 
> > Leads to compilation error:
> > 
> >    CC      migration/rdma.o
> >    CC      migration/block.o
> >    qemu/migration/rdma.c:3615:58: error: taking address of packed member 'rkey' of class or structure
> >        'RDMARegisterResult' may result in an unaligned pointer value [-Werror,-Waddress-of-packed-member]
> >                              (uintptr_t)host_addr, NULL, &reg_result->rkey,
> >                                                           ^~~~~~~~~~~~~~~~
> > 
> > This is a false warning; even if RDMARegisterResult is "packed", rkey
> > is the first field so is guaranteed to be aligned.
> 
> Not so. If you packed struct, gcc is free to abut that struct next to some
> other unaligned field in a larger struct:
> 
> https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg06743.html
> 
> > 
> > Fix it by disabling the warning only for this instance.
> 
> Ignoring the bug is not the same as fixing the bug.  You need to rework
> this, as the compiler warning is not a false negative.

In this case it is, however, I think it's easy enough to work around;
all we need is something like:

  uint23_t tmp_rkey;
                  if (qemu_rdma_register_and_get_keys(rdma, block,
                            (uintptr_t)host_addr, NULL, &tmp_rkey,
                            chunk, chunk_start, chunk_end)) {
                    error_report("cannot get rkey");
                    ret = -EINVAL;
                    goto out;
                  }
                  reg_result->rkey = tmp_rkey

Note that I think the structure pointed to by reg_result is
visible over the wire, so we can't change it's (unfortunate) layout.

Dave

> > 
> > Signed-off-by: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> > ---
> >   migration/rdma.c | 3 +++
> >   1 file changed, 3 insertions(+)
> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3226
> Virtualization:  qemu.org | libvirt.org
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH] migration/rdma: clang compilation fix

[Marcel Apfelbaum]

Hi David,

On 3/4/19 8:18 PM, Dr. David Alan Gilbert wrote:
> * Eric Blake (eblake@redhat.com) wrote:
>> On 3/4/19 2:42 AM, Marcel Apfelbaum wrote:
>>> Configuring QEMU with:
>>>           ../configure --cc=clang --enable-rdma
>>>
>>> Leads to compilation error:
>>>
>>>     CC      migration/rdma.o
>>>     CC      migration/block.o
>>>     qemu/migration/rdma.c:3615:58: error: taking address of packed member 'rkey' of class or structure
>>>         'RDMARegisterResult' may result in an unaligned pointer value [-Werror,-Waddress-of-packed-member]
>>>                               (uintptr_t)host_addr, NULL, &reg_result->rkey,
>>>                                                            ^~~~~~~~~~~~~~~~
>>>
>>> This is a false warning; even if RDMARegisterResult is "packed", rkey
>>> is the first field so is guaranteed to be aligned.
>> Not so. If you packed struct, gcc is free to abut that struct next to some
>> other unaligned field in a larger struct:
>>
>> https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg06743.html
>>
>>> Fix it by disabling the warning only for this instance.
>> Ignoring the bug is not the same as fixing the bug.  You need to rework
>> this, as the compiler warning is not a false negative.
> In this case it is, however, I think it's easy enough to work around;
> all we need is something like:
>
>    uint23_t tmp_rkey;
>                    if (qemu_rdma_register_and_get_keys(rdma, block,
>                              (uintptr_t)host_addr, NULL, &tmp_rkey,
>                              chunk, chunk_start, chunk_end)) {
>                      error_report("cannot get rkey");
>                      ret = -EINVAL;
>                      goto out;
>                    }
>                    reg_result->rkey = tmp_rkey

Thanks for the suggestion, looks OK for me, I will re-spin.

Thanks,
Marcel

> Note that I think the structure pointed to by reg_result is
> visible over the wire, so we can't change it's (unfortunate) layout.
>
> Dave
>
>>> Signed-off-by: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
>>> ---
>>>    migration/rdma.c | 3 +++
>>>    1 file changed, 3 insertions(+)
>> -- 
>> Eric Blake, Principal Software Engineer
>> Red Hat, Inc.           +1-919-301-3226
>> Virtualization:  qemu.org | libvirt.org
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH] migration/rdma: clang compilation fix

[Marcel Apfelbaum]

On 3/4/19 8:11 PM, Eric Blake wrote:
> On 3/4/19 2:42 AM, Marcel Apfelbaum wrote:
>> Configuring QEMU with:
>>          ../configure --cc=clang --enable-rdma
>>
>> Leads to compilation error:
>>
>>    CC      migration/rdma.o
>>    CC      migration/block.o
>>    qemu/migration/rdma.c:3615:58: error: taking address of packed 
>> member 'rkey' of class or structure
>>        'RDMARegisterResult' may result in an unaligned pointer value 
>> [-Werror,-Waddress-of-packed-member]
>>                              (uintptr_t)host_addr, NULL, 
>> &reg_result->rkey,
>> ^~~~~~~~~~~~~~~~
>>
>> This is a false warning; even if RDMARegisterResult is "packed", rkey
>> is the first field so is guaranteed to be aligned.
>
> Not so. If you packed struct, gcc is free to abut that struct next to 
> some other unaligned field in a larger struct:
>
> https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg06743.html
>

I wasn't referring to general case, but to this specific case. Actually 
gcc does not complain, only clang.

Anyway, Dave's suggestion is clean and works for me.

Thanks,
Marcel

>>
>> Fix it by disabling the warning only for this instance.
>
> Ignoring the bug is not the same as fixing the bug.  You need to 
> rework this, as the compiler warning is not a false negative.
>
>>
>> Signed-off-by: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
>> ---
>>   migration/rdma.c | 3 +++
>>   1 file changed, 3 insertions(+)