From 3d473e183da99d9965112e45ab43f4d06c01ccab Mon Sep 17 00:00:00 2001
From: Milian Wolff <milian.wolff@kdab.com>
Date: Tue, 24 Apr 2018 15:41:21 +0200
Subject: [PATCH] Fix race conditions in validate_mem via atomics

When atomics are not available, the address cache is disabled.

Fixes data race found by TSAN:

WARNING: ThreadSanitizer: data race (pid=17824)
  Read of size 8 at 0x7f4dac484140 by thread T3:
    #0 validate_mem ../../src/x86_64/Ginit.c:201 (libunwind.so.
8+0x00000001f836)
    #1 access_mem ../../src/x86_64/Ginit.c:242 (libunwind.so.8+0x00000001fe06)
    #2 dwarf_get ../../include/tdep-x86_64/libunwind_i.h:168 (libunwind.so.
8+0x0000000221cf)
    #3 _ULx86_64_access_reg ../../src/x86_64/Gregs.c:130 (libunwind.so.
8+0x000000022e99)
    #4 _ULx86_64_get_reg ../../src/mi/Gget_reg.c:40 (libunwind.so.
8+0x00000001e5bc)
    #5 apply_reg_state ../../src/dwarf/Gparser.c:796 (libunwind.so.
8+0x000000038209)
    #6 _ULx86_64_dwarf_step ../../src/dwarf/Gparser.c:961 (libunwind.so.
8+0x000000039dbc)
    #7 _ULx86_64_step ../../src/x86_64/Gstep.c:71 (libunwind.so.
8+0x00000002481f)
    #8 trace_init_addr ../../src/x86_64/Gtrace.c:248 (libunwind.so.
8+0x000000026f0e)
    #9 trace_lookup ../../src/x86_64/Gtrace.c:330 (libunwind.so.
8+0x000000027429)
    #10 _ULx86_64_tdep_trace ../../src/x86_64/Gtrace.c:447 (libunwind.so.
8+0x00000002789a)
    #11 unw_backtrace ../../src/mi/backtrace.c:69 (libunwind.so.
8+0x00000001cb07)

  Previous write of size 8 at 0x7f4dac484140 by thread T2:
    #0 validate_mem ../../src/x86_64/Ginit.c:220 (libunwind.so.
8+0x00000001fc54)
    #1 access_mem ../../src/x86_64/Ginit.c:242 (libunwind.so.8+0x00000001fe06)
    #2 dwarf_get ../../include/tdep-x86_64/libunwind_i.h:168 (libunwind.so.
8+0x0000000221cf)
    #3 _ULx86_64_access_reg ../../src/x86_64/Gregs.c:130 (libunwind.so.
8+0x000000022e99)
    #4 _ULx86_64_get_reg ../../src/mi/Gget_reg.c:40 (libunwind.so.
8+0x00000001e5bc)
    #5 apply_reg_state ../../src/dwarf/Gparser.c:796 (libunwind.so.
8+0x000000038209)
    #6 _ULx86_64_dwarf_step ../../src/dwarf/Gparser.c:961 (libunwind.so.
8+0x000000039dbc)
    #7 _ULx86_64_step ../../src/x86_64/Gstep.c:71 (libunwind.so.
8+0x00000002481f)
    #8 trace_init_addr ../../src/x86_64/Gtrace.c:248 (libunwind.so.
8+0x000000026f0e)
    #9 trace_lookup ../../src/x86_64/Gtrace.c:330 (libunwind.so.
8+0x000000027429)
    #10 _ULx86_64_tdep_trace ../../src/x86_64/Gtrace.c:447 (libunwind.so.
8+0x00000002789a)
    #11 unw_backtrace ../../src/mi/backtrace.c:69 (libunwind.so.
8+0x00000001cb07)

  Location is global 'last_good_addr' of size 32 at 0x7f4dac484140
(libunwind.so.8+0x000000273140)
---
 src/x86_64/Ginit.c | 71 ++++++++++++++++++++++++++++++++--------------
 1 file changed, 50 insertions(+), 21 deletions(-)

diff --git a/src/x86_64/Ginit.c b/src/x86_64/Ginit.c
index 76e183e0..21d8c496 100644
--- a/src/x86_64/Ginit.c
+++ b/src/x86_64/Ginit.c
@@ -174,14 +174,56 @@ tdep_init_mem_validate (void)
 }
 
 /* Cache of already validated addresses */
+#if HAVE_ATOMIC_OPS_H
 #define NLGA 4
-static unw_word_t last_good_addr[NLGA];
-static int lga_victim;
+static AO_T last_good_addr[NLGA];
+static AO_T lga_victim;
+
+static int
+is_cached_valid_mem(unw_word_t addr)
+{
+  int i;
+  for (i = 0; i < NLGA; i++)
+    {
+      if (addr == AO_load(&last_good_addr[i]))
+        return 1;
+    }
+  return 0;
+}
+
+static void
+cache_valid_mem(unw_word_t addr)
+{
+  int i, victim;
+  victim = AO_load(&lga_victim);
+  for (i = 0; i < NLGA; i++) {
+    if (AO_compare_and_swap(&last_good_addr[victim], 0, addr)) {
+      return;
+    }
+    victim = (victim + 1) % NLGA;
+  }
+
+  /* All slots full. Evict the victim. */
+  AO_store(&last_good_addr[victim], addr);
+  victim = (victim + 1) % NLGA;
+  AO_store(&lga_victim, victim);
+}
+#else
+static int
+is_cached_valid_mem(unw_word_t addr UNUSED)
+{
+  return 0;
+}
+
+static void
+cache_valid_mem(unw_word_t addr UNUSED)
+{
+}
+#endif
 
 static int
 validate_mem (unw_word_t addr)
 {
-  int i, victim;
   size_t len;
 
   if (PAGE_START(addr + sizeof (unw_word_t) - 1) == PAGE_START(addr))
@@ -194,28 +236,13 @@ validate_mem (unw_word_t addr)
   if (addr == 0)
     return -1;
 
-  for (i = 0; i < NLGA; i++)
-    {
-      if (last_good_addr[i] && (addr == last_good_addr[i]))
-        return 0;
-    }
+  if (is_cached_valid_mem(addr))
+    return 0;
 
   if (mem_validate_func ((void *) addr, len) == -1)
     return -1;
 
-  victim = lga_victim;
-  for (i = 0; i < NLGA; i++) {
-    if (!last_good_addr[victim]) {
-      last_good_addr[victim++] = addr;
-      return 0;
-    }
-    victim = (victim + 1) % NLGA;
-  }
-
-  /* All slots full. Evict the victim. */
-  last_good_addr[victim] = addr;
-  victim = (victim + 1) % NLGA;
-  lga_victim = victim;
+  cache_valid_mem(addr);
 
   return 0;
 }
@@ -330,8 +357,10 @@ x86_64_local_addr_space_init (void)
   local_addr_space.acc.get_proc_name = get_static_proc_name;
   unw_flush_cache (&local_addr_space, 0, 0);
 
+#if NLGA > 0
   memset (last_good_addr, 0, sizeof (unw_word_t) * NLGA);
   lga_victim = 0;
+#endif
 }
 
 #endif /* !UNW_REMOTE_ONLY */