Fix 393146 failing assert "is_DebugInfo_active(di)"

Some applications are mapping an object ro, and then unmaps it directly.
In such a case, we have a di that contains obsolete fsm.maps (not matching
OS mappings). The di for this unmapped object is not active,
and has no dinfo (have_dinfo == False).
(more generally, fsm.maps can contain a whole bunch of obsolete mappings).

Later on, some other libs can be mapped with a mapping overlapping
this obsolete mapping.

A di that never had its debug info loaded can really be discarded,
even if CG_(clo_keep_debuginfo).
In such a case, it is normal to have to discard a not active di.

(it might be better to keep fsm.maps in sync with the real OS
mapping, but that is a much bigger change/fix).

The FSM debug tracing was static, it is now dynamic according
to debug loglevel >= 3.

The below is an extract of the trace showing what happens.

SYSCALL[4384,1](257) sys_openat ( 4294967196, 0x4244398(/usr/lib/x86_64-linux-gnu/qt5/plugins/platforms/libqeglfs.so), 524288 ) --> [async] ...
SYSCALL[4384,1](257) ... [async] --> Success(0x3)
SYSCALL[4384,1](72) sys_fcntl[ARG3=='arg'] ( 3, 2, 1 )[sync] --> Success(0x0)
SYSCALL[4384,1](5) sys_newfstat ( 3, 0x1ffefff8b0 )[sync] --> Success(0x0)
SYSCALL[4384,1](5) sys_newfstat ( 3, 0x1ffefff9c0 )[sync] --> Success(0x0)
SYSCALL[4384,1](9) sys_mmap ( 0x0, 10520, 1, 1, 3, 0 )--4384-- di_notify_mmap-0:
--4384-- di_notify_mmap-1: 0x4027000-0x4029fff r--
--4384-- di_notify_mmap-2: /usr/lib/x86_64-linux-gnu/qt5/plugins/platforms/libqeglfs.so
--4384-- di_notify_mmap-3: is_rx_map 0, is_rw_map 0, is_ro_map 1
--4384-- di_notify_mmap-4: noting details in DebugInfo* at 0x10024CEA10
--4384-- di_notify_mmap-6: no dinfo loaded /usr/lib/x86_64-linux-gnu/qt5/plugins/platforms/libqeglfs.so (no rx or no rw mapping)
 --> [pre-success] Success(0x4027000)
SYSCALL[4384,1](3) sys_close ( 3 )[sync] --> Success(0x0)
SYSCALL[4384,1](11) sys_munmap ( 0x4027000, 10520 )[sync] --> Success(0x0)
  ^^^^ the above munmap has not cleaned up or removed anything in DebugInfo* at 0x10024CEA10

Later on, /usr/lib/x86_64-linux-gnu/qt5/plugins/platforms/libqxcb.so is mapped
overlapping the memory where libqeglfs.so was mapped ro.

Now, this cleans up the (useless) di that never had have_dinfo true, e.g.

------ start ELF OBJECT -------------------------------------------------------
------ name = /usr/lib/x86_64-linux-gnu/qt5/plugins/platforms/libqxcb.so
...
--4384-- Discarding syms at 0x0-0x0 in /usr/lib/x86_64-linux-gnu/qt5/plugins/platforms/libqeglfs.so (have_dinfo 0)
(the 0x0-0x0 in the trace is because there was never any text mapping for libqeglfs.so).

diff --git a/NEWS b/NEWS
index e14e44c245ab90036a138baf2db671647cf25707..bd3c61d3e615b4f8c3a4e1002efe212d86560e90 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -136,6 +136,7 @@ where XXXXXX is the bug number as listed below.
 393023  callgrind_control risks using the wrong vgdb
 393062  build-id ELF phdrs read causes "debuginfo reader: ensure_valid failed"
 393099  posix_memalign() invalid write if alignment == 0
