* Re: [RFC/dwarf-2] Add support for included files
@ 2004-05-03 22:15 Andrew Pinski
2004-05-04 0:15 ` Joel Brobecker
0 siblings, 1 reply; 22+ messages in thread
From: Andrew Pinski @ 2004-05-03 22:15 UTC (permalink / raw)
To: gdb-patches, J. Brobecker; +Cc: Andrew Pinski
I try to test building gdb with the mainline of gcc and it fails
to compile with the following error:
/home/gates/pinskia/src/gnu/combinesources/src/gdb/dwarf2read.c: In
function `dwarf2_create_include_psymtab':
/home/gates/pinskia/src/gnu/combinesources/src/gdb/dwarf2read.c:1244:
error: invalid lvalue in assignment
Could you fix the problem, please?
Thanks,
Andrew Pinski
2004-04-30 J. Brobecker <brobecker@gnat.com>
* dwarf2read.c (line_header): Add new included_p field in
field file_names.
(partial_die_info): New field has_stmt_list. New field
line_offset.
(dwarf2_create_include_psymtab): New function.
(dwarf2_build_include_psymtabs): New function.
(add_file_name): Add forward declaration. Initialize new field.
(dwarf_decode_lines): Add new parameter. Enhance this procedure
to be able to determine the list of files included by the
given unit, and build their associated psymtabs.
(dwarf2_build_psymtabs_hard): Build the psymtabs for the
included
files as well.
(psymtab_to_symtab_1): Build the symtabs of all dependencies as
well.
(read_file_scope): Update call to dwarf_decode_lines.
(read_partial_die): Handle DW_AT_stmt_list attributes.
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [RFC/dwarf-2] Add support for included files 2004-05-03 22:15 [RFC/dwarf-2] Add support for included files Andrew Pinski @ 2004-05-04 0:15 ` Joel Brobecker 2004-05-04 0:18 ` Andrew Pinski 0 siblings, 1 reply; 22+ messages in thread From: Joel Brobecker @ 2004-05-04 0:15 UTC (permalink / raw) To: Andrew Pinski; +Cc: gdb-patches [-- Attachment #1: Type: text/plain, Size: 957 bytes --] > /home/gates/pinskia/src/gnu/combinesources/src/gdb/dwarf2read.c: In > function `dwarf2_create_include_psymtab': > /home/gates/pinskia/src/gnu/combinesources/src/gdb/dwarf2read.c:1244: > error: invalid lvalue in assignment Sorry about that, this error didn't show up with the compiler I used. I just built one straight from mainline and could reproduce the error. Apparently, expressions like this are not legal? (char *) something = NULL; Anyway, I fixed the immediate problem by expanding the PST_PRIVATE macro into its actual definition, but without the cast. That should allow you to build again. 2004-05-03 Joel Brobecker <brobecker@gnat.com> * dwarf2read.c (dwarf2_create_include_psymtab): Fix build failure detected by recent versions of GCC. (psymtab_to_symtab_1): No longer use the PST_PRIVATE macro to be consistent with the usage in dwarf2_create_include_psymtab. Checked in mainline. -- Joel [-- Attachment #2: dwarf2read.c.diff --] [-- Type: text/plain, Size: 1225 bytes --] Index: dwarf2read.c =================================================================== RCS file: /cvs/src/src/gdb/dwarf2read.c,v retrieving revision 1.149 diff -u -p -r1.149 dwarf2read.c --- dwarf2read.c 3 May 2004 16:21:50 -0000 1.149 +++ dwarf2read.c 4 May 2004 00:08:10 -0000 @@ -1239,9 +1239,9 @@ dwarf2_create_include_psymtab (char *nam /* No private part is necessary for include psymtabs. This property can be used to differentiate between such include psymtabs and the regular ones. If it ever happens that a regular psymtab can - legitimally have a NULL PST_PRIVATE part, then we'll have to add a + legitimally have a NULL private part, then we'll have to add a dedicated field for that in the dwarf2_pinfo structure. */ - PST_PRIVATE (subpst) = NULL; + subpst->read_symtab_private = NULL; } /* Read the Line Number Program data and extract the list of files @@ -2109,7 +2109,7 @@ psymtab_to_symtab_1 (struct partial_symt psymtab_to_symtab_1 (pst->dependencies[i]); } - if (PST_PRIVATE (pst) == NULL) + if (pst->read_symtab_private == NULL) { /* It's an include file, no symbols to read for it. Everything is in the parent symtab. */ ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC/dwarf-2] Add support for included files 2004-05-04 0:15 ` Joel Brobecker @ 2004-05-04 0:18 ` Andrew Pinski 0 siblings, 0 replies; 22+ messages in thread From: Andrew Pinski @ 2004-05-04 0:18 UTC (permalink / raw) To: Joel Brobecker; +Cc: Andrew Pinski, gdb-patches On May 3, 2004, at 20:15, Joel Brobecker wrote: >> /home/gates/pinskia/src/gnu/combinesources/src/gdb/dwarf2read.c: In >> function `dwarf2_create_include_psymtab': >> /home/gates/pinskia/src/gnu/combinesources/src/gdb/dwarf2read.c:1244: >> error: invalid lvalue in assignment > > Sorry about that, this error didn't show up with the compiler I used. > I just built one straight from mainline and could reproduce the error. > Apparently, expressions like this are not legal? > > (char *) something = NULL; Yes that is invalid C, it was deprecated in 3.4.0 (which warns about the usage) and removed on the mainline. Thanks for fast fixing the problem. -Andrew Pinski ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC/dwarf-2] Add support for included files @ 2004-04-07 16:05 Jim Blandy 2004-04-13 5:20 ` Joel Brobecker 0 siblings, 1 reply; 22+ messages in thread From: Jim Blandy @ 2004-04-07 16:05 UTC (permalink / raw) To: Joel Brobecker; +Cc: Daniel Jacobowitz, Eli Zaretskii, gdb-patches Picking up a dropped thread from January: http://sources.redhat.com/ml/gdb-patches/2004-01/msg00015.html Some comments: It's fine for GDB core code to assume that the psymtabs mention every source file in the program. They're supposed to. The whole purpose of the psymtabs is to act as an index for the full symbol information, so that without taking the time/memory to read full symbols, we can at least find the compilation unit we do need to read. If that index doesn't mention a source file, then it's incomplete. I'm not too enthusiastic about Eli's suggestion that we put off scanning for source file names until we get a filename we can't find. It would help: we can find source files most of the time already, and when the second scan was needed, it wouldn't be any more work than what Joel's proposing we do every time. But it's adding a third phase to debug reading; I'd prefer to fix this problem without architectural changes like that, if it's practical. The original motivation for introducing psymtabs at all was speed: GDB's startup time was, believe it or not, CPU-bound. By constructing only partial symtabs at startup time, we avoided parsing types (essentially). How does this patch affect the time GDB takes to start up on itself? (If you had some monster program lying around, that would be better, but if all we've got is a brute...) If it makes a big speed difference, then let's try ignoring DW_LNE_define_file, not scanning the line number program, and just entering the files given in the file_names table as psymtabs. GCC doesn't generate DW_LNE_define_file. If that is much faster, then I think it'll be okay. (It's interesting to note that, since DW_LNE_define_file exists, the mechanisms Dwarf 3 provides for "accelerated access" are insufficient: there's no way to build a complete index of source file names without reading the line tables.) ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC/dwarf-2] Add support for included files 2004-04-07 16:05 Jim Blandy @ 2004-04-13 5:20 ` Joel Brobecker 2004-04-14 19:10 ` Jim Blandy 0 siblings, 1 reply; 22+ messages in thread From: Joel Brobecker @ 2004-04-13 5:20 UTC (permalink / raw) To: Jim Blandy; +Cc: Daniel Jacobowitz, Eli Zaretskii, gdb-patches > Picking up a dropped thread from January: > > http://sources.redhat.com/ml/gdb-patches/2004-01/msg00015.html Thank you Jim. > Some comments: > > It's fine for GDB core code to assume that the psymtabs mention every > source file in the program. They're supposed to. The whole purpose > of the psymtabs is to act as an index for the full symbol information, > so that without taking the time/memory to read full symbols, we can at > least find the compilation unit we do need to read. If that index > doesn't mention a source file, then it's incomplete. Cool. > I'm not too enthusiastic about Eli's suggestion that we put off > scanning for source file names until we get a filename we can't find. > It would help: we can find source files most of the time already, and > when the second scan was needed, it wouldn't be any more work than > what Joel's proposing we do every time. But it's adding a third phase > to debug reading; I'd prefer to fix this problem without architectural > changes like that, if it's practical. That's my feeling too. > The original motivation for introducing psymtabs at all was speed: > GDB's startup time was, believe it or not, CPU-bound. By constructing > only partial symtabs at startup time, we avoided parsing types > (essentially). How does this patch affect the time GDB takes to start > up on itself? (If you had some monster program lying around, that > would be better, but if all we've got is a brute...) Unfortunately, I don't have a monster program. I did some experimenting with GDB, though. Here is how I did it: Measuring the total elapsed time it takes GDB in batch mode to load the symbols and then exit, and did it ten times in a row inside a for loop. I measured the time difference between an unmodified GDB and a modified one. Without the change, I get a total elapsed time for the 10 consecutive runs of 9.453 secs. After my change, I get a total time of 9.604 secs. That gives us a rough increase of 1.6%, or said differently, an increase of startup time of 1 sec for each minute. That's all supposing that GDB is your fairly typical program. If somebody could send me a large binary, something like mozilla all built with dwarf-2 for instance, I'd be happy to rerun my experiment. > If it makes a big speed difference, then let's try ignoring > DW_LNE_define_file, not scanning the line number program, and just > entering the files given in the file_names table as psymtabs. GCC > doesn't generate DW_LNE_define_file. If that is much faster, then I > think it'll be okay. I tried that, but unfortunately, I've seen cases where the compiler emits an entry for an included file, but then never mentions it in the line table. For instance with an Ada program, I see that the compiler includes an entry for file "system.ads" (not sure why, is it because it's in the direct closure of my program?) in the Line Number Program Header, but then never uses it to provide line numbers. The latter is expected, since this file is a runtime file containing only declarations, so no statements causing code to be generated. If we were to skip parsing the line table, we would end up creating a psymtab for that system.ads file, and I think that it could cause an internal error later down the road when we try to promote that psymtab to a full symtab. The current code, I think, is not prepared for that situation. I can give it a try, but that's what I remember from my analysis of the current code back when I started working on this. I think it would be reasonable to argue that mentioning a file in the header without using it in the line table is a compiler bug. I am not sure about this as the spec is vague, but I wouldn't certainly object. However, if we decide that yes it is a compiler bug, and yes we should create the psymtab only based on the line table header, then we also have to accept the fact that we'll still have the problem should the compiler decide to rely on DW_LNE_define_file rather than the header to build the list of included files. Given the small startup time increase that I have measured so far, I think it's fine to scan the line table. But I am happy any way. I hope I have provided sufficient information for us to make a decision. > (It's interesting to note that, since DW_LNE_define_file exists, the > mechanisms Dwarf 3 provides for "accelerated access" are insufficient: > there's no way to build a complete index of source file names without > reading the line tables.) I really wondered about the usefulness of this opcode. Now, I am wondering even more... -- Joel ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC/dwarf-2] Add support for included files 2004-04-13 5:20 ` Joel Brobecker @ 2004-04-14 19:10 ` Jim Blandy 2004-04-15 22:13 ` Joel Brobecker 0 siblings, 1 reply; 22+ messages in thread From: Jim Blandy @ 2004-04-14 19:10 UTC (permalink / raw) To: Joel Brobecker; +Cc: Daniel Jacobowitz, Eli Zaretskii, gdb-patches Okay, let's take it as established that the speed impact is not too bad. Given the improved accuracy, I think it's worth taking the chance that we'll have to revisit it. Could you try one more thing for me? As you noticed, the only two functions that dwarf_decode_lines calls to actually record the information it gets are buildsym.c:start_subfile (via dwarf2_start_subfile) and buildsym.c:record_line. Could you change dwarf_decode_lines to take two function pointers (with an accompanying closure pointer) for those two functions? I guess this is an instance of the 'builder' pattern, so maybe name it something appropriately suggestive. Let dwarf_decode_lines continue to call dwarf2_start_subfile directly, just passing the builder func and closure along with filename and dirname. Then, instead of duplicating dwarf_decode_lines, have the existing call in read_file_scope and the new call you've added just pass different function/closures to it. If it's easy, re-run your speed tests to make sure this doesn't make some mysterious difference. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC/dwarf-2] Add support for included files 2004-04-14 19:10 ` Jim Blandy @ 2004-04-15 22:13 ` Joel Brobecker 2004-04-16 4:24 ` Jim Blandy 0 siblings, 1 reply; 22+ messages in thread From: Joel Brobecker @ 2004-04-15 22:13 UTC (permalink / raw) To: Jim Blandy; +Cc: Daniel Jacobowitz, Eli Zaretskii, gdb-patches [-- Attachment #1: Type: text/plain, Size: 2128 bytes --] > Could you try one more thing for me? Sure! > As you noticed, the only two functions that dwarf_decode_lines calls > to actually record the information it gets are > buildsym.c:start_subfile (via dwarf2_start_subfile) and > buildsym.c:record_line. Could you change dwarf_decode_lines to take > two function pointers (with an accompanying closure pointer) for those > two functions? I guess this is an instance of the 'builder' pattern, > so maybe name it something appropriately suggestive. Let > dwarf_decode_lines continue to call dwarf2_start_subfile directly, > just passing the builder func and closure along with filename and > dirname. > > Then, instead of duplicating dwarf_decode_lines, have the existing > call in read_file_scope and the new call you've added just pass > different function/closures to it. I was just wondering, before trying that, if we couldn't maybe do it in a little simpler way, but non-generic way. Using the approach you suggest (which I like): We need to define a couple of wrapper functions around start_subfile and record_line because their profile will not match the profile of our callbacks. We'll need 3 callbacks per case (3 for psymtabs, 3 for symtabs). We'll also have to use void* pointers if we want to make the context generic, which I would prefer to avoid. It's great for heavily reused code, but is this worth it for this function. What we could do instead, is: . Add a new parameter: struct partial_symtab *pst . If this pst is NULL, then scan for symtabs as before . If pst is non NULL, then scan for partial symtabs, and do not call the 2 buildsym.c functions. Just for illustration purposes, I tweaked my previous patch. Attached is a new patch that shows what it would become. I did this because even if we end up not choosing this solution, it's still a good intermediate version on which to implement your suggestion. I verified that the performance impact on GDB was still pretty small. If you like it, then I'll review, clean it up and properly submit it for review. Otherwise, I'll send a new version including your suggestion. Thanks, -- Joel [-- Attachment #2: dw2-includes.diff --] [-- Type: text/plain, Size: 15090 bytes --] Index: dwarf2read.c =================================================================== RCS file: /cvs/src/src/gdb/dwarf2read.c,v retrieving revision 1.142 diff -u -p -r1.142 dwarf2read.c --- dwarf2read.c 2 Apr 2004 04:35:46 -0000 1.142 +++ dwarf2read.c 15 Apr 2004 22:02:36 -0000 @@ -583,6 +583,13 @@ static void dwarf2_locate_sections (bfd static void dwarf2_build_psymtabs_easy (struct objfile *, int); #endif +static void dwarf2_create_include_psymtab (char *, struct partial_symtab *, + struct objfile *); + +static void dwarf2_build_include_psymtabs (struct dwarf2_cu *, + const unsigned int, + struct partial_symtab *); + static void dwarf2_build_psymtabs_hard (struct objfile *, int); static char *scan_partial_symbols (char *, CORE_ADDR *, CORE_ADDR *, @@ -629,7 +636,8 @@ static struct abbrev_info *dwarf2_lookup struct dwarf2_cu *); static char *read_partial_die (struct partial_die_info *, - bfd *, char *, struct dwarf2_cu *); + bfd *, char *, struct dwarf2_cu *, + unsigned int *); static char *read_full_die (struct die_info **, bfd *, char *, struct dwarf2_cu *, int *); @@ -684,12 +692,15 @@ static struct die_info *die_specificatio static void free_line_header (struct line_header *lh); +static void add_file_name (struct line_header *, char *, unsigned int, + unsigned int, unsigned int); + static struct line_header *(dwarf_decode_line_header (unsigned int offset, bfd *abfd, struct dwarf2_cu *cu)); static void dwarf_decode_lines (struct line_header *, char *, bfd *, - struct dwarf2_cu *); + struct dwarf2_cu *, struct partial_symtab *); static void dwarf2_start_subfile (char *, char *); @@ -1100,6 +1111,65 @@ read_comp_unit_head (struct comp_unit_he return info_ptr; } +/* Allocate a new partial symtab for file named NAME and mark this new + partial symtab as being an include of PST. */ + +static void +dwarf2_create_include_psymtab (char *name, struct partial_symtab *pst, + struct objfile *objfile) +{ + struct partial_symtab *subpst = allocate_psymtab (name, objfile); + + subpst->section_offsets = pst->section_offsets; + subpst->textlow = 0; + subpst->texthigh = 0; + + subpst->dependencies = (struct partial_symtab **) + obstack_alloc (&objfile->objfile_obstack, + sizeof (struct partial_symtab *)); + subpst->dependencies[0] = pst; + subpst->number_of_dependencies = 1; + + subpst->globals_offset = 0; + subpst->n_global_syms = 0; + subpst->statics_offset = 0; + subpst->n_static_syms = 0; + subpst->symtab = NULL; + subpst->read_symtab = pst->read_symtab; + subpst->readin = 0; + + /* No private part is necessary for include psymtabs. This property + can be used to differentiate between such include psymtabs and + the regular ones. If it ever happens that a regular psymtab can + legitimally have a NULL PST_PRIVATE part, then we'll have to add a + dedicated field for that in the dwarf2_pinfo structure. */ + PST_PRIVATE (subpst) = NULL; +} + +/* Read the line number information located at LINE_OFFSET, + and extract the list of sources files included by the + source file represented by PST. Build an include partial + symtab for each of these included files. */ + +static void +dwarf2_build_include_psymtabs (struct dwarf2_cu *cu, + const unsigned int line_offset, + struct partial_symtab *pst) +{ + struct objfile *objfile = cu->objfile; + bfd *abfd = objfile->obfd; + struct line_header *lh; + + lh = dwarf_decode_line_header (line_offset, abfd, cu); + if (lh == NULL) + return; /* No linetable, so no includes. */ + + dwarf_decode_lines (lh, NULL, abfd, cu, pst); + + free_line_header (lh); +} + + /* Build the partial symbol table by doing a quick pass through the .debug_info and .debug_abbrev sections. */ @@ -1168,6 +1238,8 @@ dwarf2_build_psymtabs_hard (struct objfi { struct cleanup *back_to_inner; struct dwarf2_cu cu; + unsigned int line_offset = 0; + beg_of_comp_unit = info_ptr; cu.objfile = objfile; @@ -1208,7 +1280,7 @@ dwarf2_build_psymtabs_hard (struct objfi /* Read the compilation unit die */ info_ptr = read_partial_die (&comp_unit_die, abfd, info_ptr, - &cu); + &cu, &line_offset); /* Set the language we're debugging */ set_cu_language (comp_unit_die.language, &cu); @@ -1269,6 +1341,10 @@ dwarf2_build_psymtabs_hard (struct objfi info_ptr = beg_of_comp_unit + cu.header.length + cu.header.initial_length_size; + /* Get the list of files included in the current compilation unit, + and build a psymtab for each of them. */ + dwarf2_build_include_psymtabs (&cu, line_offset, pst); + do_cleanups (back_to_inner); } do_cleanups (back_to); @@ -1300,7 +1376,7 @@ scan_partial_symbols (char *info_ptr, CO inside the loop. */ int info_ptr_updated = 0; - info_ptr = read_partial_die (&pdi, abfd, info_ptr, cu); + info_ptr = read_partial_die (&pdi, abfd, info_ptr, cu, NULL); /* Anonymous namespaces have no name but have interesting children, so we need to look at them. Ditto for anonymous @@ -1652,7 +1728,7 @@ add_partial_structure (struct partial_di struct partial_die_info child_pdi; next_child = read_partial_die (&child_pdi, abfd, next_child, - cu); + cu, NULL); if (!child_pdi.tag) break; if (child_pdi.tag == DW_TAG_subprogram) @@ -1691,7 +1767,7 @@ add_partial_enumeration (struct partial_ while (1) { - info_ptr = read_partial_die (&pdi, abfd, info_ptr, cu); + info_ptr = read_partial_die (&pdi, abfd, info_ptr, cu, NULL); if (pdi.tag == 0) break; if (pdi.tag != DW_TAG_enumerator || pdi.name == NULL) @@ -1916,6 +1992,32 @@ psymtab_to_symtab_1 (struct partial_symt struct cleanup *back_to; struct attribute *attr; CORE_ADDR baseaddr; + int i; + + for (i = 0; i < pst->number_of_dependencies; i++) + if (!pst->dependencies[i]->readin) + { + /* Inform about additional files that need to be read in. */ + if (info_verbose) + { + fputs_filtered (" ", gdb_stdout); + wrap_here (""); + fputs_filtered ("and ", gdb_stdout); + wrap_here (""); + printf_filtered ("%s...", pst->dependencies[i]->filename); + wrap_here (""); /* Flush output */ + gdb_flush (gdb_stdout); + } + psymtab_to_symtab_1 (pst->dependencies[i]); + } + + if (PST_PRIVATE (pst) == NULL) + { + /* It's an include file, no symbols to read for it. + Everything is in the parent symtab. */ + pst->readin = 1; + return; + } dwarf2_per_objfile = objfile_data (pst->objfile, dwarf2_objfile_data_key); @@ -2204,7 +2306,7 @@ read_file_scope (struct die_info *die, s { make_cleanup ((make_cleanup_ftype *) free_line_header, (void *) line_header); - dwarf_decode_lines (line_header, comp_dir, abfd, cu); + dwarf_decode_lines (line_header, comp_dir, abfd, cu, NULL); } } @@ -4414,11 +4516,14 @@ dwarf2_lookup_abbrev (unsigned int numbe return NULL; } -/* Read a minimal amount of information into the minimal die structure. */ +/* Read a minimal amount of information into the minimal die structure. + If not NULL, the offset where the Line Number Information data is + stored will be saved in LINE_OFFSET. */ static char * read_partial_die (struct partial_die_info *part_die, bfd *abfd, - char *info_ptr, struct dwarf2_cu *cu) + char *info_ptr, struct dwarf2_cu *cu, + unsigned int *line_offset) { unsigned int abbrev_number, bytes_read, i; struct abbrev_info *abbrev; @@ -4512,6 +4617,9 @@ read_partial_die (struct partial_die_inf part_die->sibling = dwarf2_per_objfile->info_buffer + dwarf2_get_ref_die_offset (&attr, cu); break; + case DW_AT_stmt_list: + if (line_offset != NULL) + *line_offset = DW_UNSND (&attr); default: break; } @@ -4527,7 +4635,7 @@ read_partial_die (struct partial_die_inf spec_ptr = dwarf2_per_objfile->info_buffer + dwarf2_get_ref_die_offset (&spec_attr, cu); - read_partial_die (&spec_die, abfd, spec_ptr, cu); + read_partial_die (&spec_die, abfd, spec_ptr, cu, NULL); if (spec_die.name) { part_die->name = spec_die.name; @@ -5399,7 +5507,7 @@ check_cu_functions (CORE_ADDR address, s static void dwarf_decode_lines (struct line_header *lh, char *comp_dir, bfd *abfd, - struct dwarf2_cu *cu) + struct dwarf2_cu *cu, struct partial_symtab *pst) { char *line_ptr; char *line_end; @@ -5408,6 +5516,38 @@ dwarf_decode_lines (struct line_header * CORE_ADDR baseaddr; struct objfile *objfile = cu->objfile; + /* When decoding the Line Number Program for the purpose of building + the partial symtabs included by the current CU, we need to do + the following: + + We first scan the Line Header. It contains a list of files referenced + by the Line Number Program. We then scan the Line Number Program + for opcodes changing the source file. For each file selected in + the program, we mark it as included using the FILE_IS_INCLUDED + array. Once we're finished scanning the Line Number Program, we can + then iterate over FILE_IS_INCLUDED and create a corresponding + include partial symtab for each file that was marked as included. + */ + + /* FILE_IS_INCLUDED is an array allocated on the heap, which size + is stored in FILE_IS_INCLUDED_SIZE. Each element of this array + corresponds to the file of the same index in the Line Header, + as stored in the line_header struct we built for the current unit. + Each element is initially set to zero, and then to nonzero if the + corresponding file is included. The size of this array may be + larger than necessary, and the number of meaningful entries is + stored in lh->num_file_names. */ + char *file_is_included = NULL; + int file_is_included_size = 0; + const int decode_for_pst_p = (pst != NULL); + + if (decode_for_pst_p) + { + file_is_included = xmalloc (lh->file_names_size * sizeof (char)); + memset (file_is_included, 0, lh->file_names_size * sizeof (char)); + file_is_included_size = lh->file_names_size; + } + baseaddr = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile)); line_ptr = lh->statement_program_start; @@ -5425,9 +5565,9 @@ dwarf_decode_lines (struct line_header * int basic_block = 0; int end_sequence = 0; - /* Start a subfile for the current file of the state machine. */ - if (lh->num_file_names >= file) + if (!decode_for_pst_p && lh->num_file_names >= file) { + /* Start a subfile for the current file of the state machine. */ /* lh->include_dirs and lh->file_names are 0-based, but the directory and file name numbers in the statement program are 1-based. */ @@ -5452,9 +5592,12 @@ dwarf_decode_lines (struct line_header * address += (adj_opcode / lh->line_range) * lh->minimum_instruction_length; line += lh->line_base + (adj_opcode % lh->line_range); - /* append row to matrix using current values */ - record_line (current_subfile, line, - check_cu_functions (address, cu)); + if (!decode_for_pst_p) + { + /* append row to matrix using current values */ + record_line (current_subfile, line, + check_cu_functions (address, cu)); + } basic_block = 1; } else switch (op_code) @@ -5467,7 +5610,8 @@ dwarf_decode_lines (struct line_header * { case DW_LNE_end_sequence: end_sequence = 1; - record_line (current_subfile, 0, address); + if (!decode_for_pst_p) + record_line (current_subfile, 0, address); break; case DW_LNE_set_address: address = read_address (abfd, line_ptr, cu, &bytes_read); @@ -5491,6 +5635,20 @@ dwarf_decode_lines (struct line_header * read_unsigned_leb128 (abfd, line_ptr, &bytes_read); line_ptr += bytes_read; add_file_name (lh, cur_file, dir_index, mod_time, length); + + if (decode_for_pst_p) + { + if (file_is_included_size != lh->file_names_size) + { + /* The add_file_name() operation above caused + the file_names array size in the line_header + struct to be increased. Increase our + file_is_included array size accordingly. */ + file_is_included = xrealloc (file_is_included, + lh->file_names_size); + } + file_is_included [lh->num_file_names - 1] = 0; + } } break; default: @@ -5500,8 +5658,9 @@ dwarf_decode_lines (struct line_header * } break; case DW_LNS_copy: - record_line (current_subfile, line, - check_cu_functions (address, cu)); + if (!decode_for_pst_p) + record_line (current_subfile, line, + check_cu_functions (address, cu)); basic_block = 0; break; case DW_LNS_advance_pc: @@ -5527,7 +5686,10 @@ dwarf_decode_lines (struct line_header * dir = lh->include_dirs[fe->dir_index - 1]; else dir = comp_dir; - dwarf2_start_subfile (fe->name, dir); + if (decode_for_pst_p) + file_is_included[file - 1] = 1; + else + dwarf2_start_subfile (fe->name, dir); } break; case DW_LNS_set_column: @@ -5564,6 +5726,22 @@ dwarf_decode_lines (struct line_header * } } } + } + + if (decode_for_pst_p) + { + int file_index; + + /* Now that we're done scanning the Line Header Program, we can + create the psymtab of each included file. */ + for (file_index = 0; file_index < lh->num_file_names; file_index++) + if (file_is_included[file_index] == 1) + { + char *include_name = lh->file_names [file_index].name; + + if (strcmp (include_name, pst->filename) != 0) + dwarf2_create_include_psymtab (include_name, pst, objfile); + } } } ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC/dwarf-2] Add support for included files 2004-04-15 22:13 ` Joel Brobecker @ 2004-04-16 4:24 ` Jim Blandy 2004-04-16 4:28 ` Joel Brobecker 2004-04-16 23:08 ` Joel Brobecker 0 siblings, 2 replies; 22+ messages in thread From: Jim Blandy @ 2004-04-16 4:24 UTC (permalink / raw) To: Joel Brobecker; +Cc: Daniel Jacobowitz, Eli Zaretskii, gdb-patches Joel Brobecker <brobecker@gnat.com> writes: > Using the approach you suggest (which I like): We need to define a > couple of wrapper functions around start_subfile and record_line > because their profile will not match the profile of our callbacks. > We'll need 3 callbacks per case (3 for psymtabs, 3 for symtabs). > We'll also have to use void* pointers if we want to make the context > generic, which I would prefer to avoid. It's great for heavily reused > code, but is this worth it for this function. Maybe not. If you add a comment above dwarf_decode_lines explaining how it's called, and that PST determines whether we're building psymtabs or symtabs, we'll call it even. I just approved Daniel's mondo psymtab construction patch, so one of you is going to get in the others' way. Have fun. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC/dwarf-2] Add support for included files 2004-04-16 4:24 ` Jim Blandy @ 2004-04-16 4:28 ` Joel Brobecker 2004-04-16 23:08 ` Joel Brobecker 1 sibling, 0 replies; 22+ messages in thread From: Joel Brobecker @ 2004-04-16 4:28 UTC (permalink / raw) To: Jim Blandy; +Cc: Daniel Jacobowitz, Eli Zaretskii, gdb-patches > If you add a comment above dwarf_decode_lines explaining how it's > called, and that PST determines whether we're building psymtabs or > symtabs, we'll call it even. Understood. > I just approved Daniel's mondo psymtab construction patch, so one of > you is going to get in the others' way. Have fun. My changes will be quite easy to reproduce, I believe, so I'll let him go first. Revised patch coming up... Thanks! -- Joel ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC/dwarf-2] Add support for included files 2004-04-16 4:24 ` Jim Blandy 2004-04-16 4:28 ` Joel Brobecker @ 2004-04-16 23:08 ` Joel Brobecker 2004-04-29 23:32 ` Jim Blandy 1 sibling, 1 reply; 22+ messages in thread From: Joel Brobecker @ 2004-04-16 23:08 UTC (permalink / raw) To: Jim Blandy; +Cc: Daniel Jacobowitz, Eli Zaretskii, gdb-patches [-- Attachment #1: Type: text/plain, Size: 1644 bytes --] Hello Jim, > If you add a comment above dwarf_decode_lines explaining how it's > called, and that PST determines whether we're building psymtabs or > symtabs, we'll call it even. > > I just approved Daniel's mondo psymtab construction patch, so one of > you is going to get in the others' way. Have fun. Here is an updated version of the previous patch, adapted to fit in the current dwarf2read.c after Daniel's changes, and with comments added. It's actually slightly smaller thanks to Daniel's changes :-). 2004-04-16 Joel Brobecker <brobecker@gnat.com> * dwarf2read.c (dwarf2_create_include_psymtab): New function. (dwarf2_build_include_psymtabs): New function. (read_partial_die): Add new parameter. (add_file_name): Add forward declaration. (dwarf_decode_lines): Add new parameter. Enhance this procedure to be able to determine the list of files included by the given unit, and build their associated psymtabs. (dwarf2_build_psymtabs_hard): Build the psymtabs for the included files as well. (psymtab_to_symtab_1): Build the symtabs of all dependencies as well. (read_file_scope): Update call to read_partial_die. (load_partial_dies): Likewise. Tested on x86-linux. No regression, and it fixes the following two KFAILs: KFAIL: gdb.base/sep.exp: list using location inside included file (PRMS: symtab/1607) KFAIL: gdb.base/sep.exp: breakpoint inside included file (PRMS: symtab/1607) If this patch is approved, I'll send another one removing the setup_kfails. Thanks, -- Joel [-- Attachment #2: separate.diff --] [-- Type: text/plain, Size: 15326 bytes --] Index: dwarf2read.c =================================================================== RCS file: /cvs/src/src/gdb/dwarf2read.c,v retrieving revision 1.146 diff -u -p -r1.146 dwarf2read.c --- dwarf2read.c 16 Apr 2004 16:12:52 -0000 1.146 +++ dwarf2read.c 16 Apr 2004 23:03:07 -0000 @@ -634,6 +634,13 @@ static void dwarf2_locate_sections (bfd static void dwarf2_build_psymtabs_easy (struct objfile *, int); #endif +static void dwarf2_create_include_psymtab (char *, struct partial_symtab *, + struct objfile *); + +static void dwarf2_build_include_psymtabs (struct dwarf2_cu *, + const unsigned int, + struct partial_symtab *); + static void dwarf2_build_psymtabs_hard (struct objfile *, int); static void scan_partial_symbols (struct partial_die_info *, @@ -677,7 +684,8 @@ static struct partial_die_info *load_par static char *read_partial_die (struct partial_die_info *, struct abbrev_info *abbrev, unsigned int, - bfd *, char *, struct dwarf2_cu *); + bfd *, char *, struct dwarf2_cu *, + unsigned int *); static struct partial_die_info *find_partial_die (unsigned long, struct dwarf2_cu *, @@ -739,12 +747,15 @@ static struct die_info *die_specificatio static void free_line_header (struct line_header *lh); +static void add_file_name (struct line_header *, char *, unsigned int, + unsigned int, unsigned int); + static struct line_header *(dwarf_decode_line_header (unsigned int offset, bfd *abfd, struct dwarf2_cu *cu)); static void dwarf_decode_lines (struct line_header *, char *, bfd *, - struct dwarf2_cu *); + struct dwarf2_cu *, struct partial_symtab *); static void dwarf2_start_subfile (char *, char *); @@ -1196,6 +1207,65 @@ partial_read_comp_unit_head (struct comp return info_ptr; } +/* Allocate a new partial symtab for file named NAME and mark this new + partial symtab as being an include of PST. */ + +static void +dwarf2_create_include_psymtab (char *name, struct partial_symtab *pst, + struct objfile *objfile) +{ + struct partial_symtab *subpst = allocate_psymtab (name, objfile); + + subpst->section_offsets = pst->section_offsets; + subpst->textlow = 0; + subpst->texthigh = 0; + + subpst->dependencies = (struct partial_symtab **) + obstack_alloc (&objfile->objfile_obstack, + sizeof (struct partial_symtab *)); + subpst->dependencies[0] = pst; + subpst->number_of_dependencies = 1; + + subpst->globals_offset = 0; + subpst->n_global_syms = 0; + subpst->statics_offset = 0; + subpst->n_static_syms = 0; + subpst->symtab = NULL; + subpst->read_symtab = pst->read_symtab; + subpst->readin = 0; + + /* No private part is necessary for include psymtabs. This property + can be used to differentiate between such include psymtabs and + the regular ones. If it ever happens that a regular psymtab can + legitimally have a NULL PST_PRIVATE part, then we'll have to add a + dedicated field for that in the dwarf2_pinfo structure. */ + PST_PRIVATE (subpst) = NULL; +} + +/* Read the line number information located at LINE_OFFSET, + and extract the list of sources files included by the + source file represented by PST. Build an include partial + symtab for each of these included files. */ + +static void +dwarf2_build_include_psymtabs (struct dwarf2_cu *cu, + const unsigned int line_offset, + struct partial_symtab *pst) +{ + struct objfile *objfile = cu->objfile; + bfd *abfd = objfile->obfd; + struct line_header *lh; + + lh = dwarf_decode_line_header (line_offset, abfd, cu); + if (lh == NULL) + return; /* No linetable, so no includes. */ + + dwarf_decode_lines (lh, NULL, abfd, cu, pst); + + free_line_header (lh); +} + + /* Build the partial symbol table by doing a quick pass through the .debug_info and .debug_abbrev sections. */ @@ -1266,6 +1336,7 @@ dwarf2_build_psymtabs_hard (struct objfi struct abbrev_info *abbrev; unsigned int bytes_read; struct dwarf2_per_cu_data *this_cu; + unsigned int line_offset = 0; beg_of_comp_unit = info_ptr; @@ -1294,7 +1365,7 @@ dwarf2_build_psymtabs_hard (struct objfi /* Read the compilation unit die */ abbrev = peek_die_abbrev (info_ptr, &bytes_read, &cu); info_ptr = read_partial_die (&comp_unit_die, abbrev, bytes_read, - abfd, info_ptr, &cu); + abfd, info_ptr, &cu, &line_offset); /* Set the language we're debugging */ set_cu_language (comp_unit_die.language, &cu); @@ -1358,6 +1429,10 @@ dwarf2_build_psymtabs_hard (struct objfi info_ptr = beg_of_comp_unit + cu.header.length + cu.header.initial_length_size; + /* Get the list of files included in the current compilation unit, + and build a psymtab for each of them. */ + dwarf2_build_include_psymtabs (&cu, line_offset, pst); + do_cleanups (back_to_inner); } do_cleanups (back_to); @@ -2041,6 +2116,32 @@ psymtab_to_symtab_1 (struct partial_symt struct cleanup *back_to; struct attribute *attr; CORE_ADDR baseaddr; + int i; + + for (i = 0; i < pst->number_of_dependencies; i++) + if (!pst->dependencies[i]->readin) + { + /* Inform about additional files that need to be read in. */ + if (info_verbose) + { + fputs_filtered (" ", gdb_stdout); + wrap_here (""); + fputs_filtered ("and ", gdb_stdout); + wrap_here (""); + printf_filtered ("%s...", pst->dependencies[i]->filename); + wrap_here (""); /* Flush output */ + gdb_flush (gdb_stdout); + } + psymtab_to_symtab_1 (pst->dependencies[i]); + } + + if (PST_PRIVATE (pst) == NULL) + { + /* It's an include file, no symbols to read for it. + Everything is in the parent symtab. */ + pst->readin = 1; + return; + } dwarf2_per_objfile = objfile_data (pst->objfile, dwarf2_objfile_data_key); @@ -2329,7 +2430,7 @@ read_file_scope (struct die_info *die, s { make_cleanup ((make_cleanup_ftype *) free_line_header, (void *) line_header); - dwarf_decode_lines (line_header, comp_dir, abfd, cu); + dwarf_decode_lines (line_header, comp_dir, abfd, cu, NULL); } } @@ -4636,7 +4737,7 @@ load_partial_dies (bfd *abfd, char *info } info_ptr = read_partial_die (part_die, abbrev, bytes_read, - abfd, info_ptr, cu); + abfd, info_ptr, cu, NULL); /* This two-pass algorithm for processing partial symbols has a high cost in cache pressure. Thus, handle some simple cases @@ -4768,13 +4869,16 @@ load_partial_dies (bfd *abfd, char *info } } -/* Read a minimal amount of information into the minimal die structure. */ +/* Read a minimal amount of information into the minimal die structure. + If not NULL, the offset where the Line Number Information data is + stored will be saved in LINE_OFFSET. */ static char * read_partial_die (struct partial_die_info *part_die, struct abbrev_info *abbrev, unsigned int abbrev_len, bfd *abfd, - char *info_ptr, struct dwarf2_cu *cu) + char *info_ptr, struct dwarf2_cu *cu, + unsigned int *line_offset) { unsigned int bytes_read, i; struct attribute attr; @@ -4861,6 +4965,9 @@ read_partial_die (struct partial_die_inf part_die->sibling = dwarf2_per_objfile->info_buffer + dwarf2_get_ref_die_offset (&attr, cu); break; + case DW_AT_stmt_list: + if (line_offset != NULL) + *line_offset = DW_UNSND (&attr); default: break; } @@ -5801,13 +5908,23 @@ check_cu_functions (CORE_ADDR address, s return fn->lowpc; } -/* Decode the line number information for the compilation unit whose - line number info is at OFFSET in the .debug_line section. - The compilation directory of the file is passed in COMP_DIR. */ +/* Decode the Line Number Program (LNP) for the given line_header + structure and CU. The actual information extracted and the type + of structures created from the LNP depends on the value of PST. + + 1. If PST is NULL, then this procedure uses the data from the program + to create all necessary symbol tables, and their linetables. + The compilation directory of the file is passed in COMP_DIR, + and must not be NULL. + + 2. If PST is not NULL, this procedure reads the program to determine + the list of files included by the unit represented by PST, and + builds all the associated partial symbol tables. In this case, + the value of COMP_DIR is ignored, and can thus be NULL. */ static void dwarf_decode_lines (struct line_header *lh, char *comp_dir, bfd *abfd, - struct dwarf2_cu *cu) + struct dwarf2_cu *cu, struct partial_symtab *pst) { char *line_ptr; char *line_end; @@ -5816,6 +5933,40 @@ dwarf_decode_lines (struct line_header * CORE_ADDR baseaddr; struct objfile *objfile = cu->objfile; + /* When decoding the Line Number Program for the purpose of building + the partial symtabs included by the current CU, we need to do + the following: + + We first scan the Line Header. It contains a list of files referenced + by the Line Number Program. We then scan the Line Number Program + for opcodes changing the source file. For each file selected in + the program, we mark it as included using the FILE_IS_INCLUDED + array. Once we're finished scanning the Line Number Program, we can + then iterate over FILE_IS_INCLUDED and create a corresponding + include partial symtab for each file that was marked as included. */ + + /* FILE_IS_INCLUDED is only used when we're creating the psymtabs + for the files included by the current unit. + + It is an array allocated on the heap, which size is stored in + SIZE_OF_FILE_IS_INCLUDED. Each element of this array corresponds + to the file of the same index in the Line Header, as stored in + the line_header struct we built for the current unit. Each element + is initially set to zero, and then to nonzero if the corresponding + file is included. The size of this array may be larger than + necessary, and the number of meaningful entries is stored in + lh->num_file_names. */ + char *file_is_included = NULL; + int size_of_file_is_included = 0; + const int decode_for_pst_p = (pst != NULL); + + if (decode_for_pst_p) + { + file_is_included = xmalloc (lh->file_names_size * sizeof (char)); + memset (file_is_included, 0, lh->file_names_size * sizeof (char)); + size_of_file_is_included = lh->file_names_size; + } + baseaddr = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile)); line_ptr = lh->statement_program_start; @@ -5833,9 +5984,9 @@ dwarf_decode_lines (struct line_header * int basic_block = 0; int end_sequence = 0; - /* Start a subfile for the current file of the state machine. */ - if (lh->num_file_names >= file) + if (!decode_for_pst_p && lh->num_file_names >= file) { + /* Start a subfile for the current file of the state machine. */ /* lh->include_dirs and lh->file_names are 0-based, but the directory and file name numbers in the statement program are 1-based. */ @@ -5860,9 +6011,12 @@ dwarf_decode_lines (struct line_header * address += (adj_opcode / lh->line_range) * lh->minimum_instruction_length; line += lh->line_base + (adj_opcode % lh->line_range); - /* append row to matrix using current values */ - record_line (current_subfile, line, - check_cu_functions (address, cu)); + if (!decode_for_pst_p) + { + /* append row to matrix using current values */ + record_line (current_subfile, line, + check_cu_functions (address, cu)); + } basic_block = 1; } else switch (op_code) @@ -5875,7 +6029,8 @@ dwarf_decode_lines (struct line_header * { case DW_LNE_end_sequence: end_sequence = 1; - record_line (current_subfile, 0, address); + if (!decode_for_pst_p) + record_line (current_subfile, 0, address); break; case DW_LNE_set_address: address = read_address (abfd, line_ptr, cu, &bytes_read); @@ -5899,6 +6054,20 @@ dwarf_decode_lines (struct line_header * read_unsigned_leb128 (abfd, line_ptr, &bytes_read); line_ptr += bytes_read; add_file_name (lh, cur_file, dir_index, mod_time, length); + + if (decode_for_pst_p) + { + if (size_of_file_is_included != lh->file_names_size) + { + /* The add_file_name() operation above caused + the file_names array size in the line_header + struct to be increased. Increase our + file_is_included array size accordingly. */ + file_is_included = xrealloc (file_is_included, + lh->file_names_size); + } + file_is_included [lh->num_file_names - 1] = 0; + } } break; default: @@ -5908,8 +6077,9 @@ dwarf_decode_lines (struct line_header * } break; case DW_LNS_copy: - record_line (current_subfile, line, - check_cu_functions (address, cu)); + if (!decode_for_pst_p) + record_line (current_subfile, line, + check_cu_functions (address, cu)); basic_block = 0; break; case DW_LNS_advance_pc: @@ -5935,7 +6105,10 @@ dwarf_decode_lines (struct line_header * dir = lh->include_dirs[fe->dir_index - 1]; else dir = comp_dir; - dwarf2_start_subfile (fe->name, dir); + if (decode_for_pst_p) + file_is_included[file - 1] = 1; + else + dwarf2_start_subfile (fe->name, dir); } break; case DW_LNS_set_column: @@ -5972,6 +6145,22 @@ dwarf_decode_lines (struct line_header * } } } + } + + if (decode_for_pst_p) + { + int file_index; + + /* Now that we're done scanning the Line Header Program, we can + create the psymtab of each included file. */ + for (file_index = 0; file_index < lh->num_file_names; file_index++) + if (file_is_included[file_index] == 1) + { + char *include_name = lh->file_names [file_index].name; + + if (strcmp (include_name, pst->filename) != 0) + dwarf2_create_include_psymtab (include_name, pst, objfile); + } } } ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC/dwarf-2] Add support for included files 2004-04-16 23:08 ` Joel Brobecker @ 2004-04-29 23:32 ` Jim Blandy 2004-05-01 1:14 ` Joel Brobecker 0 siblings, 1 reply; 22+ messages in thread From: Jim Blandy @ 2004-04-29 23:32 UTC (permalink / raw) To: Joel Brobecker; +Cc: Daniel Jacobowitz, Eli Zaretskii, gdb-patches Joel Brobecker <brobecker@gnat.com> writes: > Here is an updated version of the previous patch, adapted to fit in > the current dwarf2read.c after Daniel's changes, and with comments > added. It's actually slightly smaller thanks to Daniel's changes > :-). Great! I actually have still more comments: - Rather than passing 'line_offset' by reference to read_partial_die, could you add a line_offset field to 'struct partial_die_info, and have read_partial_die set that? That would seem more consistent with the rest of what read_partial_die does. Then 'dwarf2_build_include_psymtabs' should take a pointer to the CU die. That's assuming Daniel doesn't see any problem enlarging partial_die_info. If he does, maybe we could add the fields to 'struct dwarf2_cu' instead: it's a little odd to have 'read_partial_die' set the cu's fields directly, but not horribly so, as long as the die's tag is DW_TAG_compile_unit. - Is it really okay to pass NULL to dwarf_decode_lines for comp_dir? Won't the filenames of the partial symbol tables be different from those of the symbol tables? To fix this, read_partial_die would have to check for DW_AT_comp_dir attributes, too. - It seems to me the file_is_included array should be in struct line_header, with add_file_name managing its allocation / initialization in parallel with file_names. It's already the case that that structure is mutated by the act of processing the line number program, so I don't see that it's a serious change to the usage patterns of that structure type. - I'd rather see the call to dwarf2_build_include_psymtabs before the assignment to info_ptr, not after. Not that it matters much. - Thanks for fixing the comments on dwarf_decode_lines --- especially the references to arguments that had been missing for some time now. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC/dwarf-2] Add support for included files 2004-04-29 23:32 ` Jim Blandy @ 2004-05-01 1:14 ` Joel Brobecker 2004-05-01 4:57 ` Jim Blandy 0 siblings, 1 reply; 22+ messages in thread From: Joel Brobecker @ 2004-05-01 1:14 UTC (permalink / raw) To: Jim Blandy; +Cc: Daniel Jacobowitz, Eli Zaretskii, gdb-patches [-- Attachment #1: Type: text/plain, Size: 3813 bytes --] > I actually have still more comments: Jim, you'll be happy, your comments make the patch neater :-). Once you get to know the data structures a little bit more (I really should have studied them a little harder before working on this), everything seems to be falling into place almost naturally... Attached is a revised version of the patch, up to date as of now, and incorporating all your comments, except one which I wasn't sure about. > - Rather than passing 'line_offset' by reference to read_partial_die, > could you add a line_offset field to 'struct partial_die_info, and have > read_partial_die set that? That would seem more consistent with the > rest of what read_partial_die does. Then > 'dwarf2_build_include_psymtabs' should take a pointer to the CU die. This brings a nice cleanup. > That's assuming Daniel doesn't see any problem enlarging > partial_die_info. If he does, maybe we could add the fields to > 'struct dwarf2_cu' instead: it's a little odd to have > 'read_partial_die' set the cu's fields directly, but not horribly > so, as long as the die's tag is DW_TAG_compile_unit. partial_die_info seems better suited to hold that information. Hopefully Daniel won't object. > - Is it really okay to pass NULL to dwarf_decode_lines for comp_dir? > Won't the filenames of the partial symbol tables be different from > those of the symbol tables? To fix this, read_partial_die would > have to check for DW_AT_comp_dir attributes, too. I don't think we need to worry about the comp_dir when building the psymtabs for included files. The filename used for the psymtab is the same as the filename used to build the symtab (checked it by inspecting start_subfile() and end_symtab()). The only thing we could gain from computing the comp_dir as well during that phase is to compute the psymtab fullname. But then comp_dir is used to set the symtab dirname, not to compute the fullname. So it doesn't help improve the consistency between psymtab and symtab. I think that part is fine... ... At least for now :-). > - It seems to me the file_is_included array should be in struct > line_header, with add_file_name managing its allocation / > initialization in parallel with file_names. It's already the case > that that structure is mutated by the act of processing the line > number program, so I don't see that it's a serious change to the > usage patterns of that structure type. Good idea. I included it, and the code becomes one notch cleaner. > - I'd rather see the call to dwarf2_build_include_psymtabs before the > assignment to info_ptr, not after. Not that it matters much. Done. > - Thanks for fixing the comments on dwarf_decode_lines --- especially > the references to arguments that had been missing for some time now. My pleasure :-). 2004-04-30 J. Brobecker <brobecker@gnat.com> * dwarf2read.c (line_header): Add new included_p field in field file_names. (partial_die_info): New field has_stmt_list. New field line_offset. (dwarf2_create_include_psymtab): New function. (dwarf2_build_include_psymtabs): New function. (add_file_name): Add forward declaration. Initialize new field. (dwarf_decode_lines): Add new parameter. Enhance this procedure to be able to determine the list of files included by the given unit, and build their associated psymtabs. (dwarf2_build_psymtabs_hard): Build the psymtabs for the included files as well. (psymtab_to_symtab_1): Build the symtabs of all dependencies as well. (read_file_scope): Update call to dwarf_decode_lines. (read_partial_die): Handle DW_AT_stmt_list attributes. Tested on x86-linux, no regression. Fixes the two sep.exp KFAILS. OK to apply? Thanks, -- Joel [-- Attachment #2: separate2.diff --] [-- Type: text/plain, Size: 12133 bytes --] Index: dwarf2read.c =================================================================== RCS file: /cvs/src/src/gdb/dwarf2read.c,v retrieving revision 1.148 diff -u -p -r1.148 dwarf2read.c --- dwarf2read.c 19 Apr 2004 18:20:50 -0000 1.148 +++ dwarf2read.c 1 May 2004 01:07:21 -0000 @@ -341,6 +341,7 @@ struct line_header unsigned int dir_index; unsigned int mod_time; unsigned int length; + int included_p; /* Non-zero if referenced by the Line Number Program. */ } *file_names; /* The start and end of the statement program following this @@ -368,6 +369,7 @@ struct partial_die_info unsigned int is_declaration : 1; unsigned int has_type : 1; unsigned int has_specification : 1; + unsigned int has_stmt_list : 1; unsigned int has_pc_info : 1; /* Flag set if the SCOPE field of this structure has been @@ -400,6 +402,9 @@ struct partial_die_info DW_AT_extension). */ unsigned int spec_offset; + /* If HAS_STMT_LIST, the offset of the Line Number Information data. */ + unsigned int line_offset; + /* Pointers to this DIE's parent, first child, and next sibling, if any. */ struct partial_die_info *die_parent, *die_child, *die_sibling; @@ -631,6 +636,13 @@ static void dwarf2_locate_sections (bfd static void dwarf2_build_psymtabs_easy (struct objfile *, int); #endif +static void dwarf2_create_include_psymtab (char *, struct partial_symtab *, + struct objfile *); + +static void dwarf2_build_include_psymtabs (struct dwarf2_cu *, + struct partial_die_info *, + struct partial_symtab *); + static void dwarf2_build_psymtabs_hard (struct objfile *, int); static void scan_partial_symbols (struct partial_die_info *, @@ -739,12 +751,15 @@ static struct die_info *die_specificatio static void free_line_header (struct line_header *lh); +static void add_file_name (struct line_header *, char *, unsigned int, + unsigned int, unsigned int); + static struct line_header *(dwarf_decode_line_header (unsigned int offset, bfd *abfd, struct dwarf2_cu *cu)); static void dwarf_decode_lines (struct line_header *, char *, bfd *, - struct dwarf2_cu *); + struct dwarf2_cu *, struct partial_symtab *); static void dwarf2_start_subfile (char *, char *); @@ -1194,6 +1209,68 @@ partial_read_comp_unit_head (struct comp return info_ptr; } +/* Allocate a new partial symtab for file named NAME and mark this new + partial symtab as being an include of PST. */ + +static void +dwarf2_create_include_psymtab (char *name, struct partial_symtab *pst, + struct objfile *objfile) +{ + struct partial_symtab *subpst = allocate_psymtab (name, objfile); + + subpst->section_offsets = pst->section_offsets; + subpst->textlow = 0; + subpst->texthigh = 0; + + subpst->dependencies = (struct partial_symtab **) + obstack_alloc (&objfile->objfile_obstack, + sizeof (struct partial_symtab *)); + subpst->dependencies[0] = pst; + subpst->number_of_dependencies = 1; + + subpst->globals_offset = 0; + subpst->n_global_syms = 0; + subpst->statics_offset = 0; + subpst->n_static_syms = 0; + subpst->symtab = NULL; + subpst->read_symtab = pst->read_symtab; + subpst->readin = 0; + + /* No private part is necessary for include psymtabs. This property + can be used to differentiate between such include psymtabs and + the regular ones. If it ever happens that a regular psymtab can + legitimally have a NULL PST_PRIVATE part, then we'll have to add a + dedicated field for that in the dwarf2_pinfo structure. */ + PST_PRIVATE (subpst) = NULL; +} + +/* Read the Line Number Program data and extract the list of files + included by the source file represented by PST. Build an include + partial symtab for each of these included files. + + This procedure assumes that there *is* a Line Number Program in + the given CU. Callers should check that PDI->HAS_STMT_LIST is set + before calling this procedure. */ + +static void +dwarf2_build_include_psymtabs (struct dwarf2_cu *cu, + struct partial_die_info *pdi, + struct partial_symtab *pst) +{ + struct objfile *objfile = cu->objfile; + bfd *abfd = objfile->obfd; + struct line_header *lh; + + lh = dwarf_decode_line_header (pdi->line_offset, abfd, cu); + if (lh == NULL) + return; /* No linetable, so no includes. */ + + dwarf_decode_lines (lh, NULL, abfd, cu, pst); + + free_line_header (lh); +} + + /* Build the partial symbol table by doing a quick pass through the .debug_info and .debug_abbrev sections. */ @@ -1321,6 +1398,13 @@ dwarf2_build_psymtabs_hard (struct objfi also happen.) This happens in VxWorks. */ free_named_symtabs (pst->filename); + if (comp_unit_die.has_stmt_list) + { + /* Get the list of files included in the current compilation unit, + and build a psymtab for each of them. */ + dwarf2_build_include_psymtabs (&cu, &comp_unit_die, pst); + } + info_ptr = beg_of_comp_unit + cu.header.length + cu.header.initial_length_size; @@ -2006,6 +2090,32 @@ psymtab_to_symtab_1 (struct partial_symt struct cleanup *back_to; struct attribute *attr; CORE_ADDR baseaddr; + int i; + + for (i = 0; i < pst->number_of_dependencies; i++) + if (!pst->dependencies[i]->readin) + { + /* Inform about additional files that need to be read in. */ + if (info_verbose) + { + fputs_filtered (" ", gdb_stdout); + wrap_here (""); + fputs_filtered ("and ", gdb_stdout); + wrap_here (""); + printf_filtered ("%s...", pst->dependencies[i]->filename); + wrap_here (""); /* Flush output */ + gdb_flush (gdb_stdout); + } + psymtab_to_symtab_1 (pst->dependencies[i]); + } + + if (PST_PRIVATE (pst) == NULL) + { + /* It's an include file, no symbols to read for it. + Everything is in the parent symtab. */ + pst->readin = 1; + return; + } dwarf2_per_objfile = objfile_data (pst->objfile, dwarf2_objfile_data_key); @@ -2294,7 +2404,7 @@ read_file_scope (struct die_info *die, s { make_cleanup ((make_cleanup_ftype *) free_line_header, (void *) line_header); - dwarf_decode_lines (line_header, comp_dir, abfd, cu); + dwarf_decode_lines (line_header, comp_dir, abfd, cu, NULL); } } @@ -4826,6 +4936,10 @@ read_partial_die (struct partial_die_inf part_die->sibling = dwarf2_per_objfile->info_buffer + dwarf2_get_ref_die_offset (&attr, cu); break; + case DW_AT_stmt_list: + part_die->has_stmt_list = 1; + part_die->line_offset = DW_UNSND (&attr); + break; default: break; } @@ -5628,6 +5742,7 @@ add_file_name (struct line_header *lh, fe->dir_index = dir_index; fe->mod_time = mod_time; fe->length = length; + fe->included_p = 0; } @@ -5785,13 +5900,23 @@ check_cu_functions (CORE_ADDR address, s return fn->lowpc; } -/* Decode the line number information for the compilation unit whose - line number info is at OFFSET in the .debug_line section. - The compilation directory of the file is passed in COMP_DIR. */ +/* Decode the Line Number Program (LNP) for the given line_header + structure and CU. The actual information extracted and the type + of structures created from the LNP depends on the value of PST. + + 1. If PST is NULL, then this procedure uses the data from the program + to create all necessary symbol tables, and their linetables. + The compilation directory of the file is passed in COMP_DIR, + and must not be NULL. + + 2. If PST is not NULL, this procedure reads the program to determine + the list of files included by the unit represented by PST, and + builds all the associated partial symbol tables. In this case, + the value of COMP_DIR is ignored, and can thus be NULL. */ static void dwarf_decode_lines (struct line_header *lh, char *comp_dir, bfd *abfd, - struct dwarf2_cu *cu) + struct dwarf2_cu *cu, struct partial_symtab *pst) { char *line_ptr; char *line_end; @@ -5799,6 +5924,7 @@ dwarf_decode_lines (struct line_header * unsigned char op_code, extended_op, adj_opcode; CORE_ADDR baseaddr; struct objfile *objfile = cu->objfile; + const int decode_for_pst_p = (pst != NULL); baseaddr = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile)); @@ -5817,9 +5943,9 @@ dwarf_decode_lines (struct line_header * int basic_block = 0; int end_sequence = 0; - /* Start a subfile for the current file of the state machine. */ - if (lh->num_file_names >= file) + if (!decode_for_pst_p && lh->num_file_names >= file) { + /* Start a subfile for the current file of the state machine. */ /* lh->include_dirs and lh->file_names are 0-based, but the directory and file name numbers in the statement program are 1-based. */ @@ -5844,9 +5970,12 @@ dwarf_decode_lines (struct line_header * address += (adj_opcode / lh->line_range) * lh->minimum_instruction_length; line += lh->line_base + (adj_opcode % lh->line_range); - /* append row to matrix using current values */ - record_line (current_subfile, line, - check_cu_functions (address, cu)); + if (!decode_for_pst_p) + { + /* append row to matrix using current values */ + record_line (current_subfile, line, + check_cu_functions (address, cu)); + } basic_block = 1; } else switch (op_code) @@ -5859,7 +5988,8 @@ dwarf_decode_lines (struct line_header * { case DW_LNE_end_sequence: end_sequence = 1; - record_line (current_subfile, 0, address); + if (!decode_for_pst_p) + record_line (current_subfile, 0, address); break; case DW_LNE_set_address: address = read_address (abfd, line_ptr, cu, &bytes_read); @@ -5892,8 +6022,9 @@ dwarf_decode_lines (struct line_header * } break; case DW_LNS_copy: - record_line (current_subfile, line, - check_cu_functions (address, cu)); + if (!decode_for_pst_p) + record_line (current_subfile, line, + check_cu_functions (address, cu)); basic_block = 0; break; case DW_LNS_advance_pc: @@ -5915,11 +6046,13 @@ dwarf_decode_lines (struct line_header * file = read_unsigned_leb128 (abfd, line_ptr, &bytes_read); line_ptr += bytes_read; fe = &lh->file_names[file - 1]; + fe->included_p = 1; if (fe->dir_index) dir = lh->include_dirs[fe->dir_index - 1]; else dir = comp_dir; - dwarf2_start_subfile (fe->name, dir); + if (!decode_for_pst_p) + dwarf2_start_subfile (fe->name, dir); } break; case DW_LNS_set_column: @@ -5956,6 +6089,22 @@ dwarf_decode_lines (struct line_header * } } } + } + + if (decode_for_pst_p) + { + int file_index; + + /* Now that we're done scanning the Line Header Program, we can + create the psymtab of each included file. */ + for (file_index = 0; file_index < lh->num_file_names; file_index++) + if (lh->file_names[file_index].included_p == 1) + { + char *include_name = lh->file_names [file_index].name; + + if (strcmp (include_name, pst->filename) != 0) + dwarf2_create_include_psymtab (include_name, pst, objfile); + } } } ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC/dwarf-2] Add support for included files 2004-05-01 1:14 ` Joel Brobecker @ 2004-05-01 4:57 ` Jim Blandy 2004-05-03 16:25 ` Joel Brobecker 0 siblings, 1 reply; 22+ messages in thread From: Jim Blandy @ 2004-05-01 4:57 UTC (permalink / raw) To: Joel Brobecker; +Cc: Daniel Jacobowitz, Eli Zaretskii, gdb-patches Joel Brobecker <brobecker@gnat.com> writes: > > - Is it really okay to pass NULL to dwarf_decode_lines for comp_dir? > > Won't the filenames of the partial symbol tables be different from > > those of the symbol tables? To fix this, read_partial_die would > > have to check for DW_AT_comp_dir attributes, too. > > I don't think we need to worry about the comp_dir when building > the psymtabs for included files. The filename used for the psymtab > is the same as the filename used to build the symtab (checked it > by inspecting start_subfile() and end_symtab()). The only thing > we could gain from computing the comp_dir as well during that phase > is to compute the psymtab fullname. But then comp_dir is used > to set the symtab dirname, not to compute the fullname. So it doesn't > help improve the consistency between psymtab and symtab. Let me make sure I've got it right. When we're calling dwarf_decode_lines for the partial symbol tables, we never call dwarf2_start_subfile anyway --- that's for building full symbol tables --- so the use of 'dirname' in that function doesn't matter. And since it's never used to compute the full name of the symtabs, it doesn't introduce inconsistencies to omit it. If that's correct, then this patch is ready to commit. Thanks very much! ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC/dwarf-2] Add support for included files 2004-05-01 4:57 ` Jim Blandy @ 2004-05-03 16:25 ` Joel Brobecker 0 siblings, 0 replies; 22+ messages in thread From: Joel Brobecker @ 2004-05-03 16:25 UTC (permalink / raw) To: Jim Blandy; +Cc: Daniel Jacobowitz, Eli Zaretskii, gdb-patches > Let me make sure I've got it right. When we're calling > dwarf_decode_lines for the partial symbol tables, we never call > dwarf2_start_subfile anyway --- that's for building full symbol tables > --- so the use of 'dirname' in that function doesn't matter. And > since it's never used to compute the full name of the symtabs, it > doesn't introduce inconsistencies to omit it. That's right. I updated a bit the comment for that function to include this information. I think it was worthwhile. Here is the new comment describing the function. If you think any change needs to be made, let me know, I'll gladly make them. 2. If PST is not NULL, this procedure reads the program to determine the list of files included by the unit represented by PST, and builds all the associated partial symbol tables. In this case, the value of COMP_DIR is ignored, and can thus be NULL (the COMP_DIR is not used to compute the full name of the symtab, and therefore omitting it when building the partial symtab does not introduce the potential for inconsistency - a partial symtab and its associated symbtab having a different fullname -). */ > If that's correct, then this patch is ready to commit. Thanks very > much! Thanks a lot! This patch is now in. -- Joel ^ permalink raw reply [flat|nested] 22+ messages in thread
* [RFC/dwarf-2] Add support for included files
@ 2004-01-02 7:25 Joel Brobecker
2004-01-02 14:18 ` Daniel Jacobowitz
` (2 more replies)
0 siblings, 3 replies; 22+ messages in thread
From: Joel Brobecker @ 2004-01-02 7:25 UTC (permalink / raw)
To: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 4290 bytes --]
Hello,
Ada95 defines the notion of "separates", which allows a developper
to write the implementation of either a function or a package in
a separate file. Here is a small example:
foo.ads:
package Foo is
procedure Proc;
end Foo;
foo.adb:
Package body Foo is
procedure Proc is separate;
^^^^^^^^^^^
End Foo;
And then the body for procedure "Proc", as a separate in foo-proc.adb:
separate (Foo)
^^^^^^^^^^^^^^
procedure Proc is
begin
null;
end Proc;
This capability can be reproduced in C as well, using #include.
The following example is somewhat ugly with respect to the usual
coding practices, but does reproduce well the capability above:
hello.c:
void
say_hello (void)
{
printf ("Hello world.\n");
}
foo.c:
#include <stdio.h>
#include "hello.c" <--- Reproduces the "is separate"...
int
main (void)
{
say_hello ();
return 0;
}
We have noticed that, if compiled with dwarf-2, GDB is unable to
set a breakpoint inside hello.c using the file:lineno syntax:
% gcc -gdwarf-2 foo.c -o foo
% gdb foo
(gdb) b hello.c:4
No source file named hello.c.
(gdb) list hello.c:4
No source file named hello.c.
However, we can set the breakpoint using the function name:
(gdb) b say_hello
Breakpoint 1 at 0x804833e: file hello.c, line 4.
And then once this breakpoint is set, we can now insert the
breakpoint using the source location:
(gdb) b hello.c:4
Note: breakpoint 1 also set at pc 0x804833e.
Breakpoint 2 at 0x804833e: file hello.c, line 4.
(gdb) list hello.c:4
1 void
2 say_hello (void)
3 {
4 printf ("Hello world.\n");
5 }
In Dwarf-2, the list of included files is linked to the line table.
First, we have "Line Number Program Header" which contains the "file
names" table. And then the program itself which uses the DW_LNS_set_file
opcode to change from file to file. There is also the DW_LNE_defile_file
opcode that can be used to define new files instead of using the "file
names" table.
Our problem here is that we don't process this line table when building
the partial symbols. Hence, we miss the list of included files, and
therefore do not create any partial symtab for them. The problem does
not exist with stabs because the N_SOL and line number information
are embedded inside the rest of the stabs data.
If we want to support the case above, I don't see any other way but to
scan the line table as well. I suggest a fast scan, although I don't
see a simple way to skip the opcodes and their data that are ignored
during our scan without having a good knowledge of the opcode we are
skipping. This makes the function that does the quick scan of the line
table almost as fat as the one that is used to build the GDB line
table, almost a copy except that we ignore most of what we read.
Basically, I just copy/pasted the code from dwarf_decode_lines(),
simplified it for partial symtab processing, and then called it
right after the compilation unit psymtab has been built. Currently,
the function scans the line number program, and records which files
of the "file names" table have really been included in the line program
(that's the "file_included" array).
Here is a patch against GDB 6.0 that I used as a proof of concept.
It does not handle the case where the compiler uses DW_LNE_defile_file
instead of the the "file names" table yet, but that will be taken care
of. Would that be an acceptable approach to suggest for inclusion?
2004-01-02 Joel Brobecker <brobecker@gnat.com>
* dwarf2read.c (read_partial_die): Add new parameter.
(scan_partial_symbols): Update call to read_partial_die.
(dwarf2_add_include_psymtab): New function.
(dwarf2_build_include_psymtabs): New function.
(dwarf2_build_psymtabs_hard): Build the psymtabs for the
included files as well.
(psymtab_to_symtab_1): Build the symtab of all dependencies
as well.
Thank you.
--
Joel
[-- Attachment #2: dwarf2read.c.diff --]
[-- Type: text/plain, Size: 10811 bytes --]
Index: dwarf2read.c
===================================================================
RCS file: /nile.c/cvs/Dev/gdb/gdb-6.0/gdb/dwarf2read.c,v
retrieving revision 1.5
diff -u -p -r1.5 dwarf2read.c
--- dwarf2read.c 19 Nov 2003 18:02:23 -0000 1.5
+++ dwarf2read.c 2 Jan 2004 07:10:08 -0000
@@ -693,7 +693,8 @@ static struct abbrev_info *dwarf2_lookup
static char *read_partial_die (struct partial_die_info *,
bfd *, char *,
- const struct comp_unit_head *);
+ const struct comp_unit_head *,
+ unsigned int *);
static char *read_full_die (struct die_info **, bfd *, char *,
const struct comp_unit_head *);
@@ -1196,6 +1197,195 @@ read_comp_unit_head (struct comp_unit_he
return info_ptr;
}
+static void
+dwarf2_add_include_psymtab (char *name,
+ struct partial_symtab *pst,
+ struct objfile *objfile)
+{
+ struct partial_symtab *subpst = allocate_psymtab (name, objfile);
+
+ /* Copy the private data from the main psymtab. */
+ subpst->section_offsets = pst->section_offsets;
+ PST_PRIVATE (subpst) =
+ (char *) obstack_alloc (&objfile->psymbol_obstack,
+ sizeof (struct dwarf2_pinfo));
+ *PST_PRIVATE (subpst) = *PST_PRIVATE (pst);
+
+ subpst->textlow = 0;
+ subpst->texthigh = 0;
+
+ /* We could save slight bits of space by only making one of these,
+ shared by the entire set of include files. FIXME-someday. */
+ subpst->dependencies = (struct partial_symtab **)
+ obstack_alloc (&objfile->psymbol_obstack,
+ sizeof (struct partial_symtab *));
+ subpst->dependencies[0] = pst;
+ subpst->number_of_dependencies = 1;
+
+ subpst->globals_offset = 0;
+ subpst->n_global_syms = 0;
+ subpst->statics_offset = 0;
+ subpst->n_static_syms = 0;
+
+ subpst->readin = 0;
+ subpst->symtab = 0;
+ subpst->read_symtab = pst->read_symtab;
+}
+
+static void
+dwarf2_build_include_psymtabs (struct comp_unit_head *cu_header,
+ const unsigned int line_offset,
+ struct partial_symtab *pst,
+ struct objfile *objfile)
+{
+ bfd *abfd = objfile->obfd;
+ struct line_header *lh;
+ int file_index;
+ char *line_ptr;
+ char *line_end;
+ unsigned int bytes_read;
+ unsigned char op_code, extended_op, adj_opcode; // All used??? FIXME:JOEL
+ char *file_included;
+
+ lh = dwarf_decode_line_header (line_offset, abfd, cu_header);
+
+ if (lh == NULL)
+ return; /* No includes... */
+
+ line_ptr = lh->statement_program_start;
+ line_end = lh->statement_program_end;
+
+ /* FIXME: brobecker: Add comments. */
+ file_included = alloca (lh->num_file_names * sizeof (char));
+ memset (file_included, 0, lh->num_file_names * sizeof (char));
+
+ /* Read the statement sequences until there's nothing left. */
+ while (line_ptr < line_end)
+ {
+ /* state machine registers */
+ int end_sequence = 0;
+
+ /* Decode the table. */
+ while (!end_sequence)
+ {
+ op_code = read_1_byte (abfd, line_ptr);
+ line_ptr += 1;
+
+ if (op_code >= lh->opcode_base)
+ { /* Special operand. Nothing else to read. */
+ }
+ else switch (op_code)
+ {
+ case DW_LNS_extended_op:
+ line_ptr += 1; /* ignore length */
+ extended_op = read_1_byte (abfd, line_ptr);
+ line_ptr += 1;
+ switch (extended_op)
+ {
+ case DW_LNE_end_sequence:
+ end_sequence = 1;
+ break;
+ case DW_LNE_set_address:
+ read_address (abfd, line_ptr, cu_header, &bytes_read);
+ line_ptr += bytes_read;
+ break;
+ case DW_LNE_define_file:
+ {
+ // char *cur_file;
+ // unsigned int dir_index, mod_time, length;
+
+ // cur_file = read_string (abfd, line_ptr, &bytes_read);
+ // line_ptr += bytes_read;
+ // dir_index =
+ // read_unsigned_leb128 (abfd, line_ptr, &bytes_read);
+ // line_ptr += bytes_read;
+ // mod_time =
+ // read_unsigned_leb128 (abfd, line_ptr, &bytes_read);
+ // line_ptr += bytes_read;
+ // length =
+ // read_unsigned_leb128 (abfd, line_ptr, &bytes_read);
+ // line_ptr += bytes_read;
+ // add_file_name (lh, cur_file, dir_index, mod_time, length);
+ /* Ignore for now, but the compiler is allowed to
+ create the list of files via DW_LNE_define_file
+ entries too. We will likely need to take this
+ into account someday. */
+ read_string (abfd, line_ptr, &bytes_read);
+ line_ptr += bytes_read;
+ read_unsigned_leb128 (abfd, line_ptr, &bytes_read);
+ line_ptr += bytes_read;
+ read_unsigned_leb128 (abfd, line_ptr, &bytes_read);
+ line_ptr += bytes_read;
+ read_unsigned_leb128 (abfd, line_ptr, &bytes_read);
+ line_ptr += bytes_read;
+ }
+ break;
+ default:
+ complaint (&symfile_complaints,
+ "mangled .debug_line section");
+ return;
+ }
+ break;
+ case DW_LNS_copy:
+ break;
+ case DW_LNS_advance_pc:
+ read_unsigned_leb128 (abfd, line_ptr, &bytes_read);
+ line_ptr += bytes_read;
+ break;
+ case DW_LNS_advance_line:
+ read_signed_leb128 (abfd, line_ptr, &bytes_read);
+ line_ptr += bytes_read;
+ break;
+ case DW_LNS_set_file:
+ {
+ /* lh->include_dirs and lh->file_names are 0-based,
+ but the directory and file name numbers in the
+ statement program are 1-based. */
+ unsigned int file;
+
+ file = read_unsigned_leb128 (abfd, line_ptr, &bytes_read);
+ line_ptr += bytes_read;
+ file_included[file - 1] = 1;
+ }
+ break;
+ case DW_LNS_set_column:
+ read_unsigned_leb128 (abfd, line_ptr, &bytes_read);
+ line_ptr += bytes_read;
+ break;
+ case DW_LNS_negate_stmt:
+ break;
+ case DW_LNS_set_basic_block:
+ break;
+ case DW_LNS_const_add_pc:
+ break;
+ case DW_LNS_fixed_advance_pc:
+ line_ptr += 2;
+ break;
+ default:
+ { /* Unknown standard opcode, ignore it. */
+ int i;
+ for (i = 0; i < lh->standard_opcode_lengths[op_code]; i++)
+ {
+ (void) read_unsigned_leb128 (abfd, line_ptr, &bytes_read);
+ line_ptr += bytes_read;
+ }
+ }
+ }
+ }
+ }
+
+ for (file_index = 0; file_index < lh->num_file_names; file_index++)
+ if (file_included[file_index] == 1)
+ {
+ char *include_name = lh->file_names [file_index].name;
+
+ if (strcmp (include_name, pst->filename) != 0)
+ dwarf2_add_include_psymtab (include_name, pst, objfile);
+ }
+
+ free_line_header (lh);
+}
+
/* Build the partial symbol table by doing a quick pass through the
.debug_info and .debug_abbrev sections. */
@@ -1262,6 +1452,8 @@ dwarf2_build_psymtabs_hard (struct objfi
while (info_ptr < dwarf_info_buffer + dwarf_info_size)
{
struct comp_unit_head cu_header;
+ unsigned int line_offset = 0;
+
beg_of_comp_unit = info_ptr;
info_ptr = read_comp_unit_head (&cu_header, info_ptr, abfd);
@@ -1307,7 +1499,7 @@ dwarf2_build_psymtabs_hard (struct objfi
/* Read the compilation unit die */
info_ptr = read_partial_die (&comp_unit_die, abfd, info_ptr,
- &cu_header);
+ &cu_header, &line_offset);
/* Set the language we're debugging */
set_cu_language (comp_unit_die.language);
@@ -1373,6 +1565,10 @@ dwarf2_build_psymtabs_hard (struct objfi
info_ptr = beg_of_comp_unit + cu_header.length
+ cu_header.initial_length_size;
+
+ /* Get the list of files included in the current compilation unit,
+ and build a psymtab for each of them. */
+ dwarf2_build_include_psymtabs (&cu_header, line_offset, pst, objfile);
}
do_cleanups (back_to);
}
@@ -1421,7 +1617,7 @@ scan_partial_symbols (char *info_ptr, st
while (nesting_level)
{
- info_ptr = read_partial_die (&pdi, abfd, info_ptr, cu_header);
+ info_ptr = read_partial_die (&pdi, abfd, info_ptr, cu_header, NULL);
/* Anonymous namespaces have no name but are interesting. */
@@ -1677,6 +1873,32 @@ psymtab_to_symtab_1 (struct partial_symt
struct symtab *symtab;
struct cleanup *back_to;
struct attribute *attr;
+ int i;
+
+ for (i = 0; i < pst->number_of_dependencies; i++)
+ if (!pst->dependencies[i]->readin)
+ {
+ /* Inform about additional files that need to be read in. */
+ if (info_verbose)
+ {
+ fputs_filtered (" ", gdb_stdout);
+ wrap_here ("");
+ fputs_filtered ("and ", gdb_stdout);
+ wrap_here ("");
+ printf_filtered ("%s...", pst->dependencies[i]->filename);
+ wrap_here (""); /* Flush output */
+ gdb_flush (gdb_stdout);
+ }
+ psymtab_to_symtab_1 (pst->dependencies[i]);
+ }
+
+ if (pst->textlow == 0 && pst->texthigh == 0)
+ {
+ /* It's an include file, no symbols to read for it.
+ Everything is in the parent symtab. */
+ pst->readin = 1;
+ return;
+ }
/* Set local variables from the partial symbol table info. */
offset = DWARF_INFO_OFFSET (pst);
@@ -4001,7 +4223,8 @@ dwarf2_lookup_abbrev (unsigned int numbe
static char *
read_partial_die (struct partial_die_info *part_die, bfd *abfd,
- char *info_ptr, const struct comp_unit_head *cu_header)
+ char *info_ptr, const struct comp_unit_head *cu_header,
+ unsigned int *line_offset)
{
unsigned int abbrev_number, bytes_read, i;
struct abbrev_info *abbrev;
@@ -4096,6 +4319,9 @@ read_partial_die (struct partial_die_inf
part_die->sibling =
dwarf_info_buffer + dwarf2_get_ref_die_offset (&attr);
break;
+ case DW_AT_stmt_list:
+ if (line_offset != NULL)
+ *line_offset = DW_UNSND (&attr);
default:
break;
}
@@ -4111,7 +4337,7 @@ read_partial_die (struct partial_die_inf
int dummy;
spec_ptr = dwarf_info_buffer + dwarf2_get_ref_die_offset (&spec_attr);
- read_partial_die (&spec_die, abfd, spec_ptr, cu_header);
+ read_partial_die (&spec_die, abfd, spec_ptr, cu_header, NULL);
if (spec_die.name)
{
part_die->name = spec_die.name;
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [RFC/dwarf-2] Add support for included files 2004-01-02 7:25 Joel Brobecker @ 2004-01-02 14:18 ` Daniel Jacobowitz 2004-01-03 14:42 ` Joel Brobecker 2004-01-02 14:45 ` Eli Zaretskii 2004-01-05 16:18 ` Andrew Cagney 2 siblings, 1 reply; 22+ messages in thread From: Daniel Jacobowitz @ 2004-01-02 14:18 UTC (permalink / raw) To: gdb-patches On Fri, Jan 02, 2004 at 08:25:00AM +0100, Joel Brobecker wrote: > We have noticed that, if compiled with dwarf-2, GDB is unable to > set a breakpoint inside hello.c using the file:lineno syntax: > > % gcc -gdwarf-2 foo.c -o foo > % gdb foo > (gdb) b hello.c:4 > No source file named hello.c. > (gdb) list hello.c:4 > No source file named hello.c. > > However, we can set the breakpoint using the function name: > > (gdb) b say_hello > Breakpoint 1 at 0x804833e: file hello.c, line 4. > > And then once this breakpoint is set, we can now insert the > breakpoint using the source location: > > (gdb) b hello.c:4 > Note: breakpoint 1 also set at pc 0x804833e. > Breakpoint 2 at 0x804833e: file hello.c, line 4. > (gdb) list hello.c:4 > 1 void > 2 say_hello (void) > 3 { > 4 printf ("Hello world.\n"); > 5 } Yes... I've noticed this several times recently. > In Dwarf-2, the list of included files is linked to the line table. > First, we have "Line Number Program Header" which contains the "file > names" table. And then the program itself which uses the DW_LNS_set_file > opcode to change from file to file. There is also the DW_LNE_defile_file > opcode that can be used to define new files instead of using the "file > names" table. You keep saying that. I don't think it means what you think it means :) (Hint: DW_LNE_define_file, not DW_LNE_defile_file.) > Basically, I just copy/pasted the code from dwarf_decode_lines(), > simplified it for partial symtab processing, and then called it > right after the compilation unit psymtab has been built. Currently, > the function scans the line number program, and records which files > of the "file names" table have really been included in the line program > (that's the "file_included" array). > > Here is a patch against GDB 6.0 that I used as a proof of concept. > It does not handle the case where the compiler uses DW_LNE_defile_file > instead of the the "file names" table yet, but that will be taken care > of. Would that be an acceptable approach to suggest for inclusion? Well, the question is whether this works well enough. Things _are_ in these files - DW_AT_decl_file points to them. Do we need to also put things into the proper psymtabs? Otherwise the approach seems reasonable. -- Daniel Jacobowitz MontaVista Software Debian GNU/Linux Developer ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC/dwarf-2] Add support for included files 2004-01-02 14:18 ` Daniel Jacobowitz @ 2004-01-03 14:42 ` Joel Brobecker 2004-01-03 16:34 ` Eli Zaretskii 0 siblings, 1 reply; 22+ messages in thread From: Joel Brobecker @ 2004-01-03 14:42 UTC (permalink / raw) To: gdb-patches On Fri, Jan 02, 2004 at 09:18:02AM -0500, Daniel Jacobowitz wrote: > Well, the question is whether this works well enough. Things _are_ in > these files - DW_AT_decl_file points to them. Do we need to also put > things into the proper psymtabs? At this point, I don't see anything else that we need. > Otherwise the approach seems reasonable. Thanks. I can submit a proper patch soon, but there is Eli's suggestion, though: Eli Zaretskii wrote: > How about if we only do that scan when the file name is not found in > the partial symbols, i.e. just before GDB is about to give up and > report the file as nonexistent? Assuming that the cases you have in > mind are rare, this would mean faster operation in most cases. I am a bit relunctant to go that way, because I think the current approach of using a half-baked psymtabs to hold include files is a bit too adhoc for my taste. Adding an extra step after having scanned the psymtab list to iterate over all objfiles, and re-partially scan the debugging information seems to be going one step further in ``legitimizing'' this adhoc approach. If I were to add this extra step, I would rather introduce a separate data structure for include files, and then we could build the list of include files on-the-fly as you suggest when scanning this list. Trouble is, we may have also made some assumptions in several places of GDB's code that the psymtab list contains all source files. I suspect that the extra step you are suggesting will not affect most of the instances of the code scaning the psymtab list, so only a few of them will in fact need to be updated. But I'd rather fix the build_psymtab phase. It seems a bit of effort for a gain that we haven't measured. I would suggest going the simple way first and re-think it if the performance becomes noticeably worse. If somebody has a large app like mozilla that he can use to do some measurement, I would sure appreciate how much slower it is to startup with the patch I posted. (Eli, I don't know about C++, but you have a good point about lex & yacc code) Let me know what you all think. Thanks, -- Joel ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC/dwarf-2] Add support for included files 2004-01-03 14:42 ` Joel Brobecker @ 2004-01-03 16:34 ` Eli Zaretskii 2004-01-03 17:47 ` Joel Brobecker 0 siblings, 1 reply; 22+ messages in thread From: Eli Zaretskii @ 2004-01-03 16:34 UTC (permalink / raw) To: Joel Brobecker; +Cc: gdb-patches > Date: Sat, 3 Jan 2004 15:42:50 +0100 > From: Joel Brobecker <brobecker@gnat.com> > > Eli Zaretskii wrote: > > How about if we only do that scan when the file name is not found in > > the partial symbols, i.e. just before GDB is about to give up and > > report the file as nonexistent? Assuming that the cases you have in > > mind are rare, this would mean faster operation in most cases. > > I am a bit relunctant to go that way, because I think the current > approach of using a half-baked psymtabs to hold include files is > a bit too adhoc for my taste. Adding an extra step after having scanned > the psymtab list to iterate over all objfiles, and re-partially scan the > debugging information seems to be going one step further in > ``legitimizing'' this adhoc approach. I made that suggestion because it sounded like the addition you made caused some percepted slow-down of the psymtab scan. If that is not true, consider my reservations to be withdrawn. In other words, I would also like to see some measurements, as you say: > It seems a bit of effort for a gain that we haven't measured. > I would suggest going the simple way first and re-think it if > the performance becomes noticeably worse. If somebody has a large > app like mozilla that he can use to do some measurement, I would > sure appreciate how much slower it is to startup with the patch > I posted. Perhaps even a not-so-behemoth example would do fine. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC/dwarf-2] Add support for included files 2004-01-03 16:34 ` Eli Zaretskii @ 2004-01-03 17:47 ` Joel Brobecker 0 siblings, 0 replies; 22+ messages in thread From: Joel Brobecker @ 2004-01-03 17:47 UTC (permalink / raw) To: Eli Zaretskii; +Cc: gdb-patches > I made that suggestion because it sounded like the addition you made > caused some percepted slow-down of the psymtab scan. If that is not > true, consider my reservations to be withdrawn. My original message probably wasn't clear. My concern was not exactly about performance, but rather code duplication. Currently, the task of scanning the symbols in the .debug_info data to build the psymtabs is simplified by following the DW_AT_sibling attribute. This attribute is not guaranteed to be always generated when convenient for the debugger, but it helps when it is there. In the .debug_line data, there are no such links. You really have to read byte after byte to interpret the data, and the number of bytes to read depends on the instruction. If you screw up at some point, you will end up slightly off and will therefore necessarily screw up the rest of the program (or you even fail to interpret it). My prototype patch almost duplicates a piece of code that's not really that complex, but the real problem is that it doesn't know how to recover from an error. I was trying to describe this issue for which I didn't see any simple solution, except doing some surgery in the function we currently have that reads that data and stores it into the full symtab. Hmmm, maybe the best approach to mitigate my concerns would be to actually make dwarf_decode_lines() a bit more general. Right now, its purpose is limited to storing the data it read directly into the appropriate symtabs. Modifying it to allow the us to build both symtabs but also psymtabs would make it possible to share this code. Hmmm, could I even keep it as it is? The only parts that tie this function to symtab building seem to only be from using start_subfile() and record_line() from buildsym.c. So maybe we can reuse this function directly as is: it would build subfiles, and then we could dig out the necessary information from there to build the psymtabs? I even believe we could save these subfiles in the private part of the "main" psymtab so we don't have to rescan these tables again during the psymtab to symtab conversion? Sounds like a good idea, doesn't it? Before I start experimenting, does anybody know buildsym.c enough to tell me if it would be ok to use the functions above outside of the context of building symtabs, and then to save the subfiles chain for later use during the psymtab to symtab conversion? Thanks, -- Joel ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC/dwarf-2] Add support for included files 2004-01-02 7:25 Joel Brobecker 2004-01-02 14:18 ` Daniel Jacobowitz @ 2004-01-02 14:45 ` Eli Zaretskii 2004-01-05 16:18 ` Andrew Cagney 2 siblings, 0 replies; 22+ messages in thread From: Eli Zaretskii @ 2004-01-02 14:45 UTC (permalink / raw) To: Joel Brobecker; +Cc: gdb-patches > Date: Fri, 2 Jan 2004 08:25:00 +0100 > From: Joel Brobecker <brobecker@gnat.com> > > This capability can be reproduced in C as well, using #include. Doesn't this happen with C++ programs (which have code fragments on header files) as well? What about Yacc- and Lex-processed code? > If we want to support the case above, I don't see any other way but to > scan the line table as well. How about if we only do that scan when the file name is not found in the partial symbols, i.e. just before GDB is about to give up and report the file as nonexistent? Assuming that the cases you have in mind are rare, this would mean faster operation in most cases. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC/dwarf-2] Add support for included files 2004-01-02 7:25 Joel Brobecker 2004-01-02 14:18 ` Daniel Jacobowitz 2004-01-02 14:45 ` Eli Zaretskii @ 2004-01-05 16:18 ` Andrew Cagney 2004-01-05 19:17 ` Joel Brobecker 2 siblings, 1 reply; 22+ messages in thread From: Andrew Cagney @ 2004-01-05 16:18 UTC (permalink / raw) To: Joel Brobecker; +Cc: gdb-patches Joel, Want to create a "testsuite/gdb.ada/" directory with this as the first test case? I think gdb.objc would serve as a good model as that code knows how to skip things when there isn't an ada compiler. Andrew ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC/dwarf-2] Add support for included files 2004-01-05 16:18 ` Andrew Cagney @ 2004-01-05 19:17 ` Joel Brobecker 0 siblings, 0 replies; 22+ messages in thread From: Joel Brobecker @ 2004-01-05 19:17 UTC (permalink / raw) To: Andrew Cagney; +Cc: gdb-patches > Want to create a "testsuite/gdb.ada/" directory with this as the first > test case? I think gdb.objc would serve as a good model as that code > knows how to skip things when there isn't an ada compiler. That's a good idea. I will do that too (sending a "patch" first, of course). -- Joel ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2004-05-04 0:18 UTC | newest] Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2004-05-03 22:15 [RFC/dwarf-2] Add support for included files Andrew Pinski 2004-05-04 0:15 ` Joel Brobecker 2004-05-04 0:18 ` Andrew Pinski -- strict thread matches above, loose matches on Subject: below -- 2004-04-07 16:05 Jim Blandy 2004-04-13 5:20 ` Joel Brobecker 2004-04-14 19:10 ` Jim Blandy 2004-04-15 22:13 ` Joel Brobecker 2004-04-16 4:24 ` Jim Blandy 2004-04-16 4:28 ` Joel Brobecker 2004-04-16 23:08 ` Joel Brobecker 2004-04-29 23:32 ` Jim Blandy 2004-05-01 1:14 ` Joel Brobecker 2004-05-01 4:57 ` Jim Blandy 2004-05-03 16:25 ` Joel Brobecker 2004-01-02 7:25 Joel Brobecker 2004-01-02 14:18 ` Daniel Jacobowitz 2004-01-03 14:42 ` Joel Brobecker 2004-01-03 16:34 ` Eli Zaretskii 2004-01-03 17:47 ` Joel Brobecker 2004-01-02 14:45 ` Eli Zaretskii 2004-01-05 16:18 ` Andrew Cagney 2004-01-05 19:17 ` Joel Brobecker
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox