On 10/19/2016 07:30 PM, Jakub Jelinek wrote:
This patch adds support for DWARF5 .debug_line{,_str} section format,
though only if !DWARF2_ASM_LINE_DEBUG_INFO, because otherwise
.debug_line is emitted by the assembler.  For that we'll need some
coordination with gas, dunno if we want a new as directive for that,
or as command line switch, or perhaps key DWARF5 .debug_line on the
presence of .debug_line_str section (though the last one probably isn't a
good idea, because -gsplit-dwarf doesn't use .debug_line_str).

Could gas pay attention to the DWARF version in a unit header?

+      if (ndirs + idx_offset <= 256)
+       idx_form = DW_FORM_data1;
+      else if (ndirs + idx_offset <= 65536)
+       {
+         unsigned HOST_WIDE_INT sum = 1;
+         for (i = 0; i < numfiles; i++)
+           {
+             int file_idx = backmap[i];
+             int dir_idx = dirs[files[file_idx].dir_idx].dir_idx;
+             sum += size_of_uleb128 (dir_idx);
+           }
+         if (sum >= HOST_WIDE_INT_UC (2) * (numfiles + 1))
+           idx_form = DW_FORM_data2;
+       }

Why can we choose DW_FORM_data1 based only on ndirs+idx_offset, but we need to look at the files table to choose DW_FORM_data2? This at least needs a comment.

+       dw2_asm_output_data (idx_form == DW_FORM_data1 ? 1 : 2,
+                            0, NULL);

It seems that we could use an output_data function that takes a DW_FORM as one of its arguments and does the appropriate thing.

+      if (dwarf_version >= 5 && str_form == DW_FORM_line_strp)
+       {
+         node = find_AT_string_in_table (filename0, debug_line_str_hash);
+         set_indirect_string (node);
+         node->form = DW_FORM_line_strp;
+         dw2_asm_output_offset (DWARF_OFFSET_SIZE, node->label,
+                                debug_line_str_section,
+                                "File Entry: %#x: \"%s\"", 0, node->str);
+       }
+      else
+       dw2_asm_output_nstring (filename0, -1, "File Entry: %#x", 0);

Please factor this out into a separate function.

+  if (!DWARF2_ASM_LINE_DEBUG_INFO
+      && dwarf_version >= 5
+      && !(DWARF2_INDIRECT_STRING_SUPPORT_MISSING_ON_TARGET
+          || (DEBUG_STR_SECTION_FLAGS & SECTION_MERGE) == 0
+             /* FIXME: For now.  */
+          || dwarf_split_debug_info))

This should also be a separate function or macro, since it's used more than once. And please elaborate the FIXME comment.

Jason

Reply via email to