https://bugs.kde.org/show_bug.cgi?id=452802

Mark Wielaard <m...@klomp.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |m...@klomp.org

--- Comment #15 from Mark Wielaard <m...@klomp.org> ---
I wish I understood this part of the code better. But I also struggling
a bit. Let me try to talk us through the patch to see if we agree on
the why/what:

di_notify_mmap is triggered by an actual mmap (and some startup code,
for valgrind tools itself?). The problem we are dealing with is when
"enough" or "the right" parts of the file have been mapped by ld.so to
start searching for the debuginfo and setting up the offsets to map
debug addresses to addresses in memory where the code is mapped.

The goal of di_notify_mmap is to reach di_notify_ACHIEVE_ACCEPT_STATE
at which point the actual debuginfo is read. This is guarded by:

  if (di->fsm.have_rx_map && di->fsm.have_rw_map)

You patch changes that to:

   if (di->fsm.have_rx_map &&
       rw_load_count >= 1 &&
       di->fsm.have_rw_map == rw_load_count)

Now have_rw_map is slightly misnamed, but ok.

The rw_load_count comes from the new check_elf_and_get_rw_loads.
Which tries to get an elf image from the file descriptor and traverses
the phdrs looking for PT_LOAD RW mappings, with this funny counter
example:

   /*
    * Hold your horses
    * Just because The ELF file contains 2 RW PT_LOAD segments it
    * doesn't mean that Valgrind will also make 2 calls to
    * VG_(di_notify_mmap). If the stars are all aligned
    * (which usually means that the ELF file is the client
    * executable with the segment offset for the
    * second PT_LOAD falls exactly on 0x1000) then the NSegements
    * will get merged and VG_(di_notify_mmap) only gets called once. */
   if (*rw_load_count == 2 &&
       ehdr_m.e_type == ET_EXEC &&
       a_phdr.p_offset == VG_PGROUNDDN(a_phdr.p_offset) )
   {
      *rw_load_count = 1;
   }

I assume in that case ld.so merges the mmap request into one?

I cannot say I really understand everything here, but I understand why
you want to delay to actual debuginfo loading till all mapping are
there.

Is the above an accurate description of the proposed patch logic?

-- 
You are receiving this mail because:
You are watching all bug changes.

Reply via email to