[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