Created attachment 130181 [details] The script build_dpdk.sh installs and builds DPDK 19.11 SUMMARY When using valgrind 3.16.1 on an application using DPDK libraries, the following run-time error is encountered: vex amd64->IR: unhandled instruction bytes: 0xF 0xC7 0xF8 0x89 0x4 0x24 0xF 0x82 0x63 0x1 STEPS TO REPRODUCE 1. Install DPDK 19.11 and set the environment variable RTE_SDK to point to the directory where DPDK 19.11 is installed. This can be accomplished by executing the attached script build_dpdk.sh. 2. Untar the attached filed Test_Valgrind.tar 3. cd Test_Valgrind 4. make 5. valgrind ./mla_test OBSERVED RESULT epkswid@elx783742wq:~/Test_Valgrind$ ~/Downloads/valgrind-3.16.1/bin/bin/valgrind ./mla_test ==23290== Memcheck, a memory error detector ==23290== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al. ==23290== Using Valgrind-3.16.1 and LibVEX; rerun with -h for copyright info ==23290== Command: ./mla_test ==23290== vex amd64->IR: unhandled instruction bytes: 0xF 0xC7 0xF8 0x89 0x4 0x24 0xF 0x82 0x63 0x1 vex amd64->IR: REX=0 REX.W=0 REX.R=0 REX.X=0 REX.B=0 vex amd64->IR: VEX=0 VEX.L=0 VEX.nVVVV=0x0 ESC=0F vex amd64->IR: PFX.66=0 PFX.F2=0 PFX.F3=0 ==23290== valgrind: Unrecognised instruction at address 0x10c254. ==23290== at 0x10C254: rte_rand_init (in /home/epkswid/Test_Valgrind/mla_test) ==23290== by 0x13C0EC: __libc_csu_init (in /home/epkswid/Test_Valgrind/mla_test) ==23290== by 0x548CB27: (below main) (libc-start.c:266) EXPECTED RESULT No unhandled instructions should occur when executing valgrind. SOFTWARE/OS VERSIONS Windows: macOS: Linux/KDE Plasma: (available in About System) KDE Plasma Version: KDE Frameworks Version: Qt Version: ADDITIONAL INFORMATION valgrind 3.16.1 epkswid@elx783742wq:~/Test_Valgrind$ uname -a Linux elx783742wq 4.15.0-109-generic #110-Ubuntu SMP Tue Jun 23 02:39:32 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux epkswid@elx783742wq:~/Test_Valgrind$ cat /etc/*-release DISTRIB_ID=Ubuntu DISTRIB_RELEASE=18.04 DISTRIB_CODENAME=bionic DISTRIB_DESCRIPTION="Ubuntu 18.04.4 LTS" NAME="Ubuntu" VERSION="18.04.4 LTS (Bionic Beaver)" ID=ubuntu ID_LIKE=debian PRETTY_NAME="Ubuntu 18.04.4 LTS" VERSION_ID="18.04" HOME_URL="https://www.ubuntu.com/" SUPPORT_URL="https://help.ubuntu.com/" BUG_REPORT_URL="https://bugs.launchpad.net/ubuntu/" PRIVACY_POLICY_URL="https://www.ubuntu.com/legal/terms-and-policies/privacy-policy" VERSION_CODENAME=bionic UBUNTU_CODENAME=bionic epkswid@elx783742wq:~/Test_Valgrind$ gcc -v Using built-in specs. COLLECT_GCC=gcc COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/7/lto-wrapper OFFLOAD_TARGET_NAMES=nvptx-none OFFLOAD_TARGET_DEFAULT=1 Target: x86_64-linux-gnu Configured with: ../src/configure -v --with-pkgversion='Ubuntu 7.5.0-3ubuntu1~18.04' --with-bugurl=file:///usr/share/doc/gcc-7/README.Bugs --enable-languages=c,ada,c++,go,brig,d,fortran,objc,obj-c++ --prefix=/usr --with-gcc-major-version-only --program-suffix=-7 --program-prefix=x86_64-linux-gnu- --enable-shared --enable-linker-build-id --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --libdir=/usr/lib --enable-nls --enable-bootstrap --enable-clocale=gnu --enable-libstdcxx-debug --enable-libstdcxx-time=yes --with-default-libstdcxx-abi=new --enable-gnu-unique-object --disable-vtable-verify --enable-libmpx --enable-plugin --enable-default-pie --with-system-zlib --with-target-system-zlib --enable-objc-gc=auto --enable-multiarch --disable-werror --with-arch-32=i686 --with-abi=m64 --with-multilib-list=m32,m64,mx32 --enable-multilib --with-tune=generic --enable-offload-targets=nvptx-none --without-cuda-driver --enable-checking=release --build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu Thread model: posix gcc version 7.5.0 (Ubuntu 7.5.0-3ubuntu1~18.04) epkswid@elx783742wq:~/Test_Valgrind$ When the executable file mla_test is executed without valgrind, the following is printed: epkswid@elx783742wq:~/Test_Valgrind$ ./mla_test MEMPOOL: Cannot allocate tailq entry! epkswid@elx783742wq:~/Test_Valgrind$ There is thus no crash although there is an error message printed.
Created attachment 130183 [details] The source code and Makefile for reproduction of the error
This is RDSEED which is indeed not supported. Valgrind also doesn't publish support through cpuid (so the program really shouldn't use it).
Created attachment 131196 [details] patch This patch implements RDSEED support and also adds the rdseed test to the testsuite.
(In reply to Alexandra Hajkova from comment #3) > Created attachment 131196 [details] > patch > > This patch implements RDSEED support and also adds the rdseed test to the > testsuite. This looks really good, thanks. The commit message could explain the patch a bit more, which would make review a bit easier. And please add the bug number in the commit message or the URL to this bug. Also, please add a NEWS entry for the bug. It is interesting how similar rdrand and rdseed are, but they do have separate cpuid flags, and are apparently using a different way to generate the random bits. I am not sure the way a 64bit value is created from two calls to rdseed produces the correct "randomness", but it is exactly how rdrand does it. So we'll have to assume that was done correctly. Given that the rdrand and rdseed implementation are so similar I wonder if you could merge the /* 0F C7 /6 no-F2-or-F3 = RDRAND */ and /* 0F C7 /7 = RDSEED */ parts. RDSEED also has the no-F3-or-F3 part, so basically you could just check: (gregLO3ofRM(modrm) == 6 && (archinfo->hwcaps & VEX_HWCAPS_AMD64_RDRAND)) || (gregLO3ofRM(modrm) == 7 && (archinfo->hwcaps & VEX_HWCAPS_AMD64_RDSEED)) && epartIsReg(modrm) && haveNoF2noF3(pfx) && (sz == 8 || sz == 4 || sz == 2)) And then make the dLO and dHI assignments depend on gregLO3ofRM(modrm) == 6 for amd64g_dirtyhelper_RDRAND or gregLO3ofRM(modrm) == 7 for amd64g_dirtyhelper_RDSEED (and same for the DIP). That would remove some duplicate code. +/* This is wrong... assert( !(cmask != 0 && dmask != 0) ); assert( !(cmask == 0 && dmask == 0) ); +*/ It is indeed wrong. But please instead of commenting it out, either remove it or adjust the asserts to also take the new bmask into account. But all the above are really just nitpicks, nice work.
Created attachment 131234 [details] patch Hi Mark, thank you very much for the review, the updated version adresses all your objections.
Created attachment 131401 [details] patch
Thanks, looks good. Committed with a small tweak to add some brackets and updated commit message saying the implementation is based on the existing rdrand support. commit 17d2989a9e9bb07d4b5a2b9607e584845c494b1b Author: Alexandra Hájková <ahajkova@redhat.com> Date: Fri Jul 31 13:19:16 2020 +0200 amd64: Implement RDSEED This commit implements amd64 RDSEED instruction, on hosts that have it and adds the new test case - none/tests/amd64/rdseed based on the existing rdrand support. https://bugs.kde.org/show_bug.cgi?id=424298