Re: [Qemu-devel] [PATCH v2 17/19] linux-user: Use both si_code and si_signo when converting siginfo_t

[Peter Maydell]

On 8 June 2016 at 07:30, Riku Voipio <address@hidden> wrote:
> At least on Debian jessie, this blows up a selection of architectures:
>
> /home/voipio/linaro/qemu/linux-user/signal.c: In function 
> ‘host_to_target_siginfo’:
> /home/voipio/linaro/qemu/linux-user/signal.c:387:10: error: 
> ‘tgt_tmp._sifields._sigchld._stime’ may be used uninitialized in this 
> function [-Werror=maybe-uninitialized]
>          __put_user(info->_sifields._sigchld._stime,
>           ^
> /home/voipio/linaro/qemu/linux-user/signal.c:403:22: note: 
> ‘tgt_tmp._sifields._sigchld._stime’ was declared here
>      target_siginfo_t tgt_tmp;
>                       ^
> /home/voipio/linaro/qemu/linux-user/signal.c:385:10: error: 
> ‘tgt_tmp._sifields._sigchld._utime’ may be used uninitialized in this 
> function [-Werror=maybe-uninitialized]
>          __put_user(info->_sifields._sigchld._utime,
>           ^
> /home/voipio/linaro/qemu/linux-user/signal.c:403:22: note: 
> ‘tgt_tmp._sifields._sigchld._utime’ was declared here
>      target_siginfo_t tgt_tmp;
>                       ^
> /home/voipio/linaro/qemu/linux-user/signal.c:383:10: error: 
> ‘tgt_tmp._sifields._sigchld._status’ may be used uninitialized in this 
> function [-Werror=maybe-uninitialized]
>          __put_user(info->_sifields._sigchld._status,
>           ^
> /home/voipio/linaro/qemu/linux-user/signal.c:403:22: note: 
> ‘tgt_tmp._sifields._sigchld._status’ was declared here
>      target_siginfo_t tgt_tmp;
>                       ^
> cc1: all warnings being treated as errors
>
> These appear to be the architectures where setup_rt_frame isn't implemented.

So as far as I can tell this is a combination of:
 * without setup_rt_frame() the compiler makes different decisions
   about whether to inline tswap_siginfo() into host_to_target_siginfo()
   [you can provoke it on all targets by marking tswap_siginfo 'inline']
 * gcc not being able to figure out that the _sigchld fields of the union
   are only read in the tswap_siginfo() switch if they were set in the
   host_to_target_siginfo_noswap() switch (likely because the type info
   is pushed in and out of the top 16 bits of the si_code field)

The simplest fix seems to be to add this to the top of
host_to_target_siginfo_noswap():

+    /* This memset serves two purposes:
+     * (1) ensure we don't leak random junk to the guest later
+     * (2) placate false positives from gcc about fields
+     *     being used uninitialized if it chooses to inline both this
+     *     function and tswap_siginfo() into host_to_target_siginfo().
+     */
+    memset(tinfo->_sifields._pad, 0, sizeof(tinfo->_sifields._pad));

I have no idea why gcc only complains about the _sigchld fields and
not any others, though.

thanks
-- PMM