[RFA] Fix inconsistency in blockvector addrmap vs non-addrmap handling
Doug Evans
dje@google.com
Thu Jul 12 05:19:00 GMT 2012
On Tue, Jul 3, 2012 at 5:54 AM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
>> - If it does actually fix the bug in question it seems more accidental
>> than on purpose.
>
> I do not see why you think so.
I think I was confused about something.
>> - If we really want to take this step then start,end on global,static
>> blocks needs to go.
>
> <abstract chat>
> GDB needs some BLOCK_START / BLOCK_END abstraction for things like sorting
> blocks and finding the most narrow block etc.
>
> GDB needs some abstraction of the code. For -O2 -g code in fact there are no
> longer any source statements boundaries, there is no execution order etc. The
> abstraction of inferior for the purposes of GDB user interface will be
> probably always broken in some way for -O2 -g code as for real -O2 -g code one
> cannot simplify it more than what 'disas' shows.
>
> This patch tries to exactly match process_psymtab_comp_unit_reader and
> dw2_do_instantiate_symtab. I call it a bugfix. I do not try to redesign GDB
> in this patch.
>
> Whether BLOCK_END or BLOCK_START could be removed is more a GDB design change.
> Currently it is a fallback for GDB code not yet prepared to use
> BLOCKVECTOR_MAP. As usual in GDB there are kept both the older and modern
> ways how to do things as nobody wants to update all the code in GDB at once so
> we could drop the old code.
> </abstract chat>
Note that I was specifically referring to GLOBAL and STATIC blocks.
There the concept of having a start and end has lost its meaning.
They have a lower bound and an upper bound which may be useful in
sorting, but using their start/end values as start/end values is
problematic.
>
>> Until then we're just satisfying a desire to apply dwarf correctly
>> without really accomplishing anything.
>
> I tried to exploit some case where the patch of mine behaves more correctly
> than the patch of yours but I cannot. Any code which accesses STATIC_BLOCK's
> assigned ranges already checks first objfile->psymtabs_addrmap which already
> has exactly correct CU's DW_AT_ranges map.
I was playing with a case of "pc 0x42 in read in psymtab, but not in
symtab" case today.
gdb has other problems that get in the way, but having more
consistency between partial and full syms has to be a good thing.
[btw, seems kinda odd to have two addrmaps: one for psymtabs/index and
one for fullsyms. I wonder if there's an opportunity here.]
Here's an updated version of your patch to match the current tree.
2012-07-11 Jan Kratochvil <jan.kratochvil@redhat.com>
Doug Evans <dje@google.com>
* buildsym.c (end_symtab_1): Split it to ...
(end_symtab_get_static_block): ... this ...
(end_symtab_from_static_block): ... and this function.
(end_symtab, end_expandable_symtab): Call them.
* 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.
-------------- next part --------------
2012-07-11 Jan Kratochvil <jan.kratochvil@redhat.com>
Doug Evans <dje@google.com>
* buildsym.c (end_symtab_1): Split it to ...
(end_symtab_get_static_block): ... this ...
(end_symtab_from_static_block): ... and this function.
(end_symtab, end_expandable_symtab): Call them.
* 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.
Index: buildsym.c
===================================================================
RCS file: /cvs/src/src/gdb/buildsym.c,v
retrieving revision 1.100
diff -u -p -r1.100 buildsym.c
--- buildsym.c 10 Jul 2012 20:20:15 -0000 1.100
+++ buildsym.c 12 Jul 2012 04:56:06 -0000
@@ -957,42 +957,28 @@ reset_symtab_globals (void)
}
}
-/* 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.
-
- If EXPANDABLE is non-zero the dictionaries for the global and static
- blocks are made expandable.
-
- 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). */
-
-static struct symtab *
-end_symtab_1 (CORE_ADDR end_addr, struct objfile *objfile, int section,
- int expandable)
+/* 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).
+
+ END_ADDR is the same as for end_symtab: the address of the end of the
+ file's text.
+
+ If EXPANDABLE is non-zero the STATIC_BLOCK dictionary is made
+ expandable. */
+
+struct block *
+end_symtab_get_static_block (CORE_ADDR end_addr, struct objfile *objfile,
+ int expandable)
{
- 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);
@@ -1058,18 +1044,51 @@ end_symtab_1 (CORE_ADDR end_addr, struct
&& have_line_numbers == 0
&& pending_macros == NULL)
{
- /* Ignore symtabs that have no functions with real debugging
- info. */
+ /* Ignore symtabs that have no functions with real debugging info. */
+ return NULL;
+ }
+ else
+ {
+ /* Define the STATIC_BLOCK. */
+ return finish_block_internal (NULL, &file_symbols, NULL,
+ last_source_start_addr, end_addr, objfile,
+ 0, expandable);
+ }
+}
+
+/* Implementation of the second part of end_symtab. Pass STATIC_BLOCK
+ as value returned by end_symtab_get_static_block.
+
+ SECTION is the same as for end_symtab: the section number
+ (in objfile->section_offsets) of the blockvector and linetable.
+
+ If EXPANDABLE is non-zero the GLOBAL_BLOCK dictionary is made
+ expandable. */
+
+struct symtab *
+end_symtab_from_static_block (struct block *static_block,
+ struct objfile *objfile, int section,
+ int expandable)
+{
+ 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_internal (0, &file_symbols, 0, last_source_start_addr,
- end_addr, objfile, 0, expandable);
- finish_block_internal (0, &global_symbols, 0, last_source_start_addr,
- end_addr, objfile, 1, expandable);
+ finish_block_internal (NULL, &global_symbols, NULL,
+ last_source_start_addr, end_addr, objfile,
+ 1, expandable);
blockvector = make_blockvector (objfile);
}
@@ -1251,21 +1270,46 @@ end_symtab_1 (CORE_ADDR end_addr, struct
return symtab;
}
-/* See end_symtab_1 for details. */
+/* 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)
{
- return end_symtab_1 (end_addr, objfile, section, 0);
+ struct block *static_block;
+
+ static_block = end_symtab_get_static_block (end_addr, objfile, 0);
+ return end_symtab_from_static_block (static_block, objfile, section, 0);
}
-/* See end_symtab_1 for details. */
+/* Same as end_symtab except create a symtab that can be later added to. */
struct symtab *
end_expandable_symtab (CORE_ADDR end_addr, struct objfile *objfile,
int section)
{
- return end_symtab_1 (end_addr, objfile, section, 1);
+ struct block *static_block;
+
+ static_block = end_symtab_get_static_block (end_addr, objfile, 1);
+ return end_symtab_from_static_block (static_block, objfile, section, 1);
}
/* Subroutine of augment_type_symtab to simplify it.
Index: buildsym.h
===================================================================
RCS file: /cvs/src/src/gdb/buildsym.h,v
retrieving revision 1.33
diff -u -p -r1.33 buildsym.h
--- buildsym.h 10 Jul 2012 20:20:15 -0000 1.33
+++ buildsym.h 12 Jul 2012 04:56:06 -0000
@@ -258,6 +258,15 @@ extern void push_subfile (void);
extern char *pop_subfile (void);
+extern struct block *end_symtab_get_static_block (CORE_ADDR end_addr,
+ struct objfile *objfile,
+ int expandable);
+
+extern struct symtab *end_symtab_from_static_block (struct block *static_block,
+ struct objfile *objfile,
+ int section,
+ int expandable);
+
extern struct symtab *end_symtab (CORE_ADDR end_addr,
struct objfile *objfile, int section);
Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.684
diff -u -p -r1.684 dwarf2read.c
--- dwarf2read.c 10 Jul 2012 20:28:32 -0000 1.684
+++ dwarf2read.c 12 Jul 2012 04:56:07 -0000
@@ -6593,6 +6593,7 @@ process_full_comp_unit (struct dwarf2_pe
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));
@@ -6623,7 +6624,17 @@ process_full_comp_unit (struct dwarf2_pe
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, 0);
+
+ /* If the comp unit has DW_AT_ranges, it may have discontiguous ranges.
+ Also, DW_AT_ranges may record ranges not belonging to any child DIEs
+ (such as virtual method tables). Record the ranges in STATIC_BLOCK's
+ addrmap to help ensure it has an accurate map of pc values belonging to
+ this comp unit. */
+ dwarf2_record_block_ranges (cu->dies, static_block, baseaddr, cu);
+
+ symtab = end_symtab_from_static_block (static_block, objfile,
+ SECT_OFF_TEXT (objfile), 0);
if (symtab != NULL)
{
More information about the Gdb-patches
mailing list