From 8d7b78a97daea5da74211a633f9e01bc9715d649 Mon Sep 17 00:00:00 2001 From: Bernd Edlinger Date: Sun, 9 Feb 2020 21:13:17 +0100 Subject: [PATCH] Fix range end handling of inlined subroutines Since the is_stmt is now handled, it becomes possible to locate dubious is_stmt line entries at the end of an inlined function, even if the called inline function is in the same subfile. When there is a sequence of line entries at the same address where an inline range ends, and the last item has is_stmt = 0, we force all previous items to have is_stmt = 0 as well. If the last line at that address has is_stmt = 1, there is no need to change anything, since a step over will always stop at that last line from the same address, which is fine, since it is outside the subroutine. With this change we lose the ability to set breakpoints on some lines using file:line syntax, but the data is not completely lost, as the line table is still holding those lines, just with the is_stmt flag reset. This is necessary as breakpoints on these lines are problematic, as the call stack is often wrong. From the block info they appear to be located in the caller, but they are actually meant to be part of the subroutine, therefore usually one frame is missing from the callstack when the execution stops there. This is about the best we can do at the moment, unless location view information are added to the block ranges debug info structure, and location views are implemented in gdb in general. This restores the previous behavior of record_line that deletes lines at the end of a function. gdb: 2020-04-05 Bernd Edlinger * buildsym.c (buildsym_compunit::record_line): Delete previous lines at the same PC as the end sequence marker, but don't do that for a fake end sequence marker. (buildsym_compunit::record_inline_range_end, patch_inline_end_pos): New helper functions. (buildsym_compunit::end_symtab_with_blockvector): Patch line table. (buildsym_compunit::~buildsym_compunit): Cleanup m_inline_end_vector. * buildsym.h (buildsym_compunit::record_inline_range_end): Declare. (buildsym_compunit::m_inline_end_vector, buildsym_compunit::m_inline_end_vector_length, buildsym_compunit::m_inline_end_vector_nitems): New data items. * dwarf2/read.c (dwarf2_rnglists_process, dwarf2_ranges_process): Don't ignore empty ranges here. (dwarf2_ranges_read): Ignore empty ranges here. (dwarf2_record_block_ranges): Pass end of range PC to record_inline_range_end for inline functions. (dwarf_finish_line): Add new parameter end_sequence. (lnp_state_machine::record_line): Pass end_sequence to dwarf_finish_line. gdb/testsuite: 2020-04-05 Bernd Edlinger * gdb.cp/step-and-next-inline.exp: Adjust test. --- gdb/buildsym.c | 124 ++++++++++++++++++++++---- gdb/buildsym.h | 11 +++ gdb/dwarf2/read.c | 30 ++++--- gdb/testsuite/gdb.cp/step-and-next-inline.exp | 17 ---- 4 files changed, 135 insertions(+), 47 deletions(-) diff --git a/gdb/buildsym.c b/gdb/buildsym.c index fe07103..8c67e58 100644 --- a/gdb/buildsym.c +++ b/gdb/buildsym.c @@ -113,6 +113,8 @@ struct pending_block next1 = next->next; xfree ((void *) next); } + + xfree (m_inline_end_vector); } struct macro_table * @@ -691,30 +693,28 @@ struct blockvector * * sizeof (struct linetable_entry)))); } - /* The end of sequence marker is special. We need to reset the - is_stmt flag on previous lines at the same PC, otherwise these - lines may cause problems since they might be at the same address - as the following function. For instance suppose a function calls - abort there is no reason to emit a ret after that point (no joke). + /* The end of sequence marker is special. We need to delete any + previous lines at the same PC, otherwise these lines may cause + problems since they might be at the same address as the following + function. For instance suppose a function calls abort there is no + reason to emit a ret after that point (no joke). So the label may be at the same address where the following - function begins. A similar problem appears if a label is at the - same address where an inline function ends we cannot reliably tell - if this is considered part of the inline function or the calling - program or even the next inline function, so stack traces may - give surprising results. Expect gdb.cp/step-and-next-inline.exp - to fail if these lines are not modified here. */ - if (line == 0 && subfile->line_vector->nitems > 0) + function begins. There is also a fake end of sequence marker (-1) + that we emit internally when switching between different CUs + In this case, duplicate line table entries shall not be deleted. + But the line table entry looks the same in both cases. */ + if (line == 0) { - e = subfile->line_vector->item + subfile->line_vector->nitems; - do + while (subfile->line_vector->nitems > 0) { - e--; - if (e->pc != pc || e->line == 0) + e = subfile->line_vector->item + subfile->line_vector->nitems - 1; + if (e->pc != pc) break; - e->is_stmt = 0; + subfile->line_vector->nitems--; } - while (e > subfile->line_vector->item); } + else if (line == -1) + line = 0; e = subfile->line_vector->item + subfile->line_vector->nitems++; e->line = line; @@ -723,6 +723,90 @@ struct blockvector * } +/* Record a PC where a inlined subroutine ends. */ + +void +buildsym_compunit::record_inline_range_end (CORE_ADDR end) +{ + /* The performance of this function is very important, + it shall be O(n*log(n)) therefore we do not use std::vector + here since some compilers, e.g. visual studio, do not + guarantee that for vector::push_back. */ + if (m_inline_end_vector == nullptr) + { + m_inline_end_vector_length = INITIAL_LINE_VECTOR_LENGTH; + m_inline_end_vector = (CORE_ADDR *) + xmalloc (sizeof (CORE_ADDR) * m_inline_end_vector_length); + m_inline_end_vector_nitems = 0; + } + else if (m_inline_end_vector_nitems == m_inline_end_vector_length) + { + m_inline_end_vector_length *= 2; + m_inline_end_vector = (CORE_ADDR *) + xrealloc ((char *) m_inline_end_vector, + sizeof (CORE_ADDR) * m_inline_end_vector_length); + } + + m_inline_end_vector[m_inline_end_vector_nitems++] = end; +} + + +/* Patch the is_stmt bits at the given inline end address. + The line table has to be already sorted. */ + +static void +patch_inline_end_pos (struct linetable *table, CORE_ADDR end) +{ + int a = 2, b = table->nitems - 1; + struct linetable_entry *items = table->item; + + /* We need at least two items with pc = end in the table. + The lowest usable items are at pos 0 and 1, the highest + usable items are at pos b - 2 and b - 1. */ + if (a > b || end < items[1].pc || end > items[b - 2].pc) + return; + + /* Look for the first item with pc > end in the range [a,b]. + The previous element has pc = end or there is no match. + We set a = 2, since we need at least two consecutive elements + with pc = end to do anything useful. + We set b = nitems - 1, since we are not interested in the last + element which should be an end of sequence marker with line = 0 + and is_stmt = 1. */ + while (a < b) + { + int c = (a + b) / 2; + + if (end < items[c].pc) + b = c; + else + a = c + 1; + } + + a--; + if (items[a].pc != end || items[a].is_stmt) + return; + + /* When there is a sequence of line entries at the same address + where an inline range ends, and the last item has is_stmt = 0, + we force all previous items to have is_stmt = 0 as well. + Setting breakpoints at those addresses is currently not + supported, since it is unclear if the previous addresses are + part of the subroutine or the calling program. */ + do + { + /* We stop at the first line entry with a different address, + or when we see an end of sequence marker. */ + a--; + if (items[a].pc != end || items[a].line == 0) + break; + + items[a].is_stmt = 0; + } + while (a > 0); +} + + /* Subroutine of end_symtab to simplify it. Look for a subfile that matches the main source file's basename. If there is only one, and if the main source file doesn't have any symbol or line number @@ -956,6 +1040,10 @@ struct compunit_symtab * subfile->line_vector->item + subfile->line_vector->nitems, lte_is_less_than); + + for (int i = 0; i < m_inline_end_vector_nitems; i++) + patch_inline_end_pos (subfile->line_vector, + m_inline_end_vector[i]); } /* Allocate a symbol table if necessary. */ diff --git a/gdb/buildsym.h b/gdb/buildsym.h index c768a4c..2845789 100644 --- a/gdb/buildsym.h +++ b/gdb/buildsym.h @@ -190,6 +190,8 @@ struct buildsym_compunit void record_line (struct subfile *subfile, int line, CORE_ADDR pc, bool is_stmt); + void record_inline_range_end (CORE_ADDR end); + struct compunit_symtab *get_compunit_symtab () { return m_compunit_symtab; @@ -397,6 +399,15 @@ struct buildsym_compunit /* Pending symbols that are local to the lexical context. */ struct pending *m_local_symbols = nullptr; + + /* Pending inline end range addresses. */ + CORE_ADDR * m_inline_end_vector = nullptr; + + /* Number of allocated inline end range addresses. */ + int m_inline_end_vector_length = 0; + + /* Number of pending inline end range addresses. */ + int m_inline_end_vector_nitems = 0; }; diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c index bcc3116..aa32726 100644 --- a/gdb/dwarf2/read.c +++ b/gdb/dwarf2/read.c @@ -13527,10 +13527,6 @@ class process_die_scope return false; } - /* Empty range entries have no effect. */ - if (range_beginning == range_end) - continue; - range_beginning += *base; range_end += *base; @@ -13638,10 +13634,6 @@ class process_die_scope return 0; } - /* Empty range entries have no effect. */ - if (range_beginning == range_end) - continue; - range_beginning += *base; range_end += *base; @@ -13681,6 +13673,10 @@ class process_die_scope retval = dwarf2_ranges_process (offset, cu, [&] (CORE_ADDR range_beginning, CORE_ADDR range_end) { + /* Empty range entries have no effect. */ + if (range_beginning == range_end) + return; + if (ranges_pst != NULL) { CORE_ADDR lowpc; @@ -13918,6 +13914,7 @@ class process_die_scope struct gdbarch *gdbarch = get_objfile_arch (objfile); struct attribute *attr; struct attribute *attr_high; + bool inlined_subroutine = (die->tag == DW_TAG_inlined_subroutine); attr_high = dwarf2_attr (die, DW_AT_high_pc, cu); if (attr_high) @@ -13933,7 +13930,10 @@ class process_die_scope low = gdbarch_adjust_dwarf2_addr (gdbarch, low + baseaddr); high = gdbarch_adjust_dwarf2_addr (gdbarch, high + baseaddr); - cu->get_builder ()->record_block_range (block, low, high - 1); + if (inlined_subroutine) + cu->get_builder ()->record_inline_range_end (high); + if (low < high) + cu->get_builder ()->record_block_range (block, low, high - 1); } } @@ -13958,6 +13958,10 @@ class process_die_scope end += baseaddr; start = gdbarch_adjust_dwarf2_addr (gdbarch, start); end = gdbarch_adjust_dwarf2_addr (gdbarch, end); + if (inlined_subroutine) + cu->get_builder ()->record_inline_range_end (end); + if (start == end) + return; cu->get_builder ()->record_block_range (block, start, end - 1); blockvec.emplace_back (start, end); }); @@ -19479,7 +19483,7 @@ class lnp_state_machine static void dwarf_finish_line (struct gdbarch *gdbarch, struct subfile *subfile, - CORE_ADDR address, struct dwarf2_cu *cu) + CORE_ADDR address, struct dwarf2_cu *cu, bool end_sequence) { if (subfile == NULL) return; @@ -19492,7 +19496,8 @@ class lnp_state_machine paddress (gdbarch, address)); } - dwarf_record_line_1 (gdbarch, subfile, 0, address, true, cu); + dwarf_record_line_1 (gdbarch, subfile, end_sequence ? 0 : -1, address, + true, cu); } void @@ -19525,7 +19530,8 @@ class lnp_state_machine || end_sequence) { dwarf_finish_line (m_gdbarch, m_last_subfile, m_address, - m_currently_recording_lines ? m_cu : nullptr); + m_currently_recording_lines ? m_cu : nullptr, + end_sequence); } if (!end_sequence) diff --git a/gdb/testsuite/gdb.cp/step-and-next-inline.exp b/gdb/testsuite/gdb.cp/step-and-next-inline.exp index 3733fa7..e3a5793 100644 --- a/gdb/testsuite/gdb.cp/step-and-next-inline.exp +++ b/gdb/testsuite/gdb.cp/step-and-next-inline.exp @@ -52,37 +52,20 @@ proc do_test { use_header } { gdb_test "step" ".*" "step into get_alias_set" gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \ "not in inline 1" - # It's possible that this first failure (when not using a header - # file) is GCC's fault, though the remaining failures would best - # be fixed by adding location views support (though it could be - # that some easier heuristic could be figured out). Still, it is - # not certain that the first failure wouldn't also be fixed by - # having location view support, so for now it is tagged as such. - if {!$use_header} { setup_kfail "*-*-*" symtab/25507 } gdb_test "next" ".*TREE_TYPE.*" "next step 1" gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \ "not in inline 2" gdb_test "next" ".*TREE_TYPE.*" "next step 2" gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \ "not in inline 3" - if {!$use_header} { setup_kfail "*-*-*" symtab/25507 } gdb_test "next" ".*TREE_TYPE.*" "next step 3" gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \ "not in inline 4" - if {!$use_header} { setup_kfail "*-*-*" symtab/25507 } gdb_test "next" "return 0.*" "next step 4" gdb_test "bt" \ "\\s*\\#0\\s+(main|get_alias_set)\[^\r\]*${srcfile}:.*" \ "not in inline 5" - if {!$use_header} { - # With the debug from GCC 10.x (and earlier) GDB is currently - # unable to successfully complete the following tests when we - # are not using a header file. - kfail symtab/25507 "stepping tests" - return - } - clean_restart ${executable} if ![runto_main] { -- 1.9.1