Bug 425820

Created attachment 131191 [details]
Possible bug fix

This bug is essentially the same as 409429 but for vpcmpeqq instead of vpcmpeqd. Take the following example code:

#include <stdint.h>

int main()
{
  uint32_t array[32] = { 0 }; uint32_t ret[8];
  uint32_t offsets[4] = { 0, 1, 2, 3 };

  __asm__ __volatile__ (
    "vmovdqu %3, %%ymm14\n" /* make sure that ymm14 is uninitalized */
    "vmovdqu %1, %%xmm15\n" /* load offsets */
    "vpcmpeqq %%ymm14, %%ymm14, %%ymm14\n" /* set ymm14 to all-1 */
    "vpgatherdq %%ymm14, (%2, %%xmm15, 1), %%ymm9\n"
    "vmovdqu %%ymm9, %0\n"
    : "=m"(ret)
    : "m"(offsets), "r"(array), "m"(ret)
    : "%xmm15", "%ymm14", "%ymm9"
    );

  return ret[0];
}

compiled with gcc -O2 --std=gnu11 -g -mavx2 on Ubuntu 18.04 or current Debian unstable. This first mov is there to make sure that valgrind recognizes the register as uninintialized in the reduced test case. This was observed in the wild while debugging the AVX2 Keccak implementation (https://github.com/XKCP/XKCP/blob/master/lib/low/KeccakP-1600/AVX2/KeccakP-1600-AVX2.s) sharing the second vmovdqu until vpgatherdq.

valgrind 3.16.1 reports:

==15427== Memcheck, a memory error detector
==15427== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==15427== Using Valgrind-3.16.1 and LibVEX; rerun with -h for copyright info
==15427== Command: ./a.out
==15427== 
==15427== Use of uninitialised value of size 8
==15427==    at 0x1085B9: main (test.c:8)
==15427== 
==15427== Syscall param exit_group(status) contains uninitialised byte(s)
==15427==    at 0x4F21E66: _Exit (_exit.c:31)
==15427==    by 0x4E801C1: __run_exit_handlers (exit.c:132)
==15427==    by 0x4E801E9: exit (exit.c:139)
==15427==    by 0x4E5EB9D: (below main) (libc-start.c:344)
==15427== 
==15427== 
==15427== HEAP SUMMARY:
==15427==     in use at exit: 0 bytes in 0 blocks
==15427==   total heap usage: 0 allocs, 0 frees, 0 bytes allocated
==15427== 
==15427== All heap blocks were freed -- no leaks are possible
==15427== 
==15427== Use --track-origins=yes to see where uninitialised values come from
==15427== For lists of detected and suppressed errors, rerun with: -s
==15427== ERROR SUMMARY: 5 errors from 2 contexts (suppressed: 0 from 0)

Based on the fix from 409429, I've attached a preliminary patch that works for me (I'm not sure if changes to VEC/prov/host_amd64_isel.c are required as well):

==15451== Memcheck, a memory error detector
==15451== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==15451== Using Valgrind-3.17.0.GIT and LibVEX; rerun with -h for copyright info
==15451== Command: ./a.out
==15451== 
==15451== 
==15451== HEAP SUMMARY:
==15451==     in use at exit: 0 bytes in 0 blocks
==15451==   total heap usage: 0 allocs, 0 frees, 0 bytes allocated
==15451== 
==15451== All heap blocks were freed -- no leaks are possible
==15451== 
==15451== For lists of detected and suppressed errors, rerun with: -s
==15451== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

Thanks for considering.