+393146  failing assert "is_DebugInfo_active(di)"
 395709  PPC64 is missing support for the xvnegsp instruction
 395682  Accept read-only PT_LOAD segments and .rodata by ld -z separate-code
         == 384727

diff --git a/coregrind/m_debuginfo/debuginfo.c b/coregrind/m_debuginfo/debuginfo.c
index a30c7b44e51385bf3cb89dcc9643f8e1dd57cd74..0fc54bf1dd17391d1a4f9473d60b07bc77519396 100644 (file)
--- a/coregrind/m_debuginfo/debuginfo.c
+++ b/coregrind/m_debuginfo/debuginfo.c
@@ -34,6 +34,7 @@
 #include "pub_core_vki.h"
 #include "pub_core_threadstate.h"
 #include "pub_core_debuginfo.h"  /* self */
+#include "pub_core_debuglog.h"
 #include "pub_core_demangle.h"
 #include "pub_core_libcbase.h"
 #include "pub_core_libcassert.h"
@@ -68,10 +69,6 @@
 #endif
 
 
-/* Set this to 1 to enable debug printing for the
-   should-we-load-debuginfo-now? finite state machine. */
-#define DEBUG_FSM 0
-
 /* Set this to 1 to enable somewhat minimal debug printing for the
    debuginfo-epoch machinery. */
 #define DEBUG_EPOCHS 0
