diff mbox

Fix one .debug_macro bug and fix -g3 on non-HAVE_AS_DWARF2_DEBUG_LINE (both .debug_macro and .debug_macinfo)

Message ID 20110726203809.GZ2687@tyan-ft48-01.lab.bos.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek July 26, 2011, 8:38 p.m. UTC
Hi!

I've noticed that -g3 (both old .debug_macinfo and new .debug_macro) doesn't
work correctly when HAVE_AS_DWARF2_DEBUG_LINE isn't defined, because
the .debug_line section is emitted before .debug_mac{ro,info} and thus
if any DW_MACINFO_start_file/DW_MACRO_GNU_start_file ops need a file
that wasn't seen so far, it will reference a .debug_line filename table
entry that isn't present.
Fixed by the second and third hunk, by just emitting .debug_line after
.debug_macro/.debug_macinfo.

With that I've discovered that lookup_filename assumes that the string
it is called in is kept around, as it stores just the pointer and not a copy
of that string.  It seems all other places that call lookup_filename already
call it with somehow persistent string and before my .debug_macro patch so
did output_macinfo, because it never freed the malloced strings.
The reason I've added the freeing was that I sometimes reuse the pointers
for something else (the transparent_include group names) and valgrind etc.
would be unhappy about unreachable malloced blocks not being freed, albeit
so close to the end of the compilation.
Instead of copying the string always the following patch just copies it
if it stored that pointer (first hunk).

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2011-07-26  Jakub Jelinek  <jakub@redhat.com>

	* dwarf2out.c (output_macinfo_op): Ensure fd->filename points
	to GC allocated copy of the string.
	(dwarf2out_finish): Emit .debug_macinfo or .debug_macro sections
	before .debug_line, not after it.


	Jakub

Comments

Jakub Jelinek July 26, 2011, 8:50 p.m. UTC | #1
On Tue, Jul 26, 2011 at 10:38:09PM +0200, Jakub Jelinek wrote:
> With that I've discovered that lookup_filename assumes that the string
> it is called in is kept around, as it stores just the pointer and not a copy
> of that string.  It seems all other places that call lookup_filename already
> call it with somehow persistent string and before my .debug_macro patch so
> did output_macinfo, because it never freed the malloced strings.
> The reason I've added the freeing was that I sometimes reuse the pointers
> for something else (the transparent_include group names) and valgrind etc.
> would be unhappy about unreachable malloced blocks not being freed, albeit
> so close to the end of the compilation.
> Instead of copying the string always the following patch just copies it
> if it stored that pointer (first hunk).

Forgot to add, what this newly introduced bug caused was that when using
.file/.loc directives if the same file was included more than once the
second and following copy often wouldn't share the same .file number
with the older one and thus .debug_line grew unnecessarily.  And, it could
rarely happen that it would use a wrong file name table index too, if the
freed memory contained something that would match some filename.  And of
course could crash as any reads from freed memory.

	Jakub
Richard Henderson July 26, 2011, 8:56 p.m. UTC | #2
On 07/26/2011 01:38 PM, Jakub Jelinek wrote:
> 	* dwarf2out.c (output_macinfo_op): Ensure fd->filename points
> 	to GC allocated copy of the string.
> 	(dwarf2out_finish): Emit .debug_macinfo or .debug_macro sections
> 	before .debug_line, not after it.

Ok.


r~
diff mbox

Patch

--- gcc/dwarf2out.c.jj	2011-07-25 11:28:32.000000000 +0200
+++ gcc/dwarf2out.c	2011-07-26 16:19:56.000000000 +0200
@@ -20552,11 +20552,15 @@  output_macinfo_op (macinfo_entry *ref)
   size_t len;
   struct indirect_string_node *node;
   char label[MAX_ARTIFICIAL_LABEL_BYTES];
+  struct dwarf_file_data *fd;
 
   switch (ref->code)
     {
     case DW_MACINFO_start_file:
-      file_num = maybe_emit_file (lookup_filename (ref->info));
+      fd = lookup_filename (ref->info);
+      if (fd->filename == ref->info)
+	fd->filename = ggc_strdup (fd->filename);
+      file_num = maybe_emit_file (fd);
       dw2_asm_output_data (1, DW_MACINFO_start_file, "Start new file");
       dw2_asm_output_data_uleb128 (ref->lineno,
 				   "Included from line number %lu", 
@@ -22637,6 +22641,16 @@  dwarf2out_finish (const char *filename)
       output_ranges ();
     }
 
+  /* Have to end the macro section.  */
+  if (debug_info_level >= DINFO_LEVEL_VERBOSE)
+    {
+      switch_to_section (debug_macinfo_section);
+      ASM_OUTPUT_LABEL (asm_out_file, macinfo_section_label);
+      if (!VEC_empty (macinfo_entry, macinfo_table))
+	output_macinfo ();
+      dw2_asm_output_data (1, 0, "End compilation unit");
+    }
+
   /* Output the source line correspondence table.  We must do this
      even if there is no line information.  Otherwise, on an empty
      translation unit, we will generate a present, but empty,
@@ -22648,16 +22662,6 @@  dwarf2out_finish (const char *filename)
   if (! DWARF2_ASM_LINE_DEBUG_INFO)
     output_line_info ();
 
-  /* Have to end the macro section.  */
-  if (debug_info_level >= DINFO_LEVEL_VERBOSE)
-    {
-      switch_to_section (debug_macinfo_section);
-      ASM_OUTPUT_LABEL (asm_out_file, macinfo_section_label);
-      if (!VEC_empty (macinfo_entry, macinfo_table))
-        output_macinfo ();
-      dw2_asm_output_data (1, 0, "End compilation unit");
-    }
-
   /* If we emitted any DW_FORM_strp form attribute, output the string
      table too.  */
   if (debug_str_hash)