gdb/dwarf: don't enqueue CU in maybe_queue_comp_unit if already expanded
authorSimon Marchi <simon.marchi@polymtl.ca>
Tue, 23 Feb 2021 17:07:10 +0000 (12:07 -0500)
committerSimon Marchi <simon.marchi@polymtl.ca>
Tue, 23 Feb 2021 17:07:10 +0000 (12:07 -0500)
commit616c069a3f1a841e5bc63d20aec8e5b71b499f6c
tree986c3d3d74c68d4df4e72368f89f54e1561cc5f7
parent1a48f0027d52f507da1817406279c1b81a6fa262
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
gdb/ChangeLog
gdb/dwarf2/read.c