diff mbox

possible fix for PR symtab/23010

Message ID 87po34kzxh.fsf@tromey.com
State New, archived
Headers

Commit Message

Tom Tromey April 12, 2018, 7:06 p.m. UTC 
  I was talking with Keith & Pedro about PR symtab/23010 on irc this week,
because it was the bug at the base of the Rust -readnow problem that Jan
reported.

I came up with this patch yesterday.  It fixes the problem, but I didn't
write a test and also I'm not sure if it is the best way to go.

So, I thought I'd send it for commentary.

Tom

commit bc5411ff15de7e207bfb80a42790adb5d6f80852
Author: Tom Tromey <tom@tromey.com>
Date:   Thu Apr 12 08:24:41 2018 -0600

    Fix for dwz-related crash
    
    PR symtab/23010 reports a crash that occurs when using -readnow
    on a dwz-generated debuginfo file.
    
    The crash occurs because the DWARF has a partial CU with no language
    set, and then a full CU that references this partial CU using
    DW_AT_abstract_origin.
    
    In this case, the partial CU is read by dw2_expand_all_symtabs using
    language_minimal; but then this conflicts with the creation of the
    block's symbol table in the C++ CU.
    
    This patch fixes the problem by arranging for partial CUs not to be
    read by -readnow.  I tend to think that it doesn't make sense to read
    a partial CU in isolation -- they should only be read when imported
    into some other CU.
    
    In conjunction with some other patches I am going to post, this also
    fixes the Rust -readnow crash that Jan reported.
    
    There are two problems with this patch:
    
    1. It is difficult to reason about.  There are many cases where I've
       patched the code to call init_cutu_and_read_dies with the flag set
       to "please do read partial units" -- but I find it difficult to be
       sure that this is always correct.
    
    2. It is still missing a standalone test case.  This seemed hard.
    
    2018-04-12  Tom Tromey  <tom@tromey.com>
    
            PR symtab/23010:
            * dwarf2read.c (load_cu, dw2_do_instantiate_symtab)
            (dw2_instantiate_symtab): Add skip_partial parameter.
            (dw2_find_last_source_symtab, dw2_map_expand_apply)
            (dw2_lookup_symbol, dw2_expand_symtabs_for_function)
            (dw2_expand_all_symtabs, dw2_expand_symtabs_with_fullname)
            (dw2_expand_symtabs_matching_one)
            (dw2_find_pc_sect_compunit_symtab)
            (dw2_debug_names_lookup_symbol)
            (dw2_debug_names_expand_symtabs_for_function): Update.
            (init_cutu_and_read_dies): Add skip_partial parameter.
            (process_psymtab_comp_unit, build_type_psymtabs_1)
            (process_skeletonless_type_unit, load_partial_comp_unit)
            (psymtab_to_symtab_1): Update.
            (load_full_comp_unit): Add skip_partial parameter.
            (process_imported_unit_die, dwarf2_read_addr_index)
            (follow_die_offset, dwarf2_fetch_die_loc_sect_off)
            (dwarf2_fetch_constant_bytes, dwarf2_fetch_die_type_sect_off)
            (read_signatured_type): Update.

Comments

Keith Seitz April 17, 2018, 7:17 p.m. UTC | #1 
On 04/12/2018 12:06 PM, Tom Tromey wrote:
> I was talking with Keith & Pedro about PR symtab/23010 on irc this week,
> because it was the bug at the base of the Rust -readnow problem that Jan
> reported.
> 
> I came up with this patch yesterday.  It fixes the problem, but I didn't
> write a test and also I'm not sure if it is the best way to go.
> 
> So, I thought I'd send it for commentary.

I've looked into this quite a bit, too. In my case, I was looking specifically at the assertion caused by passing "-readnow" with libwebkit.so.debug on Fedora (Jan's reproducer).

I tried for darn near a week to come up with an isolated reproducer to no avail. :-(

Based on what I was seeing, I came up with a different approach, but I don't care for it at all. I find the proposed solution a whole lot less risky. [My approach was to have language_minimal CUs "inherit" the importing CU's language in inherit_abstract_dies. I can dream up a few instances where this might be problematic. I don't know if they're really legitimate use cases, but nothing in the standard specifically discredits my imaginings.]

>     There are two problems with this patch:
>     
>     1. It is difficult to reason about.  There are many cases where I've
>        patched the code to call init_cutu_and_read_dies with the flag set
>        to "please do read partial units" -- but I find it difficult to be
>        sure that this is always correct.
>     
>     2. It is still missing a standalone test case.  This seemed hard.

It is. :-)

>     2018-04-12  Tom Tromey  <tom@tromey.com>
>     
>             PR symtab/23010:
>             * dwarf2read.c (load_cu, dw2_do_instantiate_symtab)
>             (dw2_instantiate_symtab): Add skip_partial parameter.
>             (dw2_find_last_source_symtab, dw2_map_expand_apply)
>             (dw2_lookup_symbol, dw2_expand_symtabs_for_function)
>             (dw2_expand_all_symtabs, dw2_expand_symtabs_with_fullname)
>             (dw2_expand_symtabs_matching_one)
>             (dw2_find_pc_sect_compunit_symtab)
>             (dw2_debug_names_lookup_symbol)
>             (dw2_debug_names_expand_symtabs_for_function): Update.
>             (init_cutu_and_read_dies): Add skip_partial parameter.
>             (process_psymtab_comp_unit, build_type_psymtabs_1)
>             (process_skeletonless_type_unit, load_partial_comp_unit)
>             (psymtab_to_symtab_1): Update.
>             (load_full_comp_unit): Add skip_partial parameter.
>             (process_imported_unit_die, dwarf2_read_addr_index)
>             (follow_die_offset, dwarf2_fetch_die_loc_sect_off)
>             (dwarf2_fetch_constant_bytes, dwarf2_fetch_die_type_sect_off)
>             (read_signatured_type): Update.

Aside: didn't we decide that "update all callers" was sufficient? [I'm only curious -- not asking for changes to a silly ChangeLog.]

This patch looks reasonable to me, but I would ask you to consider mentioning why partial_units are skipped where they are (even if to just reference the problem or bug?). That's these two hunks, I think:

> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
> index e3f544a6a4..406aa0d52e 100644
> --- a/gdb/dwarf2read.c
> +++ b/gdb/dwarf2read.c
> @@ -2870,7 +2870,7 @@ dw2_do_instantiate_symtab (struct dwarf2_per_cu_data *per_cu)
>        : (per_cu->v.psymtab == NULL || !per_cu->v.psymtab->readin))
>      {
>        queue_comp_unit (per_cu, language_minimal);
> -      load_cu (per_cu);
> +      load_cu (per_cu, true);
>  
>        /* If we just loaded a CU from a DWO, and we're working with an index
>  	 that may badly handle TUs, load all the TUs in that DWO as well.

and

> @@ -4144,7 +4144,7 @@ dw2_expand_all_symtabs (struct objfile *objfile)
>      {
>        dwarf2_per_cu_data *per_cu = dwarf2_per_objfile->get_cutu (i);
>  
> -      dw2_instantiate_symtab (per_cu);
> +      dw2_instantiate_symtab (per_cu, true);
>      }
>  }
>  

I've been manually testing this patch with everything that I can think of on libwebkit.so, and I cannot trigger anything ill-behaved at all.

I recommend a maintainer approve this, even if it is more a band-aid than a "proper" fix. It fixes a fairly big (and maybe even common) problem with minimal impact/risk.

Keith
Tom Tromey April 19, 2018, 8:39 p.m. UTC | #2 
>>>>> "Keith" == Keith Seitz <keiths@redhat.com> writes:

Keith> I tried for darn near a week to come up with an isolated
Keith> reproducer to no avail. :-(

If you have a test case, could you send it, even it if doesn't work?
I could try to poke at it a little.

Keith> Aside: didn't we decide that "update all callers" was sufficient? [I'm
Keith> only curious -- not asking for changes to a silly ChangeLog.]

Yeah, old habits I guess.

Keith> This patch looks reasonable to me, but I would ask you to consider
Keith> mentioning why partial_units are skipped where they are (even if to
Keith> just reference the problem or bug?). That's these two hunks, I think:

Good idea, will do.

Tom
Joel Brobecker April 30, 2018, 10:44 p.m. UTC | #3 
Hello,


> > I was talking with Keith & Pedro about PR symtab/23010 on irc this week,
> > because it was the bug at the base of the Rust -readnow problem that Jan
> > reported.
> > 
> > I came up with this patch yesterday.  It fixes the problem, but I didn't
> > write a test and also I'm not sure if it is the best way to go.
> > 
> > So, I thought I'd send it for commentary.

I am not sure either, but I can't think of anything else.

> >     2018-04-12  Tom Tromey  <tom@tromey.com>
> >     
> >             PR symtab/23010:
> >             * dwarf2read.c (load_cu, dw2_do_instantiate_symtab)
> >             (dw2_instantiate_symtab): Add skip_partial parameter.
> >             (dw2_find_last_source_symtab, dw2_map_expand_apply)
> >             (dw2_lookup_symbol, dw2_expand_symtabs_for_function)
> >             (dw2_expand_all_symtabs, dw2_expand_symtabs_with_fullname)
> >             (dw2_expand_symtabs_matching_one)
> >             (dw2_find_pc_sect_compunit_symtab)
> >             (dw2_debug_names_lookup_symbol)
> >             (dw2_debug_names_expand_symtabs_for_function): Update.
> >             (init_cutu_and_read_dies): Add skip_partial parameter.
> >             (process_psymtab_comp_unit, build_type_psymtabs_1)
> >             (process_skeletonless_type_unit, load_partial_comp_unit)
> >             (psymtab_to_symtab_1): Update.
> >             (load_full_comp_unit): Add skip_partial parameter.
> >             (process_imported_unit_die, dwarf2_read_addr_index)
> >             (follow_die_offset, dwarf2_fetch_die_loc_sect_off)
> >             (dwarf2_fetch_constant_bytes, dwarf2_fetch_die_type_sect_off)
> >             (read_signatured_type): Update.
[...]
> This patch looks reasonable to me, but I would ask you to consider
> mentioning why partial_units are skipped where they are (even if to
> just reference the problem or bug?). That's these two hunks, I think:

+1.

> I've been manually testing this patch with everything that I can think
> of on libwebkit.so, and I cannot trigger anything ill-behaved at all.
> 
> I recommend a maintainer approve this, even if it is more a band-aid
> than a "proper" fix. It fixes a fairly big (and maybe even common)
> problem with minimal impact/risk.

I reviewed the patch as best as I could, but as Tom says, it's hard
to reason. But at the same time, it was conservative, as the new
param is false by default except in a couple of cases.

I'd love for another maintainer to take a look, especially if we are
going to consider this patch for inclusion in 8.1.1. But I can't
think of someone who was actively involved in this area.

Considering the fact that this has had two reviews, and also that
it comes from you, whom I trust quite a bit for changes in this area,
let's give others a week to provide comments. Failing that, let's
push it, to see how well it fares.

We may decide to skip this bug for 8.1.1, though. Although, thinking
aloud, if there was any regression caused by it, it would be with units
which haven't been expanded yet, right? A workaround would be to trigger
the unit's expansion, which seems easy enough. So, small risk vs
no-crash reward.... Hmmm...
Joel Brobecker May 7, 2018, 5:13 p.m. UTC | #4 
Hello Global Maintainers,

I was wondering if anyone had any thoughts regarding Tom patch.
https://sourceware.org/ml/gdb-patches/2018-04/msg00234.html

Below are my comments on it, and also my interrogations on whether
we might want this patch in 8.1.1 or not.

Additional thoughts:
  - This is a regression
  - This is an internal error, so it can be fairly problematic
  - It only happens with -readnow, it seems, which I assume
    is not widely used considering the performance and memory
    cost of this feature.

I might tip in favor of putting it in, considering the fact that
I don't think there is much of a workaround, but I would not make
that call just on my own, because the patch is far from obvious.


> > >     2018-04-12  Tom Tromey  <tom@tromey.com>
> > >     
> > >             PR symtab/23010:
> > >             * dwarf2read.c (load_cu, dw2_do_instantiate_symtab)
> > >             (dw2_instantiate_symtab): Add skip_partial parameter.
> > >             (dw2_find_last_source_symtab, dw2_map_expand_apply)
> > >             (dw2_lookup_symbol, dw2_expand_symtabs_for_function)
> > >             (dw2_expand_all_symtabs, dw2_expand_symtabs_with_fullname)
> > >             (dw2_expand_symtabs_matching_one)
> > >             (dw2_find_pc_sect_compunit_symtab)
> > >             (dw2_debug_names_lookup_symbol)
> > >             (dw2_debug_names_expand_symtabs_for_function): Update.
> > >             (init_cutu_and_read_dies): Add skip_partial parameter.
> > >             (process_psymtab_comp_unit, build_type_psymtabs_1)
> > >             (process_skeletonless_type_unit, load_partial_comp_unit)
> > >             (psymtab_to_symtab_1): Update.
> > >             (load_full_comp_unit): Add skip_partial parameter.
> > >             (process_imported_unit_die, dwarf2_read_addr_index)
> > >             (follow_die_offset, dwarf2_fetch_die_loc_sect_off)
> > >             (dwarf2_fetch_constant_bytes, dwarf2_fetch_die_type_sect_off)
> > >             (read_signatured_type): Update.
> [...]
> > This patch looks reasonable to me, but I would ask you to consider
> > mentioning why partial_units are skipped where they are (even if to
> > just reference the problem or bug?). That's these two hunks, I think:
> 
> +1.
> 
> > I've been manually testing this patch with everything that I can think
> > of on libwebkit.so, and I cannot trigger anything ill-behaved at all.
> > 
> > I recommend a maintainer approve this, even if it is more a band-aid
> > than a "proper" fix. It fixes a fairly big (and maybe even common)
> > problem with minimal impact/risk.
> 
> I reviewed the patch as best as I could, but as Tom says, it's hard
> to reason. But at the same time, it was conservative, as the new
> param is false by default except in a couple of cases.
> 
> I'd love for another maintainer to take a look, especially if we are
> going to consider this patch for inclusion in 8.1.1. But I can't
> think of someone who was actively involved in this area.
> 
> Considering the fact that this has had two reviews, and also that
> it comes from you, whom I trust quite a bit for changes in this area,
> let's give others a week to provide comments. Failing that, let's
> push it, to see how well it fares.
> 
> We may decide to skip this bug for 8.1.1, though. Although, thinking
> aloud, if there was any regression caused by it, it would be with units
> which haven't been expanded yet, right? A workaround would be to trigger
> the unit's expansion, which seems easy enough. So, small risk vs
> no-crash reward.... Hmmm...
> 
> -- 
> Joel
Joel Brobecker May 14, 2018, 5:38 p.m. UTC | #5 
Hi Tom,

On Mon, May 07, 2018 at 10:13:09AM -0700, Joel Brobecker wrote:
> Hello Global Maintainers,
> 
> I was wondering if anyone had any thoughts regarding Tom patch.
> https://sourceware.org/ml/gdb-patches/2018-04/msg00234.html
> 
> Below are my comments on it, and also my interrogations on whether
> we might want this patch in 8.1.1 or not.
> 
> Additional thoughts:
>   - This is a regression
>   - This is an internal error, so it can be fairly problematic
>   - It only happens with -readnow, it seems, which I assume
>     is not widely used considering the performance and memory
>     cost of this feature.
> 
> I might tip in favor of putting it in, considering the fact that
> I don't think there is much of a workaround, but I would not make
> that call just on my own, because the patch is far from obvious.

Considering that the patch has been under review for 1 month, and
has been available for additional comments for a couple of weeks
since my review, and that this patch is a potential for inclusion
in 8.1.1, I propose we rebase, make whatever tiny adjustments that
were discussed, and start by pushing it to master.

As mentioned in my weekly update of the 8.1.1 release, we can then
wait a week or two before deciding whether we want it in 8.1.1 or not.

WDYT?

Thanks!
Tom Tromey May 17, 2018, 4:19 p.m. UTC | #6 
>>>>> "Keith" == Keith Seitz <keiths@redhat.com> writes:

Keith> This patch looks reasonable to me, but I would ask you to consider
Keith> mentioning why partial_units are skipped where they are (even if to
Keith> just reference the problem or bug?). That's these two hunks, I think:

>> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
>> index e3f544a6a4..406aa0d52e 100644
>> --- a/gdb/dwarf2read.c
>> +++ b/gdb/dwarf2read.c
>> @@ -2870,7 +2870,7 @@ dw2_do_instantiate_symtab (struct dwarf2_per_cu_data *per_cu)
>> : (per_cu->v.psymtab == NULL || !per_cu->v.psymtab->readin))
>> {
>> queue_comp_unit (per_cu, language_minimal);
>> -      load_cu (per_cu);
>> +      load_cu (per_cu, true);
>> 
>> /* If we just loaded a CU from a DWO, and we're working with an index
>> that may badly handle TUs, load all the TUs in that DWO as well.

This use of "true" actually seems weird to me on re-reading.
dw2_do_instantiate_symtab doesn't actually use the skip_partial
parameter I added.

So, I changed this to pass skip_partial to load_cu.
I verified that the -readnow test on libwebkit2gtk still works.

>> @@ -4144,7 +4144,7 @@ dw2_expand_all_symtabs (struct objfile *objfile)
>> {
>> dwarf2_per_cu_data *per_cu = dwarf2_per_objfile->get_cutu (i);
>> 
>> -      dw2_instantiate_symtab (per_cu);
>> +      dw2_instantiate_symtab (per_cu, true);
>> }
>> }
>> 

Here I added this comment:

      /* We don't want to directly expand a partial CU, because if we
	 read it with the wrong language, then assertion failures can
	 be triggered later on.  See PR symtab/23010.  So, tell
	 dw2_instantiate_symtab to skip partial CUs -- any important
	 partial CU will be read via DW_TAG_imported_unit anyway.  */


I looked at your test case a bit but I also couldn't make it fail.

I'm pushing this patch in now.

Tom
Sergio Durigan Junior May 18, 2018, 12:25 a.m. UTC | #7 
On Monday, May 07 2018, Joel Brobecker wrote:

> I was wondering if anyone had any thoughts regarding Tom patch.
> https://sourceware.org/ml/gdb-patches/2018-04/msg00234.html
>
> Below are my comments on it, and also my interrogations on whether
> we might want this patch in 8.1.1 or not.
>
> Additional thoughts:
>   - This is a regression
>   - This is an internal error, so it can be fairly problematic
>   - It only happens with -readnow, it seems, which I assume
>     is not widely used considering the performance and memory
>     cost of this feature.
>
> I might tip in favor of putting it in, considering the fact that
> I don't think there is much of a workaround, but I would not make
> that call just on my own, because the patch is far from obvious.

[ CCing Pedro.  ]

Hey Joel,

Just my two cents here.  This patch apparently fixes a bunch of bugs
filed against Fedora GDB, which may indicate that either (a) it's not
related only to -readnow, or (b) more people use -readnow than we know
of ;-).

