This is the mail archive of the binutils@sourceware.org mailing list for the binutils 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] |
On Mon, Oct 26, 2009 at 3:11 PM, Jim Wilson <wilson@codesourcery.com> wrote: > On 08/22/2009 04:53 PM, H.J. Lu wrote: >> >> This patch hadles ".file" directives properly with a new testcase from gcc >> 3.4. >> OK to install? > > Now that I have time to properly review this... > > The stuff for excluding group sections from simple section name string > searches looks mostly OK. > > I see that bfd_make_section_old_way contains the entire contents of the > function bfd_get_section_by_name. ?This was OK when the latter was a 5-line > function, but now that it is a 20-line function, maybe it is better to have > bfd_make_section_old_way call bfd_get_section_by_name instead of duplicating > all 20 lines? ?Except that would require some interface changes, because > bfd_get_section_by_name can return NULL for two different things, one which > bfd_make_section_old_way ignores, and one which needs special handling. ?So > it is a little harder, but it still seems potentially worthwhile. ?Maybe > part of this can be split out to its own function or macro? > > I see that you only fixed those two functions, but you left similar > functions like bfd_make_section and bfd_make_section_with_flags unfixed. > ?Maybe because these aren't called from gas/dwarf2dbg.c? ?Shouldn't we be > fixing all of the functions? ?If we do need to fix more functions, then that > means more copies of the same 20-lines of code, which makes it more > important to avoid duplication of this code. > > I think the stuff for deciding whether to emit a debug_line section has more > problems. > > There are some minor issues. ?For instance in as.c you added a comment >> >> /* If assembler shiuld debug info. ?*/ > > which needs to be fixed. ?Need to change "shiuld" to "should generate". > > You changed the behaviour of the dwarf2_finish function, but you didn't > update the comment before the function start which is now wrong with your > patch. > > You added two variables gen_debug and dwarf2_directive_used, and a test for > them >> >> + ?if (!dwarf2_directive_used && gen_debug == DEBUG_UNSPECIFIED) > > But this test seems to be equivalent to "if (!all_segs)" so there is no need > for these two new variables. > > The bigger problem here is that with your patch dwarf2_finish now does > ?if (!all_segs) return; > ?... > ?if (!all_segs && emit_other_sections) return; > which doesn't make a lot of sense. ?The current code handles the case where > the compiler emits a debug_info section, but does not emit any .file or .loc > directives. ?.loc will only be emitted if there is code. It will not be > emitted for an input file that contains only variables. GCC will always emit > .file if the assembler supports it, but it is possible that it might be > missing if someone configured gcc wrong. ?It isn't obvious whether we need > to handle this case, it is probably safer if we do. > > This is why my suggested solution takes a slightly different tack here. > ?Instead of checking for -g or .file/.loc, I check for a non-empty > debug_line section. ?If there is one, then it should be safe to assume that > the compiler emitted a correct debug_line section, and the assembler should > not emit one. ?My suggested patch can be found here > ?http://sourceware.org/ml/binutils/2009-08/msg00358.html > Here is the updated patch. OK to install? Thanks. -- H.J. --- binutils/testsuite/ 2009-10-27 H.J. Lu <hongjiu.lu@intel.com> PR gas/10531 * binutils-all/objdump.W: Remove bogus line debug info. gas/ 2009-10-27 Jim Wilson <wilson@codesourcery.com> PR gas/10531 * dwarf2dbg.c (dwarf2_finish): Don't generate .debug_line section if it isn't empty. gas/testsuite/ 2009-10-27 H.J. Lu <hongjiu.lu@intel.com> PR gas/10531 * gas/elf/dwarf2-1.d: New. * gas/elf/dwarf2-1.s: Likewise. * gas/elf/dwarf2-2.d: Likewise. * gas/elf/dwarf2-2.s: Likewise. * gas/elf/dwarf2-3.d: Likewise. * gas/elf/dwarf2-3.s: Likewise. * gas/i386/debug1.d: Likewise. * gas/i386/debug1.s: Likewise.
Attachment:
binutils-group-10.patch
Description: Text document
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |