This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

possible fix for PR symtab/23010


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.

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;
 }


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]