This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
> 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:
separate2.diff
Description: Text document
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |