[Valgrind-developers] [valgrind] Explicitly load libc and any sonames that contain mandatory specs

[Mark W. <ma...@so...>]

https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=8228fe7f696b30c7b6b6daf576fc189bf8d6f8c2

commit 8228fe7f696b30c7b6b6daf576fc189bf8d6f8c2
Author: Mark Wielaard <ma...@kl...>
Date:   Fri Sep 1 19:10:17 2023 +0200

    Explicitly load libc and any sonames that contain mandatory specs
    
    We really need symtab for glibc and ld.so libraries early for redir.
    Some distros move these into separate debuginfo files, which means
    we need to fully load them early.
    
    https://bugs.kde.org/show_bug.cgi?id=473745

Diff:
---
 NEWS                              |  1 +
 coregrind/m_debuginfo/debuginfo.c | 41 ++++++++++++++++++---------------------
 coregrind/m_redir.c               | 23 ++++++++++++++++------
 coregrind/pub_core_debuginfo.h    |  2 ++
 coregrind/pub_core_redir.h        |  2 +-
 5 files changed, 40 insertions(+), 29 deletions(-)

diff --git a/NEWS b/NEWS
index 6f13d53569..832d24e45a 100644
--- a/NEWS
+++ b/NEWS
@@ -52,6 +52,7 @@ are not entered into bugzilla tend to get forgotten about or ignored.
 472963  Broken regular expression in configure.ac
 473604  Fix bug472219.c compile failure with Clang 16
 473677  make check compile failure with Clang 16 based on GCC 13.x
+473745  must-be-redirected function - strlen
 473870  FreeBSD 14 applications fail early at startup
 473944  Handle mold linker split RW PT_LOAD segments correctly
 n-i-bz  Allow arguments with spaces in .valgrindrc files
diff --git a/coregrind/m_debuginfo/debuginfo.c b/coregrind/m_debuginfo/debuginfo.c
index c37e50b9d3..0c4eb99c0d 100644
--- a/coregrind/m_debuginfo/debuginfo.c
+++ b/coregrind/m_debuginfo/debuginfo.c
@@ -1445,27 +1445,34 @@ ULong VG_(di_notify_mmap)( Addr a, Bool allow_SkFileV, Int use_fd )
    }
 }
 
+/* Load DI if it hasn't already been been loaded.  */
+void VG_(di_load_di)( DebugInfo *di )
+{
+   if (di->deferred) {
+      di->deferred = False;
+      ML_(read_elf_debug) (di);
+      ML_(canonicaliseTables)( di );
+
+      /* Check invariants listed in
+         Comment_on_IMPORTANT_REPRESENTATIONAL_INVARIANTS in
+         priv_storage.h. */
+      check_CFSI_related_invariants(di);
+      ML_(finish_CFSI_arrays)(di);
+   }
+}
+
 /* Load DI if it has a text segment containing A and DI hasn't already
    been loaded.  */
 
 void VG_(load_di)( DebugInfo *di, Addr a)
 {
-   if (!di->deferred
-       || !di->text_present
+   if (!di->text_present
        || di->text_size <= 0
        || di->text_avma > a
        || a >= di->text_avma + di->text_size)
       return;
 
-   di->deferred = False;
-   ML_(read_elf_debug) (di);
-   ML_(canonicaliseTables)( di );
-
-   /* Check invariants listed in
-      Comment_on_IMPORTANT_REPRESENTATIONAL_INVARIANTS in
-      priv_storage.h. */
-   check_CFSI_related_invariants(di);
-   ML_(finish_CFSI_arrays)(di);
+   VG_(di_load_di)(di);
 }
 
 /* Attempt to load DebugInfo with a text segment containing A,
@@ -1477,17 +1484,7 @@ void VG_(addr_load_di)( Addr a )
 
    di = VG_(find_DebugInfo)(VG_(current_DiEpoch)(), a);
    if (di != NULL)
-      if (di->deferred) {
-         di->deferred = False;
-         ML_(read_elf_debug) (di);
-         ML_(canonicaliseTables)( di );
-
-         /* Check invariants listed in
-            Comment_on_IMPORTANT_REPRESENTATIONAL_INVARIANTS in
-            priv_storage.h. */
-         check_CFSI_related_invariants(di);
-         ML_(finish_CFSI_arrays)(di);
-      }
+      VG_(di_load_di)(di);
 }
 
 /* Unmap is simpler - throw away any SegInfos intersecting 
diff --git a/coregrind/m_redir.c b/coregrind/m_redir.c
index 37c67f4c13..cef241b4f8 100644
--- a/coregrind/m_redir.c
+++ b/coregrind/m_redir.c
@@ -255,7 +255,7 @@ typedef
 typedef
    struct _TopSpec {
       struct _TopSpec* next; /* linked list */
