[RFA] Fix inconsistency in blockvector addrmap vs non-addrmap handling

Doug Evans dje@google.com
Mon Jun 25 11:39:00 GMT 2012


On Sun, Jun 24, 2012 at 11:33 AM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> On Fri, 22 Jun 2012 22:51:41 +0200, Doug Evans wrote:
>> But remember that the static block  (or global block) doesn't exist
>> until the call to end_symtab, and, barring even more work, you need to
>> get the range into pending_addrmap before the call to make_blockvector
>> where pending_addrmap is turned into a fixed addrmap.
>
> One could use either callback for end_sym there or split end_sym (which I have
> chosen).
>
> Unfortunately I do not have time now to finish a proper testcase also
> exploiting the discontiguous CU so posting just the fix here.  I should get to
> the testcase back on Tuesday.  In fact I do not know if this fix works at all
> for the discontiguous CU case.
>
> No regressions on {x86_64,x86_64-m32,i686}-fedorarawhide-linux-gnu against the
> patch:
>        http://sourceware.org/ml/gdb-patches/2012-06/msg00109.html
>        Subject: [RFA] Fix inconsistency in blockvector addrmap vs non-addrmap handling
>
>
> Thanks,
> Jan
>
>
> gdb/
> 2012-06-24  Jan Kratochvil  <jan.kratochvil@redhat.com>
>
>        * buildsym.c (end_symtab): Split it to ...
>        (end_symtab_get_static_block): ... this ...
>        (end_symtab_from_static_block): ... and this function.
>        (end_symtab): New function.
>        * buildsym.h (end_symtab_get_static_block)
>        (end_symtab_from_static_block): New declarations.
>        * dwarf2read.c (process_full_comp_unit): New variable static_block.
>        Set its valid CU ranges.
>
> diff --git a/gdb/buildsym.c b/gdb/buildsym.c
> index f1fb4be..2de2ef8 100644
> --- a/gdb/buildsym.c
> +++ b/gdb/buildsym.c
> @@ -923,38 +923,20 @@ block_compar (const void *ap, const void *bp)
>          - (BLOCK_START (b) < BLOCK_START (a)));
>  }
>
> -/* Finish the symbol definitions for one main source file, close off
> -   all the lexical contexts for that file (creating struct block's for
> -   them), then make the struct symtab for that file and put it in the
> -   list of all such.
> -
> -   END_ADDR is the address of the end of the file's text.  SECTION is
> -   the section number (in objfile->section_offsets) of the blockvector
> -   and linetable.
> +/* Implementation of the first part of end_symtab.  It allows modifying
> +   STATIC_BLOCK before it gets finalized by end_symtab_from_static_block.
> +   If the returned value is NULL there is no blockvector created for
> +   this symtab (you still must call end_symtab_from_static_block).  */
>
> -   Note that it is possible for end_symtab() to return NULL.  In
> -   particular, for the DWARF case at least, it will return NULL when
> -   it finds a compilation unit that has exactly one DIE, a
> -   TAG_compile_unit DIE.  This can happen when we link in an object
> -   file that was compiled from an empty source file.  Returning NULL
> -   is probably not the correct thing to do, because then gdb will
> -   never know about this empty file (FIXME).  */
> -
> -struct symtab *
> -end_symtab (CORE_ADDR end_addr, struct objfile *objfile, int section)
> +struct block *
> +end_symtab_get_static_block (CORE_ADDR end_addr, struct objfile *objfile)
>  {
> -  struct symtab *symtab = NULL;
> -  struct blockvector *blockvector;
> -  struct subfile *subfile;
> -  struct context_stack *cstk;
> -  struct subfile *nextsub;
> -
>   /* Finish the lexical context of the last function in the file; pop
>      the context stack.  */
>
>   if (context_stack_depth > 0)
>     {
> -      cstk = pop_context ();
> +      struct context_stack *cstk = pop_context ();
>       /* Make a block for the local symbols within.  */
>       finish_block (cstk->name, &local_symbols, cstk->old_blocks,
>                    cstk->start_addr, end_addr, objfile);
> @@ -1021,15 +1003,41 @@ end_symtab (CORE_ADDR end_addr, struct objfile *objfile, int section)
>     {
>       /* Ignore symtabs that have no functions with real debugging
>          info.  */
> +      return NULL;
> +    }
> +  else
> +    {
> +      /* Define the STATIC_BLOCK.  */
> +      return finish_block (NULL, &file_symbols, NULL, last_source_start_addr,
> +                          end_addr, objfile);
> +    }
> +}
> +
> +/* Implementation of the second part of end_symtab.  Pass STATIC_BLOCK
> +   as value returned by end_symtab_get_static_block.  */
> +
> +struct symtab *
> +end_symtab_from_static_block (struct block *static_block,
> +                             struct objfile *objfile, int section)
> +{
> +  struct symtab *symtab = NULL;
> +  struct blockvector *blockvector;
> +  struct subfile *subfile;
> +  struct subfile *nextsub;
> +
> +  if (static_block == NULL)
> +    {
> +      /* Ignore symtabs that have no functions with real debugging
> +         info.  */
>       blockvector = NULL;
>     }
>   else
>     {
> -      /* Define the STATIC_BLOCK & GLOBAL_BLOCK, and build the
> +      CORE_ADDR end_addr = BLOCK_END (static_block);
> +
> +      /* Define after STATIC_BLOCK also GLOBAL_BLOCK, and build the
>          blockvector.  */
> -      finish_block (0, &file_symbols, 0, last_source_start_addr,
> -                   end_addr, objfile);
> -      finish_block_internal (0, &global_symbols, 0, last_source_start_addr,
> +      finish_block_internal (NULL, &global_symbols, NULL, last_source_start_addr,
>                             end_addr, objfile, 1);
>       blockvector = make_blockvector (objfile);
>     }
> @@ -1219,6 +1227,36 @@ end_symtab (CORE_ADDR end_addr, struct objfile *objfile, int section)
>   return symtab;
>  }
>
> +/* Finish the symbol definitions for one main source file, close off
> +   all the lexical contexts for that file (creating struct block's for
> +   them), then make the struct symtab for that file and put it in the
> +   list of all such.
> +
> +   END_ADDR is the address of the end of the file's text.  SECTION is
> +   the section number (in objfile->section_offsets) of the blockvector
> +   and linetable.
> +
> +   Note that it is possible for end_symtab() to return NULL.  In
> +   particular, for the DWARF case at least, it will return NULL when
> +   it finds a compilation unit that has exactly one DIE, a
> +   TAG_compile_unit DIE.  This can happen when we link in an object
> +   file that was compiled from an empty source file.  Returning NULL
> +   is probably not the correct thing to do, because then gdb will
> +   never know about this empty file (FIXME).
> +
> +   If you need to modify STATIC_BLOCK before it is finalized you should
> +   call end_symtab_get_static_block and end_symtab_from_static_block
> +   yourself.  */
> +
> +struct symtab *
> +end_symtab (CORE_ADDR end_addr, struct objfile *objfile, int section)
> +{
> +  struct block *static_block;
> +
> +  static_block = end_symtab_get_static_block (end_addr, objfile);
> +  return end_symtab_from_static_block (static_block, objfile, section);
> +}
> +
>  /* Push a context block.  Args are an identifying nesting level
>    (checkable when you pop it), and the starting PC address of this
>    context.  */
> diff --git a/gdb/buildsym.h b/gdb/buildsym.h
> index 4448267..89324e9 100644
> --- a/gdb/buildsym.h
> +++ b/gdb/buildsym.h
> @@ -260,6 +260,11 @@ extern char *pop_subfile (void);
>
>  extern struct symtab *end_symtab (CORE_ADDR end_addr,
>                                  struct objfile *objfile, int section);
> +extern struct block *end_symtab_get_static_block (CORE_ADDR end_addr,
> +                                                 struct objfile *objfile);
> +extern struct symtab *end_symtab_from_static_block (struct block *static_block,
> +                                                   struct objfile *objfile,
> +                                                   int section);
>
>  /* Defined in stabsread.c.  */
>
> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
> index aa42b4c..62119ad 100644
> --- a/gdb/dwarf2read.c
> +++ b/gdb/dwarf2read.c
> @@ -5790,6 +5790,7 @@ process_full_comp_unit (struct dwarf2_per_cu_data *per_cu,
>   struct symtab *symtab;
>   struct cleanup *back_to, *delayed_list_cleanup;
>   CORE_ADDR baseaddr;
> +  struct block *static_block;
>
>   baseaddr = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
>
> @@ -5820,7 +5821,15 @@ process_full_comp_unit (struct dwarf2_per_cu_data *per_cu,
>      it, by scanning the DIE's below the compilation unit.  */
>   get_scope_pc_bounds (cu->dies, &lowpc, &highpc, cu);
>
> -  symtab = end_symtab (highpc + baseaddr, objfile, SECT_OFF_TEXT (objfile));
> +  static_block = end_symtab_get_static_block (highpc + baseaddr, objfile);
> +
> +  /* This CU may have discontiguous DW_AT_ranges and the CU may have addresses
> +     not belonging to any of its child DIEs (such as virtual method tables).
> +     Assign such addresses to STATIC_BLOCK's addrmap.  */
> +  dwarf2_record_block_ranges (cu->dies, static_block, baseaddr, cu);
> +
> +  symtab = end_symtab_from_static_block (static_block, objfile,
> +                                        SECT_OFF_TEXT (objfile));
>
>   if (symtab != NULL)
>     {

This is on the right track, but it's incomplete (IMO).

- Why call get_scope_pc_bounds if we're going to then explicitly
attach DW_TAG_compile_unit's low_pc/high_pc/ranges attributes to
STATIC_BLOCK? [we're processing these attributes twice, blech]

- If it does actually fix the bug in question it seems more accidental
than on purpose.
  - e.g. if the DW_TAG_compile_unit's DW_AT_ranges doesn't include some minsym,
    what happens?

- If we really want to take this step then start,end on global,static
blocks needs to go.
  Until then we're just satisfying a desire to apply dwarf correctly
without really accomplishing anything.



More information about the Gdb-patches mailing list