@@ -443,27 +440,31 @@ static void free_DebugInfo ( DebugInfo* di )
 */
 static void discard_or_archive_DebugInfo ( DebugInfo* di )
 {
-   const HChar* reason  = "munmap";
-   const Bool   archive = VG_(clo_keep_debuginfo);
+   /*  di->have_dinfo can be False when an object is mapped "ro"
+       and then unmapped before the debug info is loaded.
+       In other words, debugInfo_list might contain many di that have
+       no OS mappings, even if their fsm.maps still contain mappings.
+       Such (left over) mappings can overlap with real mappings.
+       Search for FSMMAPSNOTCLEANEDUP: below for more details. */
+   /* If a di has no dinfo, we can discard even if VG_(clo_keep_debuginfo). */
+   const Bool   archive = VG_(clo_keep_debuginfo) && di->have_dinfo;
 
    DebugInfo** prev_next_ptr = &debugInfo_list;
    DebugInfo*  curr          =  debugInfo_list;
 
-   /* It must be active! */
-   vg_assert(is_DebugInfo_active(di));
+   /* If di->have_dinfo, then it must be active! */
+   vg_assert(!di->have_dinfo || is_DebugInfo_active(di));
    while (curr) {
       if (curr == di) {
          /* Found it; (remove from list and free it), or archive it. */
-         if (curr->have_dinfo
-             && (VG_(clo_verbosity) > 1 || VG_(clo_trace_redir)))
-            VG_(message)(Vg_DebugMsg, 
-                         "%s syms at %#lx-%#lx in %s due to %s()\n",
-                         archive ? "Archiving" : "Discarding",
-                         di->text_avma, 
-                         di->text_avma + di->text_size,
-                         curr->fsm.filename ? curr->fsm.filename
-                                            : "???",
-                         reason);
+         if (VG_(clo_verbosity) > 1 || VG_(clo_trace_redir))
+            VG_(dmsg)("%s syms at %#lx-%#lx in %s (have_dinfo %d)\n",
+                      archive ? "Archiving" : "Discarding",
+                      di->text_avma, 
+                      di->text_avma + di->text_size,
+                      curr->fsm.filename ? curr->fsm.filename
+                                           : "???",
+                      curr->have_dinfo);
          vg_assert(*prev_next_ptr == curr);
          if (!archive) {
             *prev_next_ptr = curr->next;
@@ -1021,7 +1022,7 @@ ULong VG_(di_notify_mmap)( Addr a, Bool allow_SkFileV, Int use_fd )
    Int        actual_fd, oflags;
    SysRes     preadres;
    HChar      buf1k[1024];
-   Bool       debug = (DEBUG_FSM != 0);
+   const Bool       debug = VG_(debugLog_getLevel)() >= 3;
    SysRes     statres;
    struct vg_stat statbuf;
 
@@ -1034,11 +1035,11 @@ ULong VG_(di_notify_mmap)( Addr a, Bool allow_SkFileV, Int use_fd )
    vg_assert(seg);
 
    if (debug) {
-      VG_(printf)("di_notify_mmap-0:\n");
-      VG_(printf)("di_notify_mmap-1: %#lx-%#lx %c%c%c\n",
-                  seg->start, seg->end, 
-                  seg->hasR ? 'r' : '-',
-                  seg->hasW ? 'w' : '-',seg->hasX ? 'x' : '-' );
+      VG_(dmsg)("di_notify_mmap-0:\n");
+      VG_(dmsg)("di_notify_mmap-1: %#lx-%#lx %c%c%c\n",
+                seg->start, seg->end, 
+                seg->hasR ? 'r' : '-',
+                seg->hasW ? 'w' : '-',seg->hasX ? 'x' : '-' );
    }
 
    /* guaranteed by aspacemgr-linux.c, sane_NSegment() */
@@ -1064,7 +1065,7 @@ ULong VG_(di_notify_mmap)( Addr a, Bool allow_SkFileV, Int use_fd )
       return 0;
 
    if (debug)
-      VG_(printf)("di_notify_mmap-2: %s\n", filename);
+      VG_(dmsg)("di_notify_mmap-2: %s\n", filename);
 
    /* Only try to read debug information from regular files.  */
    statres = VG_(stat)(filename, &statbuf);
@@ -1167,9 +1168,9 @@ ULong VG_(di_notify_mmap)( Addr a, Bool allow_SkFileV, Int use_fd )
 #  endif
 
    if (debug)
-      VG_(printf)("di_notify_mmap-3: "
-                  "is_rx_map %d, is_rw_map %d, is_ro_map %d\n",
-                  (Int)is_rx_map, (Int)is_rw_map, (Int)is_ro_map);
+      VG_(dmsg)("di_notify_mmap-3: "
+                "is_rx_map %d, is_rw_map %d, is_ro_map %d\n",
+                (Int)is_rx_map, (Int)is_rw_map, (Int)is_ro_map);
 
    /* Ignore mappings with permissions we can't possibly be interested in. */
    if (!(is_rx_map || is_rw_map || is_ro_map))
@@ -1253,15 +1254,15 @@ ULong VG_(di_notify_mmap)( Addr a, Bool allow_SkFileV, Int use_fd )
       mjw/thh.  */
    if (di->have_dinfo) {
       if (debug)
-         VG_(printf)("di_notify_mmap-4x: "
-                     "ignoring mapping because we already read debuginfo "
-                     "for DebugInfo* %p\n", di);
+         VG_(dmsg)("di_notify_mmap-4x: "
+                   "ignoring mapping because we already read debuginfo "
+                   "for DebugInfo* %p\n", di);
       return 0;
    }
 
    if (debug)
-      VG_(printf)("di_notify_mmap-4: "
-                  "noting details in DebugInfo* at %p\n", di);
+      VG_(dmsg)("di_notify_mmap-4: "
+                "noting details in DebugInfo* at %p\n", di);
 
    /* Note the details about the mapping. */
    DebugInfoMapping map;
@@ -1285,13 +1286,14 @@ ULong VG_(di_notify_mmap)( Addr a, Bool allow_SkFileV, Int use_fd )
          already read debuginfo for this object.  So let's do so now.
          Yee-ha! */
       if (debug)
-         VG_(printf)("di_notify_mmap-5: "
-                     "achieved accept state for %s\n", filename);
+         VG_(dmsg)("di_notify_mmap-5: "
+                   "achieved accept state for %s\n", filename);
       return di_notify_ACHIEVE_ACCEPT_STATE ( di );
    } else {
-      /* If we don't have an rx and rw mapping, or if we already have
-         debuginfo for this mapping for whatever reason, go no
-         further. */
+      /* If we don't have an rx and rw mapping, go no further. */
+      if (debug)
+         VG_(dmsg)("di_notify_mmap-6: "
+                   "no dinfo loaded %s (no rx or no rw mapping)\n", filename);
       return 0;
    }
 }
@@ -1336,16 +1338,16 @@ void VG_(di_notify_mprotect)( Addr a, SizeT len, UInt prot )
    declaration of struct _DebugInfoFSM for details. */
 void VG_(di_notify_vm_protect)( Addr a, SizeT len, UInt prot )
 {
-   Bool debug = (DEBUG_FSM != 0);
+   const Bool debug = VG_(debugLog_getLevel)() >= 3;
 
    Bool r_ok = toBool(prot & VKI_PROT_READ);
    Bool w_ok = toBool(prot & VKI_PROT_WRITE);
    Bool x_ok = toBool(prot & VKI_PROT_EXEC);
    if (debug) {
-      VG_(printf)("di_notify_vm_protect-0:\n");
-      VG_(printf)("di_notify_vm_protect-1: %#lx-%#lx %c%c%c\n",
-                  a, a + len - 1,
-                  r_ok ? 'r' : '-', w_ok ? 'w' : '-', x_ok ? 'x' : '-' );
+      VG_(dmsg)("di_notify_vm_protect-0:\n");
+      VG_(dmsg)("di_notify_vm_protect-1: %#lx-%#lx %c%c%c\n",
+                a, a + len - 1,
+                r_ok ? 'r' : '-', w_ok ? 'w' : '-', x_ok ? 'x' : '-' );
    }
 
    Bool do_nothing = True;
@@ -1354,8 +1356,8 @@ void VG_(di_notify_vm_protect)( Addr a, SizeT len, UInt prot )
 #  endif
    if (do_nothing /* wrong platform */) {
       if (debug)
-         VG_(printf)("di_notify_vm_protect-2: wrong platform, "
-                     "doing nothing.\n");
+         VG_(dmsg)("di_notify_vm_protect-2: wrong platform, "
+                   "doing nothing.\n");
       return;
    }
 
@@ -1367,7 +1369,7 @@ void VG_(di_notify_vm_protect)( Addr a, SizeT len, UInt prot )
       is found, conclude we're in an accept state and read debuginfo
       accordingly. */
    if (debug)
-      VG_(printf)("di_notify_vm_protect-3: looking for existing DebugInfo*\n");
+      VG_(dmsg)("di_notify_vm_protect-3: looking for existing DebugInfo*\n");
    DebugInfo* di;
    DebugInfoMapping *map = NULL;
    Word i;
@@ -1397,8 +1399,8 @@ void VG_(di_notify_vm_protect)( Addr a, SizeT len, UInt prot )
       return; /* didn't find anything */
 
    if (debug)
-     VG_(printf)("di_notify_vm_protect-4: found existing DebugInfo* at %p\n",
-                 di);
+     VG_(dmsg)("di_notify_vm_protect-4: found existing DebugInfo* at %p\n",
+               di);
 
    /* Do the upgrade.  Simply update the flags of the mapping
       and pretend we never saw the RO map at all. */
@@ -1419,7 +1421,7 @@ void VG_(di_notify_vm_protect)( Addr a, SizeT len, UInt prot )
    /* Check if we're now in an accept state and read debuginfo.  Finally. */
    if (di->fsm.have_rx_map && di->fsm.have_rw_map && !di->have_dinfo) {
       if (debug)
-         VG_(printf)("di_notify_vm_protect-5: "
+         VG_(dmsg)("di_notify_vm_protect-5: "
                      "achieved accept state for %s\n", di->fsm.filename);
       ULong di_handle __attribute__((unused))
          = di_notify_ACHIEVE_ACCEPT_STATE( di );