-      const DebugInfo* seginfo;    /* symbols etc */
+      DebugInfo* seginfo;    /* symbols etc */
       Spec*      specs;      /* specs pulled out of seginfo */
       Bool       mark; /* transient temporary used during deletion */
    }
@@ -312,7 +312,7 @@ static void   show_active ( const HChar* left, const Active* act );
 static void   handle_maybe_load_notifier( const HChar* soname, 
                                           const HChar* symbol, Addr addr );
 
-static void   handle_require_text_symbols ( const DebugInfo* );
+static void   handle_require_text_symbols ( DebugInfo* );
 
 /*------------------------------------------------------------*/
 /*--- NOTIFICATIONS                                        ---*/
@@ -324,7 +324,7 @@ void generate_and_add_actives (
         Spec*    specs, 
         TopSpec* parent_spec,
 	/* debuginfo and the owning TopSpec */
-        const DebugInfo* di,
+        DebugInfo* di,
         TopSpec* parent_sym 
      );
 
@@ -385,7 +385,7 @@ static HChar const* advance_to_comma ( HChar const* c ) {
    topspecs list, and (2) figure out what new binding are now active,
    and, as a result, add them to the actives mapping. */
 
-void VG_(redir_notify_new_DebugInfo)( const DebugInfo* newdi )
+void VG_(redir_notify_new_DebugInfo)( DebugInfo* newdi )
 {
    Bool         ok, isWrap, isGlobal;
    Int          i, nsyms, becTag, becPrio;
@@ -421,6 +421,12 @@ void VG_(redir_notify_new_DebugInfo)( const DebugInfo* newdi )
    newdi_soname = VG_(DebugInfo_get_soname)(newdi);
    vg_assert(newdi_soname != NULL);
 
+   /* libc is special, because it contains some of the core redirects.
+      Make sure it is fully loaded.  */
+   if (0 == VG_(strcmp)(newdi_soname, libc_soname) ||
+       0 == VG_(strcmp)(newdi_soname, pthread_soname))
+      VG_(di_load_di)(newdi);
+
 #ifdef ENABLE_INNER
    {
       /* When an outer Valgrind is executing an inner Valgrind, the
@@ -814,7 +820,7 @@ void generate_and_add_actives (
         Spec*    specs, 
         TopSpec* parent_spec,
 	/* seginfo and the owning TopSpec */
-        const DebugInfo* di,
+        DebugInfo* di,
         TopSpec* parent_sym 
      )
 {
@@ -846,6 +852,11 @@ void generate_and_add_actives (
 
       sp->mark = VG_(string_match)( sp->from_sopatt, soname );
       anyMark = anyMark || sp->mark;
+
+      /* The symtab might be in a separate debuginfo file. Make sure the
+        debuginfo is fully loaded.  */
+      if (sp->mark && sp->mandatory)
+         VG_(di_load_di)(di);
    }
 
    /* shortcut: if none of the sonames match, there will be no bindings. */
@@ -1792,7 +1803,7 @@ void handle_maybe_load_notifier( const HChar* soname,
    symbols that satisfy any --require-text-symbol= specifications that
    apply to it, and abort the run with an error message if not.
 */
-static void handle_require_text_symbols ( const DebugInfo* di )
+static void handle_require_text_symbols ( DebugInfo* di )
 {
    /* First thing to do is figure out which, if any,
       --require-text-symbol specification strings apply to this
diff --git a/coregrind/pub_core_debuginfo.h b/coregrind/pub_core_debuginfo.h
index 6e93bb93c5..4d6ebda816 100644
--- a/coregrind/pub_core_debuginfo.h
+++ b/coregrind/pub_core_debuginfo.h
@@ -78,6 +78,8 @@ extern void VG_(di_notify_vm_protect)( Addr a, SizeT len, UInt prot );
 
 extern void VG_(addr_load_di)( Addr a );
 
+extern void VG_(di_load_di)( DebugInfo *di );
+
 extern void VG_(load_di)( DebugInfo *di, Addr a );
 
 extern void VG_(di_discard_ALL_debuginfo)( void );
diff --git a/coregrind/pub_core_redir.h b/coregrind/pub_core_redir.h
index b88ca98f92..0cf0b009ea 100644
--- a/coregrind/pub_core_redir.h
+++ b/coregrind/pub_core_redir.h
@@ -61,7 +61,7 @@
 //--------------------------------------------------------------------
 
 /* Notify the module of a new DebugInfo (called from m_debuginfo). */
-extern void VG_(redir_notify_new_DebugInfo)( const DebugInfo* );
+extern void VG_(redir_notify_new_DebugInfo)( DebugInfo* );
 
 /* Notify the module of the disappearance of a DebugInfo (also called
    from m_debuginfo). */