In either case, and speaking without much knowledge of the patch itself,
I think it should be included in 8.1.1.  At least I know I will include
it in our Fedora GDB (and well, if Tom is willing to backport it, then
he'll also save me some time!).

Cheers,
diff mbox

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index f2bb3d0af3..46e3c9d6cd 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,27 @@ 
 2018-04-12  Tom Tromey  <tom@tromey.com>
 
+	PR symtab/23010:
+	* dwarf2read.c (load_cu, dw2_do_instantiate_symtab)
+	(dw2_instantiate_symtab): Add skip_partial parameter.
+	(dw2_find_last_source_symtab, dw2_map_expand_apply)
+	(dw2_lookup_symbol, dw2_expand_symtabs_for_function)
+	(dw2_expand_all_symtabs, dw2_expand_symtabs_with_fullname)
+	(dw2_expand_symtabs_matching_one)
+	(dw2_find_pc_sect_compunit_symtab)
+	(dw2_debug_names_lookup_symbol)
+	(dw2_debug_names_expand_symtabs_for_function): Update.
+	(init_cutu_and_read_dies): Add skip_partial parameter.
+	(process_psymtab_comp_unit, build_type_psymtabs_1)
+	(process_skeletonless_type_unit, load_partial_comp_unit)
+	(psymtab_to_symtab_1): Update.
+	(load_full_comp_unit): Add skip_partial parameter.
+	(process_imported_unit_die, dwarf2_read_addr_index)
+	(follow_die_offset, dwarf2_fetch_die_loc_sect_off)
+	(dwarf2_fetch_constant_bytes, dwarf2_fetch_die_type_sect_off)
+	(read_signatured_type): Update.
+
+2018-04-12  Tom Tromey  <tom@tromey.com>
+
 	* dwarf2read.c (read_structure_type): Set TYPE_NAME for rust
 	unions.
 
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index e3f544a6a4..406aa0d52e 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -1832,7 +1832,7 @@  static void create_all_comp_units (struct dwarf2_per_objfile *dwarf2_per_objfile
 
 static int create_all_type_units (struct dwarf2_per_objfile *dwarf2_per_objfile);
 
-static void load_full_comp_unit (struct dwarf2_per_cu_data *,
+static void load_full_comp_unit (struct dwarf2_per_cu_data *, bool,
 				 enum language);
 
 static void process_full_comp_unit (struct dwarf2_per_cu_data *,
@@ -1932,7 +1932,7 @@  static const gdb_byte *read_and_check_comp_unit_head
 
 static void init_cutu_and_read_dies
   (struct dwarf2_per_cu_data *this_cu, struct abbrev_table *abbrev_table,
-   int use_existing_cu, int keep,
+   int use_existing_cu, int keep, bool skip_partial,
    die_reader_func_ftype *die_reader_func, void *data);
 
 static void init_cutu_and_read_dies_simple
@@ -2835,12 +2835,12 @@  create_quick_file_names_table (unsigned int nr_initial_entries)
    processing PER_CU->CU.  dw2_setup must have been already called.  */
 
 static void
-load_cu (struct dwarf2_per_cu_data *per_cu)
+load_cu (struct dwarf2_per_cu_data *per_cu, bool skip_partial)
 {
   if (per_cu->is_debug_types)
     load_full_type_unit (per_cu);
   else
-    load_full_comp_unit (per_cu, language_minimal);
+    load_full_comp_unit (per_cu, skip_partial, language_minimal);
 
   if (per_cu->cu == NULL)
     return;  /* Dummy CU.  */
@@ -2851,7 +2851,7 @@  load_cu (struct dwarf2_per_cu_data *per_cu)
 /* Read in the symbols for PER_CU.  */
 
 static void
-dw2_do_instantiate_symtab (struct dwarf2_per_cu_data *per_cu)
+dw2_do_instantiate_symtab (struct dwarf2_per_cu_data *per_cu, bool skip_partial)
 {
   struct dwarf2_per_objfile *dwarf2_per_objfile = per_cu->dwarf2_per_objfile;
 
@@ -2870,7 +2870,7 @@  dw2_do_instantiate_symtab (struct dwarf2_per_cu_data *per_cu)
       : (per_cu->v.psymtab == NULL || !per_cu->v.psymtab->readin))
     {
       queue_comp_unit (per_cu, language_minimal);
-      load_cu (per_cu);
+      load_cu (per_cu, true);
 
       /* If we just loaded a CU from a DWO, and we're working with an index
 	 that may badly handle TUs, load all the TUs in that DWO as well.
@@ -2897,7 +2897,7 @@  dw2_do_instantiate_symtab (struct dwarf2_per_cu_data *per_cu)
    table.  */
 
 static struct compunit_symtab *
-dw2_instantiate_symtab (struct dwarf2_per_cu_data *per_cu)
+dw2_instantiate_symtab (struct dwarf2_per_cu_data *per_cu, bool skip_partial)
 {
   struct dwarf2_per_objfile *dwarf2_per_objfile = per_cu->dwarf2_per_objfile;
 
@@ -2906,7 +2906,7 @@  dw2_instantiate_symtab (struct dwarf2_per_cu_data *per_cu)
     {
       free_cached_comp_units freer (dwarf2_per_objfile);
       scoped_restore decrementer = increment_reading_symtab ();
-      dw2_do_instantiate_symtab (per_cu);
+      dw2_do_instantiate_symtab (per_cu, skip_partial);
       process_cu_includes (dwarf2_per_objfile);
     }
 
@@ -3741,7 +3741,7 @@  dw2_find_last_source_symtab (struct objfile *objfile)
   struct dwarf2_per_objfile *dwarf2_per_objfile
     = get_dwarf2_per_objfile (objfile);
   dwarf2_per_cu_data *dwarf_cu = dwarf2_per_objfile->all_comp_units.back ();
-  compunit_symtab *cust = dw2_instantiate_symtab (dwarf_cu);
+  compunit_symtab *cust = dw2_instantiate_symtab (dwarf_cu, false);
 
   if (cust == NULL)
     return NULL;
@@ -3797,7 +3797,7 @@  dw2_map_expand_apply (struct objfile *objfile,
 
   /* This may expand more than one symtab, and we want to iterate over
      all of them.  */
-  dw2_instantiate_symtab (per_cu);
+  dw2_instantiate_symtab (per_cu, false);
 
   return iterate_over_some_symtabs (name, real_path, objfile->compunit_symtabs,
 				    last_made, callback);
@@ -4037,7 +4037,7 @@  dw2_lookup_symbol (struct objfile *objfile, int block_index,
   while ((per_cu = dw2_symtab_iter_next (&iter)) != NULL)
     {
       struct symbol *sym, *with_opaque = NULL;
-      struct compunit_symtab *stab = dw2_instantiate_symtab (per_cu);
+      struct compunit_symtab *stab = dw2_instantiate_symtab (per_cu, false);
       const struct blockvector *bv = COMPUNIT_BLOCKVECTOR (stab);
       struct block *block = BLOCKVECTOR_BLOCK (bv, block_index);
 
@@ -4128,7 +4128,7 @@  dw2_expand_symtabs_for_function (struct objfile *objfile,
 			func_name);
 
   while ((per_cu = dw2_symtab_iter_next (&iter)) != NULL)
-    dw2_instantiate_symtab (per_cu);
+    dw2_instantiate_symtab (per_cu, false);
 
 }
 
@@ -4144,7 +4144,7 @@  dw2_expand_all_symtabs (struct objfile *objfile)
     {
       dwarf2_per_cu_data *per_cu = dwarf2_per_objfile->get_cutu (i);
 
-      dw2_instantiate_symtab (per_cu);
+      dw2_instantiate_symtab (per_cu, true);
     }
 }
 
@@ -4176,7 +4176,7 @@  dw2_expand_symtabs_with_fullname (struct objfile *objfile,
 
 	  if (filename_cmp (this_fullname, fullname) == 0)
 	    {
-	      dw2_instantiate_symtab (per_cu);
+	      dw2_instantiate_symtab (per_cu, false);
 	      break;
 	    }
 	}
@@ -5000,7 +5000,7 @@  dw2_expand_symtabs_matching_one
       bool symtab_was_null
 	= (per_cu->v.quick->compunit_symtab == NULL);
 
-      dw2_instantiate_symtab (per_cu);
+      dw2_instantiate_symtab (per_cu, false);
 
       if (expansion_notify != NULL
 	  && symtab_was_null
@@ -5250,7 +5250,8 @@  dw2_find_pc_sect_compunit_symtab (struct objfile *objfile,
 	     paddress (get_objfile_arch (objfile), pc));
 
   result
-    = recursively_find_pc_sect_compunit_symtab (dw2_instantiate_symtab (data),
+    = recursively_find_pc_sect_compunit_symtab (dw2_instantiate_symtab (data,
+									false),
 						pc);
   gdb_assert (result != NULL);
   return result;
@@ -6049,7 +6050,7 @@  dw2_debug_names_lookup_symbol (struct objfile *objfile, int block_index_int,
   while ((per_cu = iter.next ()) != NULL)
     {
       struct symbol *sym, *with_opaque = NULL;
-      struct compunit_symtab *stab = dw2_instantiate_symtab (per_cu);
+      struct compunit_symtab *stab = dw2_instantiate_symtab (per_cu, false);
       const struct blockvector *bv = COMPUNIT_BLOCKVECTOR (stab);
       struct block *block = BLOCKVECTOR_BLOCK (bv, block_index);
 
@@ -6111,7 +6112,7 @@  dw2_debug_names_expand_symtabs_for_function (struct objfile *objfile,
 
       struct dwarf2_per_cu_data *per_cu;
       while ((per_cu = iter.next ()) != NULL)
-	dw2_instantiate_symtab (per_cu);
+	dw2_instantiate_symtab (per_cu, false);
     }
 }
 
@@ -7410,6 +7411,7 @@  static void
 init_cutu_and_read_dies (struct dwarf2_per_cu_data *this_cu,
 			 struct abbrev_table *abbrev_table,
 			 int use_existing_cu, int keep,
+			 bool skip_partial,
 			 die_reader_func_ftype *die_reader_func,
 			 void *data)
 {
@@ -7548,6 +7550,9 @@  init_cutu_and_read_dies (struct dwarf2_per_cu_data *this_cu,
   init_cu_die_reader (&reader, cu, section, NULL, abbrev_table);
   info_ptr = read_full_die (&reader, &comp_unit_die, info_ptr, &has_children);
 
+  if (skip_partial && comp_unit_die->tag == DW_TAG_partial_unit)
+    return;
+
   /* If we are in a DWO stub, process it and then read in the "real" CU/TU
      from the DWO file.  read_cutu_die_from_dwo will allocate the abbreviation
      table from the DWO file and pass the ownership over to us.  It will be
@@ -8042,14 +8047,14 @@  process_psymtab_comp_unit (struct dwarf2_per_cu_data *this_cu,
     free_one_cached_comp_unit (this_cu);
 
   if (this_cu->is_debug_types)
-    init_cutu_and_read_dies (this_cu, NULL, 0, 0, build_type_psymtabs_reader,
-			     NULL);
+    init_cutu_and_read_dies (this_cu, NULL, 0, 0, false,
+			     build_type_psymtabs_reader, NULL);
   else
     {
       process_psymtab_comp_unit_data info;
       info.want_partial_unit = want_partial_unit;
       info.pretend_language = pretend_language;
-      init_cutu_and_read_dies (this_cu, NULL, 0, 0,
+      init_cutu_and_read_dies (this_cu, NULL, 0, 0, false,
 			       process_psymtab_comp_unit_reader, &info);
     }
 
@@ -8209,7 +8214,7 @@  build_type_psymtabs_1 (struct dwarf2_per_objfile *dwarf2_per_objfile)
 	}
 
       init_cutu_and_read_dies (&tu.sig_type->per_cu, abbrev_table.get (),
-			       0, 0, build_type_psymtabs_reader, NULL);
+			       0, 0, false, build_type_psymtabs_reader, NULL);
     }
 }
 
@@ -8316,7 +8321,7 @@  process_skeletonless_type_unit (void **slot, void *info)
   *slot = entry;
 
   /* This does the job that build_type_psymtabs_1 would have done.  */
-  init_cutu_and_read_dies (&entry->per_cu, NULL, 0, 0,
+  init_cutu_and_read_dies (&entry->per_cu, NULL, 0, 0, false,
 			   build_type_psymtabs_reader, NULL);
 
   return 1;
@@ -8464,7 +8469,7 @@  load_partial_comp_unit_reader (const struct die_reader_specs *reader,
 static void
 load_partial_comp_unit (struct dwarf2_per_cu_data *this_cu)
 {
-  init_cutu_and_read_dies (this_cu, NULL, 1, 1,
+  init_cutu_and_read_dies (this_cu, NULL, 1, 1, false,
 			   load_partial_comp_unit_reader, NULL);
 }
 
@@ -9558,7 +9563,7 @@  psymtab_to_symtab_1 (struct partial_symtab *pst)
       return;
     }
 
-  dw2_do_instantiate_symtab (per_cu);
+  dw2_do_instantiate_symtab (per_cu, false);
 }
 
 /* Trivial hash function for die_info: the hash value of a DIE
@@ -9627,11 +9632,12 @@  load_full_comp_unit_reader (const struct die_reader_specs *reader,
 
 static void
 load_full_comp_unit (struct dwarf2_per_cu_data *this_cu,
+		     bool skip_partial,
 		     enum language pretend_language)
 {
   gdb_assert (! this_cu->is_debug_types);
 
-  init_cutu_and_read_dies (this_cu, NULL, 1, 1,
+  init_cutu_and_read_dies (this_cu, NULL, 1, 1, skip_partial,
 			   load_full_comp_unit_reader, &pretend_language);
 }
 
@@ -10458,7 +10464,7 @@  process_imported_unit_die (struct die_info *die, struct dwarf2_cu *cu)
 
       /* If necessary, add it to the queue and load its DIEs.  */
       if (maybe_queue_comp_unit (cu, per_cu, cu->language))
-	load_full_comp_unit (per_cu, cu->language);
+	load_full_comp_unit (per_cu, false, cu->language);
 
       VEC_safe_push (dwarf2_per_cu_ptr, cu->per_cu->imported_symtabs,
 		     per_cu);
@@ -19569,7 +19575,7 @@  dwarf2_read_addr_index (struct dwarf2_per_cu_data *per_cu,
 
       /* Note: We can't use init_cutu_and_read_dies_simple here,
 	 we need addr_base.  */
-      init_cutu_and_read_dies (per_cu, NULL, 0, 0,
+      init_cutu_and_read_dies (per_cu, NULL, 0, 0, false,
 			       dwarf2_read_addr_index_reader, &aidata);
       addr_base = aidata.addr_base;
       addr_size = aidata.addr_size;
@@ -22776,7 +22782,7 @@  follow_die_offset (sect_offset sect_off, int offset_in_dwz,
 
       /* If necessary, add it to the queue and load its DIEs.  */
       if (maybe_queue_comp_unit (cu, per_cu, cu->language))
-	load_full_comp_unit (per_cu, cu->language);
+	load_full_comp_unit (per_cu, false, cu->language);
 
       target_cu = per_cu->cu;
     }
@@ -22784,7 +22790,7 @@  follow_die_offset (sect_offset sect_off, int offset_in_dwz,
     {
       /* We're loading full DIEs during partial symbol reading.  */
       gdb_assert (dwarf2_per_objfile->reading_partial_symbols);
-      load_full_comp_unit (cu->per_cu, language_minimal);
+      load_full_comp_unit (cu->per_cu, false, language_minimal);
     }
 
   *ref_cu = target_cu;
@@ -22838,7 +22844,7 @@  dwarf2_fetch_die_loc_sect_off (sect_offset sect_off,
   struct objfile *objfile = dwarf2_per_objfile->objfile;
 
   if (per_cu->cu == NULL)
-    load_cu (per_cu);
+    load_cu (per_cu, false);
   cu = per_cu->cu;
   if (cu == NULL)
     {
@@ -22945,7 +22951,7 @@  dwarf2_fetch_constant_bytes (sect_offset sect_off,
   struct objfile *objfile = per_cu->dwarf2_per_objfile->objfile;
 
   if (per_cu->cu == NULL)
-    load_cu (per_cu);
+    load_cu (per_cu, false);
   cu = per_cu->cu;
   if (cu == NULL)
     {
@@ -23067,7 +23073,7 @@  dwarf2_fetch_die_type_sect_off (sect_offset sect_off,
   struct die_info *die;
 
   if (per_cu->cu == NULL)
-    load_cu (per_cu);
+    load_cu (per_cu, false);
   cu = per_cu->cu;
   if (!cu)
     return NULL;
@@ -23348,7 +23354,7 @@  read_signatured_type (struct signatured_type *sig_type)
   gdb_assert (per_cu->is_debug_types);
   gdb_assert (per_cu->cu == NULL);
 
-  init_cutu_and_read_dies (per_cu, NULL, 0, 1,
+  init_cutu_and_read_dies (per_cu, NULL, 0, 1, false,
 			   read_signatured_type_reader, NULL);
   sig_type->per_cu.tu_read = 1;
 }