From: Bart Van Assche <bvanassche@acm.org>
Date: Sat, 31 May 2008 07:55:51 +0000 (+0000)
Subject: - Replaced algorithm for suppressing false positives on symbol
X-Git-Tag: svn/VALGRIND_3_4_0~533
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ae68d9241588668fbe2a642e96f3151cb63d2fc0;p=thirdparty%2Fvalgrind.git

- Replaced algorithm for suppressing false positives on symbol
  relocations: instead of not instrumenting code in .plt sections (which
  is x86 specific), suppress all data accesses to .plt and .got.plt
  sections (which works both on x86 and on PowerPC).
- A warning is now printed upon startup to inform users about false
  positives which will be reported on PowerPC because with the current
  version of VEX it is impossible for tools to recognize atomic PowerPC
  instructions as such.


git-svn-id: svn://svn.valgrind.org/valgrind/trunk@8160
---

diff --git a/exp-drd/drd_main.c b/exp-drd/drd_main.c
index 5ff2aace87..e2de78bc38 100644
--- a/exp-drd/drd_main.c
+++ b/exp-drd/drd_main.c
@@ -554,6 +554,52 @@ void drd_stop_using_nonstack_mem(const Addr a1, const SizeT len)
   drd_stop_using_mem(a1, len, False);
 }
 
+/** Suppress data race reports on all addresses contained in .plt and
+ *  .got.plt sections inside the address range [ a, a + len [. The data in
+ *  these sections is modified by _dl_relocate_object() every time a function
+ *  in a shared library is called for the first time. Since the first call
+ *  to a function in a shared library can happen from a multithreaded context,
+ *  such calls can cause conflicting accesses. See also Ulrich Drepper's
+ *  paper "How to Write Shared Libraries" for more information about relocation
+ *  (http://people.redhat.com/drepper/dsohowto.pdf).
+ */
+static void suppress_relocation_conflicts(const Addr a, const SizeT len)
+{
+  const DebugInfo* di;
+
+#if 0
+  VG_(printf)("Evaluating range @ 0x%lx size %ld\n", a, len);
+#endif
+
+  for (di = VG_(next_seginfo)(0); di; di = VG_(next_seginfo)(di))
+  {
+    Addr  avma;
+    SizeT size;
+
+    avma = VG_(seginfo_get_plt_avma)(di);
+    size = VG_(seginfo_get_plt_size)(di);
+    if (a <= avma && avma + size <= a + len)
+    {
+#if 0
+      VG_(printf)("Suppressing .plt @ 0x%lx size %ld\n", avma, size);
+#endif
+      tl_assert(VG_(seginfo_sect_kind)(NULL, 0, avma) == Vg_SectPLT);
+      drd_start_suppression(avma, avma + size, ".plt");
+    }
+
+    avma = VG_(seginfo_get_gotplt_avma)(di);
+    size = VG_(seginfo_get_gotplt_size)(di);
+    if (a <= avma && avma + size <= a + len)
+    {
+#if 0
+      VG_(printf)("Suppressing .got.plt @ 0x%lx size %ld\n", avma, size);
+#endif
+      tl_assert(VG_(seginfo_sect_kind)(NULL, 0, avma) == Vg_SectGOTPLT);
+      drd_start_suppression(avma, avma + size, ".gotplt");
+    }
+  }
+}
+
 static
 void drd_start_using_mem_w_perms(const Addr a, const SizeT len,
                                  const Bool rr, const Bool ww, const Bool xx)
@@ -561,6 +607,8 @@ void drd_start_using_mem_w_perms(const Addr a, const SizeT len,
   thread_set_vg_running_tid(VG_(get_running_tid)());
 
   drd_start_using_mem(a, len);
+
+  suppress_relocation_conflicts(a, len);
 }
 
 /* Called by the core when the stack of a thread grows, to indicate that */
@@ -836,6 +884,12 @@ void drd_post_clo_init(void)
 {
 #  if defined(VGP_x86_linux) || defined(VGP_amd64_linux)
   /* fine */
+#  elif defined(VGP_ppc64_linux) || defined(VGP_ppc64_linux)
+  VG_(printf)(
+"\nWARNING: support for PowerPC-specific atomic instructions like lwarx and\n"
+"stwcx is not yet complete. As a result, false positives will be reported on\n"
+"code that uses these instructions. This will happen e.g. when printf() is\n"
+"called from more than one thread.\n\n");
 #  else
   VG_(printf)("\nWARNING: DRD has only been tested on x86-linux and amd64-linux.\n\n");
 #  endif
@@ -976,12 +1030,6 @@ IRSB* drd_instrument(VgCallbackClosure* const closure,
 
     switch (st->tag)
     {
-    case Ist_IMark:
-      instrument = VG_(seginfo_sect_kind)(NULL, 0, st->Ist.IMark.addr)
-        != Vg_SectPLT;
-      addStmtToIRSB(bb, st);
-      break;
-
     case Ist_MBE:
       switch (st->Ist.MBE.event)
       {
diff --git a/glibc-2.X-drd.supp b/glibc-2.X-drd.supp
index 26e1735286..ab486293ae 100644
--- a/glibc-2.X-drd.supp
+++ b/glibc-2.X-drd.supp
@@ -1,15 +1,6 @@
 {
    dl
    exp-drd:ConflictingAccess
-   fun:_dl_lookup_symbol_x
-   fun:_dl_fixup
-   fun:_dl_runtime_resolve
-}
-{
-   dl
-   exp-drd:ConflictingAccess
-   fun:do_lookup_x
-   fun:_dl_lookup_symbol_x
    fun:_dl_fixup
    fun:_dl_runtime_resolve
 }
@@ -19,11 +10,6 @@
    fun:_dl_fini
    fun:exit
 }
-{
-   dl
-   exp-drd:ConflictingAccess
-   fun:_dl_fixup
-}
 {
    dl-2.6.*
    exp-drd:ConflictingAccess
@@ -37,32 +23,6 @@
    obj:/lib*/ld-*.so
    obj:/lib*/ld-*.so
 }
-{
-   dl-dlsym-1
-   exp-drd:ConflictingAccess
-   obj:/lib*/ld-*.so
-   obj:/lib*/libc-*.so
-   obj:/lib*/libdl-*.so
-}
-{
-   dl-dlsym-2
-   exp-drd:ConflictingAccess
-   obj:/lib*/libc-*.so
-   obj:/lib*/libdl-*.so
-   obj:/lib*/ld-*.so
-}
-{
-   dl-dlsym-3
-   exp-drd:ConflictingAccess
-   obj:/lib*/ld-*.so
-   obj:/lib*/ld-*.so
-   obj:/lib*/libc-*.so
-}
-{
-   dl-dl_addr
-   exp-drd:ConflictingAccess
-   fun:_dl_addr
-}
 {
    libc
    exp-drd:ConflictingAccess