[Bug gdb/26828] SIGSEGV in follow_die_offset dwarf2/read.c:22950

["cvs-commit at gcc dot gnu.org" <sourceware-bugzilla@sourceware.org>]

https://sourceware.org/bugzilla/show_bug.cgi?id=26828

--- Comment #27 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Simon Marchi <simark@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=616c069a3f1a841e5bc63d20aec8e5b71b499f6c

commit 616c069a3f1a841e5bc63d20aec8e5b71b499f6c
Author: Simon Marchi <simon.marchi@polymtl.ca>
Date:   Tue Feb 23 12:07:10 2021 -0500

    gdb/dwarf: don't enqueue CU in maybe_queue_comp_unit if already expanded

    The previous commit log described how items could be left lingering in
    the dwarf2_per_bfd::queue and how that could cause trouble.

    This patch fixes the issue by changing maybe_queue_comp_unit so that it
    doesn't put a CU in the to-expand queue if that CU is already expanded.
    This will make it so that when dwarf2_fetch_die_type_sect_off calls
    follow_die_offset and maybe_queue_comp_unit, it won't enqueue the target
    CU, because it will see the CU is already expanded.

    This assumes that if a CU is dwarf2_fetch_die_type_sect_off's target CU,
    it will have previously been expanded.  I think it is the case, but I
    can't be 100% sure.  If that's not true, the assertions added in the
    following patch will catch it, and it means we'll have to re-think a bit
    more how things work (it wouldn't be well handled at all today anyway).

    This fixes something else in maybe_queue_comp_unit that looks wrong.
    Imagine the DIEs of a CU are loaded in memory, but that CU is not
    expanded.  In that case, maybe_queue_comp_unit will use this early
    return:

      /* If the compilation unit is already loaded, just mark it as
         used.  */
      dwarf2_cu *cu = per_objfile->get_cu (per_cu);
      if (cu != nullptr)
        {
          cu->last_used = 0;
          return 0;
        }

    ... so the CU won't be queued for expansion.  Whether the DIEs of a CU
    are loaded in memory and whether that CU is expanded are two orthogonal
    things, but that function appears to mix them.  So, move the queuing
    above that check / early return, so that if the CU's DIEs are loaded in
    memory but the CU is not expanded yet, it gets enqueued.

    I tried to improve maybe_queue_comp_unit's documentation to clarify what
    the return value means.  By clarifying this, I noticed that two callers
    (follow_die_offset and follow_die_sig_1) access the CU's DIEs after
    calling maybe_queue_comp_unit, only relying on maybe_queue_comp_unit's
    return value to tell whether DIEs need to be loaded first or not.  As
    explained in the new comment, this is problematic:
    maybe_queue_comp_unit's return value doesn't tell whether DIEs are
    currently loaded, it means whether maybe_queue_comp_unit requires the
    caller to load them.  If the CU is already expanded but the DIEs to have
    been freed, maybe_queue_comp_unit returns 0, meaning "I don't need you
    to load the DIEs".  So if these two functions (follow_die_offset and
    follow_die_sig_1) need to access the DIEs in any case, for their own
    usage, they should make sure to load them if they are not loaded
    already.  I therefore added an extra check to the condition they use,
    making it so they will always load the DIEs if they aren't already.

    From what I found, other callers don't care for the CU's DIEs, they call
    maybe_queue_comp_unit to ensure the CU gets expanded eventually, but
    don't care for it after that.

    gdb/ChangeLog:

            PR gdb/26828
            * dwarf2/read.c (maybe_queue_comp_unit): Check if CU is expanded
            to decide whether or not to enqueue it for expansion.
            (follow_die_offset, follow_die_sig_1): Ensure we load the DIEs
            after calling maybe_queue_comp_unit.

    Change-Id: Id98c6b60669f4b4b21b9be16d0518fc62bdf686a

-- 
You are receiving this mail because:
You are on the CC list for the bug.