On 10/31/2016 02:38 PM, Jakub Jelinek wrote:
On Mon, Oct 31, 2016 at 11:25:26AM -0400, Jason Merrill wrote:
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?
Maybe. Guess it needs to be decided in binutils and then we adjust
gcc if any changes are needed.
+ 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.
I'll add a comment. The reason is that DW_FORM_data1, if it can be used,
is always more compact than (or as compact as) DW_FORM_udata. While the
choice in between DW_FORM_data2 and DW_FORM_udata if we have 256 to 65535
directories needs to be decided by computing both the size it would take
for all the uleb128s and compare that to the size it takes to encode all
as data2.
+ 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.
Had a brief look at this, but it seems hard without big changes in
dwarf2asm.[ch] or lots of duplication. The problem is that
dw2_asm_output_data* are varargs functions, so if we add some wrapper
that picks what to call based on a enum dwarf_form argument, we'd need to
convert all of them to take va_list instead, and add various further
wrappers. Is this ok to leave this part out?
+ 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.
So like
static void
output_line_string (enum dwarf_form form, const char *str,
const char *entry_kind, unsigned int idx)
in the patch below?
+ 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.
Like DWARF5_USE_DEBUG_LINE_STR macro in the patch below?
For -gsplit-dwarf, the problem is I'm really not familiar with what tools
are used post assembly for this (and likely the tools need adjustment
for DWARF5 anyway), so what is in the patches is just partial support what
appeared to be easily done by myself. For the rest I've been hoping Cary
or somebody else could finish it up.
OK.
Jason
