PATCH: PR gas/10531: Strange assembler warning message on section group

[Jim Wilson]

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

Jim