Bug 484935 - [patch] Valgrind reports false "Conditional jump or move depends on...

Patch contributed by Peter Edwards <peadar@arista.com>

Testcase modified and added to a new directory, memcheck/tests/arm64

diff --git a/.gitignore b/.gitignore
index abc693c825e1b8d089d293d72a401b4680ad6e56..3623378acd6cf4e025cd77269b3e0f0248beae40 100644 (file)
--- a/.gitignore
+++ b/.gitignore
@@ -1104,6 +1104,15 @@
 /memcheck/tests/amd64-solaris/ldsoexec
 /memcheck/tests/amd64-solaris/scalar
 
+# /memcheck/tests/arm64/
+/memcheck/tests/arm64/*.diff*
+/memcheck/tests/arm64/*.dSYM
+/memcheck/tests/arm64/*.out
+/memcheck/tests/arm64/.deps
+/memcheck/tests/arm64/Makefile
+/memcheck/tests/arm64/Makefile.in
+/memcheck/tests/arm64/bug484935
+
 # /memcheck/tests/arm64-linux/
 /memcheck/tests/arm64-linux/.deps
 /memcheck/tests/arm64-linux/Makefile

diff --git a/NEWS b/NEWS
index 818901e7f1d73f198d82cfbfa8dcd60284103761..1c3f9dce57764ec5ada2bf878d3cb0e780eecf51 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -75,6 +75,8 @@ are not entered into bugzilla tend to get forgotten about or ignored.
 484002  Add suppression for invalid read in glibc's __wcpncpy_avx2() via wcsxfrm()
 484426  aarch64: 0.5 gets rounded to 0
 484480  False positives when using sem_trywait
+484935  [patch] Valgrind reports false "Conditional jump or move depends on
+        uninitialised value" errors for aarch64 signal handlers
 n-i-bz  Add redirect for memccpy
 
 To see details of a given bug, visit

diff --git a/configure.ac b/configure.ac
index 21ec7a07f17e2c82696c807b27e6ffe9abd93e04..b191eabcbacfb282ef8333355f396157b383ea69 100755 (executable)
--- a/configure.ac
+++ b/configure.ac
@@ -5614,6 +5614,7 @@ AC_CONFIG_FILES([
    memcheck/tests/Makefile
    memcheck/tests/common/Makefile
    memcheck/tests/amd64/Makefile
+   memcheck/tests/arm64/Makefile
    memcheck/tests/x86/Makefile
    memcheck/tests/linux/Makefile
    memcheck/tests/linux/debuginfod-check.vgtest

diff --git a/coregrind/m_sigframe/sigframe-arm64-linux.c b/coregrind/m_sigframe/sigframe-arm64-linux.c
index 5ec303f888a5d84806072fb4689ac68c7f45cd72..f98d563b150f4e5cbd5539bf1a1a047784697040 100644 (file)
--- a/coregrind/m_sigframe/sigframe-arm64-linux.c
+++ b/coregrind/m_sigframe/sigframe-arm64-linux.c
@@ -197,8 +197,6 @@ void VG_(sigframe_create)( ThreadId tid,
    tst->arch.vex.guest_X2 = (Addr)&rsf->sig.uc;
 
    VG_(set_SP)(tid, sp);
-   VG_TRACK( post_reg_write, Vg_CoreSignal, tid, VG_O_STACK_PTR,
-             sizeof(Addr));
    tst->arch.vex.guest_X0 = sigNo; 
 
    if (flags & VKI_SA_RESTORER)
@@ -208,6 +206,19 @@ void VG_(sigframe_create)( ThreadId tid,
           = (Addr)&VG_(arm64_linux_SUBST_FOR_rt_sigreturn);
 
    tst->arch.vex.guest_PC = (Addr)handler;
+
+   VG_TRACK( post_reg_write, Vg_CoreSignal, tid,
+         VG_O_STACK_PTR, sizeof(Addr));
+   VG_TRACK( post_reg_write, Vg_CoreSignal, tid,
+         offsetof(VexGuestARM64State, guest_X0), sizeof(Addr));
+   VG_TRACK( post_reg_write, Vg_CoreSignal, tid,
+         offsetof(VexGuestARM64State, guest_X1), sizeof(Addr));
+   VG_TRACK( post_reg_write, Vg_CoreSignal, tid,
+         offsetof(VexGuestARM64State, guest_X2), sizeof(Addr));
+   VG_TRACK( post_reg_write, Vg_CoreSignal, tid,
+         offsetof(VexGuestARM64State, guest_X30), sizeof(Addr));
+   VG_TRACK( post_reg_write, Vg_CoreSignal, tid,
+         offsetof(VexGuestARM64State, guest_PC), sizeof(Addr));
 }
 
 

diff --git a/memcheck/tests/Makefile.am b/memcheck/tests/Makefile.am
index 2bf96aded78f9618f12a34ed2d602dad68c6a50d..f4402d0f43fccb6709ea6bc10bd44899816cb499 100644 (file)
--- a/memcheck/tests/Makefile.am
+++ b/memcheck/tests/Makefile.am
@@ -10,6 +10,9 @@ endif
 if VGCONF_ARCHS_INCLUDE_AMD64
 SUBDIRS += amd64
 endif
+if VGCONF_ARCHS_INCLUDE_ARM64
+SUBDIRS += arm64
+endif
 if VGCONF_ARCHS_INCLUDE_MIPS32
 SUBDIRS += mips32
 endif

diff --git a/memcheck/tests/arm64/Makefile.am b/memcheck/tests/arm64/Makefile.am
new file mode 100644 (file)
index 0000000..790732f
--- /dev/null
+++ b/memcheck/tests/arm64/Makefile.am
@@ -0,0 +1,18 @@
+
+include $(top_srcdir)/Makefile.tool-tests.am
+
+dist_noinst_SCRIPTS = filter_stderr
+
+
+EXTRA_DIST = \
+       bug484935.stderr.exp bug484935.vgtest
+
+check_PROGRAMS = \
+       bug484935
+
+
+AM_CFLAGS    += @FLAG_M64@
+AM_CXXFLAGS  += @FLAG_M64@
+AM_CCASFLAGS += @FLAG_M64@
+
+

diff --git a/memcheck/tests/arm64/bug484935.c b/memcheck/tests/arm64/bug484935.c
new file mode 100644 (file)
index 0000000..ff96f07
--- /dev/null
+++ b/memcheck/tests/arm64/bug484935.c
@@ -0,0 +1,53 @@
+#include <assert.h>
+#include <signal.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/time.h>
+
+static void signalHandler(int sig, siginfo_t* info, void* uctx_v)
+{
+   if (sig != SIGALRM)
+      abort();
+   if (info == 0)
+      abort();
+   if (uctx_v == 0)
+      abort();
+}
+
+void* load_memory_content(void** ptr)
+{
+   void* result;
+   __asm__ volatile(
+      // load x0, x1, x2 with data from ptr, and loop for a while. If we get
+      // a signal in the loop, these registers have uninitialized data in
+      // them, but should be valid inside the signal handler. Without our
+      // patch, valgrind complains. We can remove the individual lines from
+      // the patch, and see each argument in turn affecting valgrind
+      "LDR x0, [%1]\n"
+      "LDR x1, [%1, #8]\n"
+      "LDR x2, [%1, #16]\n"
+      "mov %0, x0\n"
+      "mov x3, #2000\n"
+      "loop:"
+      " subs x3, x3, #1\n"
+      " b.ne loop\n"
+      : "=r"(result)
+      : "r"(ptr)
+      : "x0", "x1", "x2", "x3");
+   return result;
+}
+
+int main()
+{
+   struct sigaction sa;
+   memset(&sa, 0, sizeof sa);
+   sa.sa_flags     = SA_SIGINFO;
+   sa.sa_sigaction = signalHandler;
+   int rc          = sigaction(SIGALRM, &sa, 0);
+   assert(rc == 0);
+   struct itimerval timer = {{0, 1000}, {0, 1000}};
+   setitimer(ITIMER_REAL, &timer, 0);
+   void** q = malloc(100);
+   for (int i = 0; i < 1000; ++i)
+      load_memory_content(q);
+}

diff --git a/memcheck/tests/arm64/bug484935.stderr.exp b/memcheck/tests/arm64/bug484935.stderr.exp
new file mode 100644 (file)
index 0000000..e69de29

diff --git a/memcheck/tests/arm64/bug484935.vgtest b/memcheck/tests/arm64/bug484935.vgtest
new file mode 100644 (file)
index 0000000..26f54c9
--- /dev/null
+++ b/memcheck/tests/arm64/bug484935.vgtest
@@ -0,0 +1,2 @@
+prog: bug484935
+vgopts: -q

diff --git a/memcheck/tests/arm64/filter_stderr b/memcheck/tests/arm64/filter_stderr
new file mode 100755 (executable)
index 0000000..a778e97
--- /dev/null
+++ b/memcheck/tests/arm64/filter_stderr
@@ -0,0 +1,3 @@
+#! /bin/sh
+
+../filter_stderr "$@"