Follow up for valgrind-3.13.0-accept-read-only-PT_LOAD-segments-and-rodata.patch. From e752326cc050803c3bcfde1f8606bead66ff9642 Mon Sep 17 00:00:00 2001 From: Julian Seward Date: Tue, 14 Aug 2018 10:13:46 +0200 Subject: [PATCH] VG_(di_notify_mmap): once we've read debuginfo for an object, ignore all further mappings. n-i-bz. Once we've read debuginfo for an object, ignore all further mappings. If we don't do that, applications that mmap in their own objects to inspect them for whatever reason, will cause "irrelevant" mappings to be recorded in the object's fsm.maps table. This can lead to serious problems later on. This has become necessary because 64aa729bfae71561505a40c12755bd6b55bb3061 of Thu Jul 12 2018 (the fix for bug 395682) started recording readonly segments in the fsm.maps table, where before they were ignored. --- coregrind/m_debuginfo/debuginfo.c | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/coregrind/m_debuginfo/debuginfo.c b/coregrind/m_debuginfo/debuginfo.c index c36d498..55c05cb 100644 --- a/coregrind/m_debuginfo/debuginfo.c +++ b/coregrind/m_debuginfo/debuginfo.c @@ -1200,6 +1200,32 @@ ULong VG_(di_notify_mmap)( Addr a, Bool allow_SkFileV, Int use_fd ) di = find_or_create_DebugInfo_for( filename ); vg_assert(di); + /* Ignore all mappings for this filename once we've read debuginfo for it. + This avoids the confusion of picking up "irrelevant" mappings in + applications which mmap their objects outside of ld.so, for example + Firefox's Gecko profiler. + + What happens in that case is: the application maps the object "ro" for + whatever reason. We record the mapping di->fsm.maps. The application + later unmaps the object. However, the mapping is not removed from + di->fsm.maps. Later, when some other (unrelated) object is mapped (via + ld.so) into that address space, we first unload any debuginfo that has a + mapping intersecting that area. That means we will end up incorrectly + unloading debuginfo for the object with the "irrelevant" mappings. This + causes various problems, not least because it can unload the debuginfo + for libc.so and so cause malloc intercepts to become un-intercepted. + + This fix assumes that all mappings made once we've read debuginfo for + an object are irrelevant. I think that's OK, but need to check with + 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); + return 0; + } + if (debug) VG_(printf)("di_notify_mmap-4: " "noting details in DebugInfo* at %p\n", di); @@ -1220,7 +1246,8 @@ ULong VG_(di_notify_mmap)( Addr a, Bool allow_SkFileV, Int use_fd ) di->fsm.have_ro_map |= is_ro_map; /* So, finally, are we in an accept state? */ - if (di->fsm.have_rx_map && di->fsm.have_rw_map && !di->have_dinfo) { + vg_assert(!di->have_dinfo); + if (di->fsm.have_rx_map && di->fsm.have_rw_map) { /* Ok, so, finally, we found what we need, and we haven't already read debuginfo for this object. So let's do so now. Yee-ha! */ -- 2.9.3