Bug 425820 - Failure to recognize vpcmpeqq as a dependency breaking idiom
Summary: Failure to recognize vpcmpeqq as a dependency breaking idiom
Status: RESOLVED FIXED
Alias: None
Product: valgrind
Classification: Developer tools
Component: memcheck (other bugs)
Version First Reported In: unspecified
Platform: Debian unstable Linux
: NOR normal
Target Milestone: ---
Assignee: Julian Seward
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-08-26 08:27 UTC by Sebastian Ramacher
Modified: 2020-09-19 10:06 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed/Implemented In:
Sentry Crash Report:


Attachments
Possible bug fix (1.18 KB, patch)
2020-08-26 08:27 UTC, Sebastian Ramacher
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Sebastian Ramacher 2020-08-26 08:27:41 UTC
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.
Comment 1 Julian Seward 2020-09-19 10:06:56 UTC
Fixed in the trunk, rev 92eec784a3f1863397ec00d333d16469795627bf,
along with a bunch of other missing cases.  Thanks for the report.