* [PATCH v3 2/3] Fix range end handling of inlined subroutines @ 2021-09-05 19:15 Bernd Edlinger 2021-09-08 20:53 ` Bruno Larsen 2021-09-10 9:11 ` Andrew Burgess 0 siblings, 2 replies; 5+ messages in thread From: Bernd Edlinger @ 2021-09-05 19:15 UTC (permalink / raw) To: gdb-patches [-- Attachment #1: Type: text/plain, Size: 3857 bytes --] Currently there is a problem when debugging optimized code when the inferior stops at an inline sub-range end PC. It is unclear if that location is from the inline function or from the calling function. Therefore the call stack is often wrong. This patch detects the "weak" line table entries which are likely part of the previous inline block, and if we have such a location, it assumes the location belongs to the previous block. Additionally it may happen that the infrun machinery steps from one inline range to another inline range of the same inline function. That can look like jumping back and forth from the calling program to the inline function, while really the inline function just jumps from a hot to a cold section of the code, i.e. error handling. Additionally it may happen that one inline sub-range is empty or the inline is completely empty. But filtering that information away is not the right solution, since although there is no actual code from the inline, it is still possible that variables from an inline function can be inspected here. Conceptually this patch uses a heuristic to work around a deficiency in the dwarf-4 and dwarf-5 rnglists structure. There should be a location view number for each inline sub-range begin PC and end PC, similar to the DW_AT_GNU_entry_view which is the location view for the inline entry point. gdb: 2021-01-01 Bernd Edlinger <bernd.edlinger@hotmail.de> * block.c (blockvector_for_pc_sect): For weak line table values, use the inline frame at PC - 1 if it is deeper. * dwarf2/read.c (dwarf2_rnglists_process, dwarf2_ranges_process): Don't ignore empty ranges here. (dwarf2_ranges_read): Ignore empty ranges here. (dwarf2_get_pc_bounds): Allow empty inlined subroutines. (partial_die_info::read): Likewise. (dwarf_finish_line): Add new parameter end_sequence. (lnp_state_machine::m_last_file, lnp_state_machine::m_last_address, lnp_state_machine::m_stmt_at_address): Remove data items. (lnp_state_machine::record_line): Do not filter line entries here, except for line == 0. Pass end_sequence to dwarf_finish_line. * infcmd.c (prepare_one_step): Ignore empty inline ranges. * infrun.c (process_event_stop_test): Don't stop in !is_stmt lines when stepping into inlines. Don't stop at the call site again, when stepping within inlined subroutines with multiple ranges. Don't refresh the step info the when executing !is_stmt lines of of a different inlined subroutine. * symtab.c (find_pc_sect_line): Handle is_weak. gdb/testsuite: 2021-01-01 Bernd Edlinger <bernd.edlinger@hotmail.de> * gdb.base/empty-inline.exp: New test. * gdb.base/empty-inline.c: New test. * gdb.cp/empty-inline.exp: New test. * gdb.cp/empty-inline.cc: New test. * gdb.cp/step-and-next-inline.exp: Remove KFAILs, enhance test. * gdb.cp/step-and-next-inline.cc: Work around failure with gcc-9.3.0 from ubuntu 20.04. --- gdb/block.c | 15 ++- gdb/dwarf2/read.c | 84 +++---------- gdb/infcmd.c | 3 +- gdb/infrun.c | 33 ++++- gdb/symtab.c | 18 +-- gdb/testsuite/gdb.base/empty-inline.c | 43 +++++++ gdb/testsuite/gdb.base/empty-inline.exp | 50 ++++++++ gdb/testsuite/gdb.cp/empty-inline.cc | 33 +++++ gdb/testsuite/gdb.cp/empty-inline.exp | 55 ++++++++ gdb/testsuite/gdb.cp/step-and-next-inline.cc | 6 + gdb/testsuite/gdb.cp/step-and-next-inline.exp | 174 ++++++++++---------------- 11 files changed, 329 insertions(+), 185 deletions(-) create mode 100644 gdb/testsuite/gdb.base/empty-inline.c create mode 100644 gdb/testsuite/gdb.base/empty-inline.exp create mode 100644 gdb/testsuite/gdb.cp/empty-inline.cc create mode 100644 gdb/testsuite/gdb.cp/empty-inline.exp [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0002-Fix-range-end-handling-of-inlined-subroutines.patch --] [-- Type: text/x-patch; name="0002-Fix-range-end-handling-of-inlined-subroutines.patch", Size: 30040 bytes --] From 7112251fb65740d7cafa97034ba2b72c0fced721 Mon Sep 17 00:00:00 2001 From: Bernd Edlinger <bernd.edlinger@hotmail.de> Date: Tue, 3 Nov 2020 18:41:43 +0100 Subject: [PATCH v3 2/3] Fix range end handling of inlined subroutines Currently there is a problem when debugging optimized code when the inferior stops at an inline sub-range end PC. It is unclear if that location is from the inline function or from the calling function. Therefore the call stack is often wrong. This patch detects the "weak" line table entries which are likely part of the previous inline block, and if we have such a location, it assumes the location belongs to the previous block. Additionally it may happen that the infrun machinery steps from one inline range to another inline range of the same inline function. That can look like jumping back and forth from the calling program to the inline function, while really the inline function just jumps from a hot to a cold section of the code, i.e. error handling. Additionally it may happen that one inline sub-range is empty or the inline is completely empty. But filtering that information away is not the right solution, since although there is no actual code from the inline, it is still possible that variables from an inline function can be inspected here. Conceptually this patch uses a heuristic to work around a deficiency in the dwarf-4 and dwarf-5 rnglists structure. There should be a location view number for each inline sub-range begin PC and end PC, similar to the DW_AT_GNU_entry_view which is the location view for the inline entry point. gdb: 2021-01-01 Bernd Edlinger <bernd.edlinger@hotmail.de> * block.c (blockvector_for_pc_sect): For weak line table values, use the inline frame at PC - 1 if it is deeper. * dwarf2/read.c (dwarf2_rnglists_process, dwarf2_ranges_process): Don't ignore empty ranges here. (dwarf2_ranges_read): Ignore empty ranges here. (dwarf2_get_pc_bounds): Allow empty inlined subroutines. (partial_die_info::read): Likewise. (dwarf_finish_line): Add new parameter end_sequence. (lnp_state_machine::m_last_file, lnp_state_machine::m_last_address, lnp_state_machine::m_stmt_at_address): Remove data items. (lnp_state_machine::record_line): Do not filter line entries here, except for line == 0. Pass end_sequence to dwarf_finish_line. * infcmd.c (prepare_one_step): Ignore empty inline ranges. * infrun.c (process_event_stop_test): Don't stop in !is_stmt lines when stepping into inlines. Don't stop at the call site again, when stepping within inlined subroutines with multiple ranges. Don't refresh the step info the when executing !is_stmt lines of of a different inlined subroutine. * symtab.c (find_pc_sect_line): Handle is_weak. gdb/testsuite: 2021-01-01 Bernd Edlinger <bernd.edlinger@hotmail.de> * gdb.base/empty-inline.exp: New test. * gdb.base/empty-inline.c: New test. * gdb.cp/empty-inline.exp: New test. * gdb.cp/empty-inline.cc: New test. * gdb.cp/step-and-next-inline.exp: Remove KFAILs, enhance test. * gdb.cp/step-and-next-inline.cc: Work around failure with gcc-9.3.0 from ubuntu 20.04. --- gdb/block.c | 15 ++- gdb/dwarf2/read.c | 84 +++---------- gdb/infcmd.c | 3 +- gdb/infrun.c | 33 ++++- gdb/symtab.c | 18 +-- gdb/testsuite/gdb.base/empty-inline.c | 43 +++++++ gdb/testsuite/gdb.base/empty-inline.exp | 50 ++++++++ gdb/testsuite/gdb.cp/empty-inline.cc | 33 +++++ gdb/testsuite/gdb.cp/empty-inline.exp | 55 ++++++++ gdb/testsuite/gdb.cp/step-and-next-inline.cc | 6 + gdb/testsuite/gdb.cp/step-and-next-inline.exp | 174 ++++++++++---------------- 11 files changed, 329 insertions(+), 185 deletions(-) create mode 100644 gdb/testsuite/gdb.base/empty-inline.c create mode 100644 gdb/testsuite/gdb.base/empty-inline.exp create mode 100644 gdb/testsuite/gdb.cp/empty-inline.cc create mode 100644 gdb/testsuite/gdb.cp/empty-inline.exp diff --git a/gdb/block.c b/gdb/block.c index 4cb9573..2a0ce6f 100644 --- a/gdb/block.c +++ b/gdb/block.c @@ -205,7 +205,20 @@ struct symbol * return NULL; if (pblock) - *pblock = b; + { + struct symtab_and_line sal = find_pc_sect_line (pc, section, 0); + if (sal.line != 0 && sal.pc == pc && sal.is_weak) + { + const struct block *b2 = find_block_in_blockvector (bl, pc - 1); + const struct block *b0 = b; + while (BLOCK_SUPERBLOCK (b0) && !BLOCK_FUNCTION (b0)) + b0 = BLOCK_SUPERBLOCK (b0); + if (contained_in (b2, b0)) + b = b2; + } + *pblock = b; + } + return bl; } diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c index 5385a3a..358266a 100644 --- a/gdb/dwarf2/read.c +++ b/gdb/dwarf2/read.c @@ -13838,10 +13838,6 @@ class process_die_scope return false; } - /* Empty range entries have no effect. */ - if (range_beginning == range_end) - continue; - /* Only DW_RLE_offset_pair needs the base address added. */ if (rlet == DW_RLE_offset_pair) { @@ -13957,10 +13953,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; @@ -14001,6 +13993,10 @@ class process_die_scope retval = dwarf2_ranges_process (offset, cu, tag, [&] (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; @@ -14109,8 +14105,8 @@ class process_die_scope return PC_BOUNDS_NOT_PRESENT; } - /* partial_die_info::read has also the strict LOW < HIGH requirement. */ - if (high <= low) + /* partial_die_info::read has also the same low < high requirement. */ + if (low > high || (low == high && die->tag != DW_TAG_inlined_subroutine)) return PC_BOUNDS_INVALID; /* When using the GNU linker, .gnu.linkonce. sections are used to @@ -19439,8 +19435,9 @@ struct type * sect_offset_str (sect_off), objfile_name (objfile)); } - /* dwarf2_get_pc_bounds has also the strict low < high requirement. */ - else if (lowpc >= highpc) + /* dwarf2_get_pc_bounds has also the same low < high requirement. */ + else if (lowpc > highpc + || (lowpc == highpc && tag != DW_TAG_inlined_subroutine)) { struct objfile *objfile = per_objfile->objfile; struct gdbarch *gdbarch = objfile->arch (); @@ -20891,21 +20888,9 @@ class lnp_state_machine /* Additional bits of state we need to track. */ - /* The last file that we called dwarf2_start_subfile for. - This is only used for TLLs. */ - unsigned int m_last_file = 0; /* The last file a line number was recorded for. */ struct subfile *m_last_subfile = NULL; - /* The address of the last line entry. */ - CORE_ADDR m_last_address; - - /* Set to true when a previous line at the same address (using - m_last_address) had m_is_stmt true. This is reset to false when a - line entry at a new address (m_address different to m_last_address) is - processed. */ - bool m_stmt_at_address = false; - /* When true, record the lines we decode. */ bool m_currently_recording_lines = false; @@ -21057,7 +21042,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; @@ -21070,7 +21055,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 @@ -21097,37 +21083,17 @@ class lnp_state_machine else if (m_op_index == 0 || end_sequence) { fe->included_p = true; - if (m_record_lines_p) - { - /* When we switch files we insert an end maker in the first file, - switch to the second file and add a new line entry. The - problem is that the end marker inserted in the first file will - discard any previous line entries at the same address. If the - line entries in the first file are marked as is-stmt, while - the new line in the second file is non-stmt, then this means - the end marker will discard is-stmt lines so we can have a - non-stmt line. This means that there are less addresses at - which the user can insert a breakpoint. - - To improve this we track the last address in m_last_address, - and whether we have seen an is-stmt at this address. Then - when switching files, if we have seen a stmt at the current - address, and we are switching to create a non-stmt line, then - discard the new line. */ - bool file_changed - = m_last_subfile != m_cu->get_builder ()->get_current_subfile (); - bool ignore_this_line - = ((file_changed && !end_sequence && m_last_address == m_address - && !m_is_stmt && m_stmt_at_address) - || (!end_sequence && m_line == 0)); - - if ((file_changed && !ignore_this_line) || end_sequence) + if (m_record_lines_p && (end_sequence || m_line != 0)) + { + if (m_last_subfile != m_cu->get_builder ()->get_current_subfile () + || 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 || m_is_stmt); } - if (!end_sequence && !ignore_this_line) + if (!end_sequence) { bool is_stmt = producer_is_codewarrior (m_cu) || m_is_stmt; @@ -21146,15 +21112,6 @@ class lnp_state_machine } } } - - /* Track whether we have seen any m_is_stmt true at m_address in case we - have multiple line table entries all at m_address. */ - if (m_last_address != m_address) - { - m_stmt_at_address = false; - m_last_address = m_address; - } - m_stmt_at_address |= m_is_stmt; } lnp_state_machine::lnp_state_machine (struct dwarf2_cu *cu, gdbarch *arch, @@ -21174,9 +21131,6 @@ class lnp_state_machine m_address = gdbarch_adjust_dwarf2_line (arch, 0, 0); m_is_stmt = lh->default_is_stmt; m_discriminator = 0; - - m_last_address = m_address; - m_stmt_at_address = false; } void diff --git a/gdb/infcmd.c b/gdb/infcmd.c index c183b60..e1d9bb1 100644 --- a/gdb/infcmd.c +++ b/gdb/infcmd.c @@ -976,7 +976,8 @@ enum async_reply_reason if (SYMBOL_CLASS (sym) == LOC_BLOCK) { const block *block = SYMBOL_BLOCK_VALUE (sym); - if (BLOCK_END (block) < tp->control.step_range_end) + if (BLOCK_END (block) < tp->control.step_range_end + && BLOCK_END (block) > tp->control.step_range_start) tp->control.step_range_end = BLOCK_END (block); } } diff --git a/gdb/infrun.c b/gdb/infrun.c index 694bbe6..37d44ae 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -7106,6 +7106,31 @@ struct wait_one_event return; } + /* Handle the case when subroutines have multiple ranges. + When we step from one part to the next part of the same subroutine, + all subroutine levels are skipped again which begin here. + Compensate for this by removing all skipped subroutines, + which were already executing from the user's perspective. */ + + if (frame_id_eq (get_stack_frame_id (get_current_frame ()), + ecs->event_thread->control.step_stack_frame_id) + && inline_skipped_frames (ecs->event_thread) + && ecs->event_thread->control.step_frame_id.artificial_depth > 0 + && ecs->event_thread->control.step_frame_id.code_addr_p) + { + const struct block *prev, *curr; + int depth = 0; + prev = block_for_pc (ecs->event_thread->control.step_frame_id.code_addr); + curr = block_for_pc (ecs->event_thread->stop_pc ()); + while (curr && block_inlined_p (curr) && !contained_in (prev, curr)) + { + depth ++; + curr = BLOCK_SUPERBLOCK (curr); + } + while (inline_skipped_frames (ecs->event_thread) > depth) + step_into_inline_frame (ecs->event_thread); + } + /* Look for "calls" to inlined functions, part one. If the inline frame machinery detected some skipped call sites, we have entered a new inline function. */ @@ -7167,6 +7192,8 @@ struct wait_one_event infrun_debug_printf ("stepping through inlined function"); if (ecs->event_thread->control.step_over_calls == STEP_OVER_ALL + || ecs->event_thread->stop_pc () != stop_pc_sal.pc + || !stop_pc_sal.is_stmt || inline_frame_is_marked_for_skip (false, ecs->event_thread)) keep_going (ecs); else @@ -7192,8 +7219,8 @@ struct wait_one_event end_stepping_range (ecs); return; } - else if (frame_id_eq (get_frame_id (get_current_frame ()), - ecs->event_thread->control.step_frame_id)) + else if (frame_id_eq (get_stack_frame_id (get_current_frame ()), + ecs->event_thread->control.step_stack_frame_id)) { /* We are not at the start of a statement, and we have not changed frame. @@ -7237,7 +7264,7 @@ struct wait_one_event ecs->event_thread->control.step_range_end = stop_pc_sal.end; ecs->event_thread->control.may_range_step = 1; if (refresh_step_info) - set_step_info (ecs->event_thread, frame, stop_pc_sal); + set_step_info (ecs->event_thread, get_current_frame (), stop_pc_sal); infrun_debug_printf ("keep going"); keep_going (ecs); diff --git a/gdb/symtab.c b/gdb/symtab.c index fa3f422..5e95d8a 100644 --- a/gdb/symtab.c +++ b/gdb/symtab.c @@ -3269,7 +3269,11 @@ struct symtab_and_line save prev if it represents the end of a function (i.e. line number 0) instead of a real line. */ - if (prev && prev->line && (!best || prev->pc > best->pc)) + if (prev && prev->line && (!best || prev->pc > best->pc + || (prev->pc == best->pc + && (best->pc == pc + ? !best->is_stmt + : best->is_weak)))) { best = prev; best_symtab = iter_s; @@ -3287,7 +3291,7 @@ struct symtab_and_line while (tmp > first && (tmp - 1)->pc == tmp->pc && (tmp - 1)->line != 0 && !tmp->is_stmt) --tmp; - if (tmp->is_stmt) + if (tmp->is_stmt && (tmp->pc == pc || !tmp->is_weak)) best = tmp; } @@ -3310,18 +3314,14 @@ struct symtab_and_line We used to return alt->line - 1 here, but that could be anywhere; if we don't have line number info for this PC, don't make some up. */ - val.pc = pc; - } - else if (best->line == 0) - { - /* If our best fit is in a range of PC's for which no line - number info is available (line number is zero) then we didn't - find any valid line information. */ + if (notcurrent) + pc++; val.pc = pc; } else { val.is_stmt = best->is_stmt; + val.is_weak = best->is_weak; val.symtab = best_symtab; val.line = best->line; val.pc = best->pc; diff --git a/gdb/testsuite/gdb.base/empty-inline.c b/gdb/testsuite/gdb.base/empty-inline.c new file mode 100644 index 0000000..46944f8 --- /dev/null +++ b/gdb/testsuite/gdb.base/empty-inline.c @@ -0,0 +1,43 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2021 Free Software Foundation, Inc. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see <http://www.gnu.org/licenses/>. */ + +static int test0 (void) +{ + asm (""); + return 1; /* line 21 */ +} + +int __attribute__((noinline, noclone)) +#ifdef __CET__ + __attribute__((nocf_check)) +#endif +test1 (int x) +{ + asm (""); + return x+1; /* line 31 */ +} + +int +#ifdef __CET__ + __attribute__((nocf_check)) +#endif +main() +{ + test1 (test0 ()); /* line 40 */ + test1 (test0 ()); /* line 41 */ + return 0; +} diff --git a/gdb/testsuite/gdb.base/empty-inline.exp b/gdb/testsuite/gdb.base/empty-inline.exp new file mode 100644 index 0000000..5f3e04a --- /dev/null +++ b/gdb/testsuite/gdb.base/empty-inline.exp @@ -0,0 +1,50 @@ +# Copyright 2021 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. + +standard_testfile .c + +if [get_compiler_info] { + return -1 +} + +if { [test_compiler_info gcc*] && ![supports_statement_frontiers] } { + return -1 +} + +global srcfile testfile + +set options {debug nowarnings optimize=-O2} +if { [supports_statement_frontiers] } { + lappend options additional_flags=-gstatement-frontiers +} + +if { [prepare_for_testing "failed to prepare" $binfile \ + $srcfile $options] } { + return -1 +} + +if ![runto_main] { + fail "can't run to main" + return -1 +} + +gdb_test "frame 0" "\\s*\\#0\\s+main.*${srcfile}:40.*" "in main" +gdb_test "step" ".*test0.*${srcfile}:21.*" "step into test0" +gdb_test "frame 1" "\\s*\\#1\\s+main.*${srcfile}:40.*" "frame1" +gdb_test "step" ".*test1.*${srcfile}:31.*" "step into test1" +gdb_test "frame 1" "\\s*\\#1.*in main.*${srcfile}:40.*" "frame2" +gdb_test "step" ".*main.*${srcfile}:41.*" "step back to main" +gdb_test "next" ".*return 0;.*" "step over test0+1" +gdb_test "frame 0" "\\s*\\#0\\s+main.*${srcfile}:42.*" "in main again" diff --git a/gdb/testsuite/gdb.cp/empty-inline.cc b/gdb/testsuite/gdb.cp/empty-inline.cc new file mode 100644 index 0000000..d23f45c --- /dev/null +++ b/gdb/testsuite/gdb.cp/empty-inline.cc @@ -0,0 +1,33 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2021 Free Software Foundation, Inc. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see <http://www.gnu.org/licenses/>. */ + +/* PR 25987 */ +struct MyClass; +struct ptr { + MyClass* get() { return t; } /* line 21 */ + MyClass* t; +}; +struct MyClass { void call(); }; +void MyClass::call() { + *(volatile char*)(nullptr) = 1; /* line 26 */ +} +static void intermediate(ptr p) { + p.get()->call(); /* line 29 */ +} +int main() { + intermediate(ptr{new MyClass}); +} diff --git a/gdb/testsuite/gdb.cp/empty-inline.exp b/gdb/testsuite/gdb.cp/empty-inline.exp new file mode 100644 index 0000000..98f4058 --- /dev/null +++ b/gdb/testsuite/gdb.cp/empty-inline.exp @@ -0,0 +1,55 @@ +# Copyright 2021 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. + +# PR 25987 +standard_testfile .cc + +if {[skip_cplus_tests]} { + return -1 +} + +if {![supports_statement_frontiers]} { + return -1 +} + +set options {c++ debug nowarnings optimize=-Og} +lappend options additional_flags=-gstatement-frontiers +if { [prepare_for_testing "failed to prepare" $testfile \ + $srcfile $options] } { + return -1 +} + +if ![runto_main] { + fail "can't run to main" + return +} + +gdb_test "bt" "\\s*\\#0\\s+main.*" "in main" +#break at the empty inline function ptr::get +gdb_test "b get" ".*" "break at get" +gdb_test "c" ".*" "continue to get" +#call frame 1 is at line 29 +gdb_test "bt" [multi_line "\\s*\\#0\\s+ptr::get\[^\r\]*${srcfile}:21" \ + "\\s*\\#1\\s+intermediate\[^\r\]*${srcfile}:29" \ + ".*"] \ + "at get" +#print a local value here +gdb_test "p t" ".*\\\$1 = \\(MyClass \\*\\) 0x.*" "print t" +gdb_test "c" ".*SIGSEGV.*" "continue to SIGSEGV" +#call frame 1 is at line 29 +gdb_test "bt" [multi_line "\\s*\\#0\\s+MyClass::call\[^\r\]*${srcfile}:26" \ + "\\s*\\#1\\s+0x\[^\r\]*${srcfile}:29" \ + ".*"] \ + "at call" diff --git a/gdb/testsuite/gdb.cp/step-and-next-inline.cc b/gdb/testsuite/gdb.cp/step-and-next-inline.cc index 26b29d0..d64d155 100644 --- a/gdb/testsuite/gdb.cp/step-and-next-inline.cc +++ b/gdb/testsuite/gdb.cp/step-and-next-inline.cc @@ -46,6 +46,9 @@ tree_check (tree *t, int i) #endif /* USE_NEXT_INLINE_H */ int __attribute__((noinline, noclone)) +#ifdef __CET__ + __attribute__((nocf_check)) +#endif get_alias_set (tree *t) { if (t != NULL @@ -59,6 +62,9 @@ get_alias_set (tree *t) tree xx; int +#ifdef __CET__ + __attribute__((nocf_check)) +#endif main() { get_alias_set (&xx); diff --git a/gdb/testsuite/gdb.cp/step-and-next-inline.exp b/gdb/testsuite/gdb.cp/step-and-next-inline.exp index 13d801b..5af1e38 100644 --- a/gdb/testsuite/gdb.cp/step-and-next-inline.exp +++ b/gdb/testsuite/gdb.cp/step-and-next-inline.exp @@ -20,7 +20,7 @@ if [get_compiler_info "c++"] { return -1 } -if {[test_compiler_info gcc*] && ![supports_statement_frontiers] } { +if { [test_compiler_info gcc*] && ![supports_statement_frontiers] } { return -1 } @@ -29,13 +29,6 @@ if {[test_compiler_info gcc*] && ![supports_statement_frontiers] } { proc do_test { use_header } { global srcfile testfile - if { $use_header } { - # This test will not pass due to poor debug information - # generated by GCC (at least upto 10.x). See - # https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94474 - return - } - set options {c++ debug nowarnings optimize=-O2} if { [supports_statement_frontiers] } { lappend options additional_flags=-gstatement-frontiers @@ -67,109 +60,21 @@ 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. - set have_kfail [expr [test_compiler_info gcc*] && !$use_header] - - set ok 1 - gdb_test_multiple "next" "next step 1" { - -re -wrap "if \\(t->x != i\\)" { - set ok 0 - send_gdb "next\n" - exp_continue - } - -re -wrap ".*TREE_TYPE.* != 1" { - if { $ok } { - pass $gdb_test_name - } else { - if { $have_kfail } { - setup_kfail "*-*-*" symtab/25507 - } - fail $gdb_test_name - } - } - } + gdb_test "next" ".*TREE_TYPE.*" "next step 1" gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \ "not in inline 2" - - set ok 1 - gdb_test_multiple "next" "next step 2" { - -re -wrap "return x;" { - set ok 0 - send_gdb "next\n" - exp_continue - } - -re -wrap ".*TREE_TYPE.* != 2" { - if { $ok } { - pass $gdb_test_name - } else { - if { $have_kfail } { - setup_kfail "*-*-*" symtab/25507 - } - fail $gdb_test_name - } - } - } + gdb_test "next" ".*TREE_TYPE.*" "next step 2" gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \ "not in inline 3" - - set ok 1 - gdb_test_multiple "next" "next step 3" { - -re -wrap "return x;" { - set ok 0 - send_gdb "next\n" - exp_continue - } - -re -wrap ".*TREE_TYPE.* != 3\\)" { - if { $ok } { - pass $gdb_test_name - } else { - if { $have_kfail } { - setup_kfail "*-*-*" symtab/25507 - } - fail $gdb_test_name - } - } - } + gdb_test "next" ".*TREE_TYPE.*" "next step 3" gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \ "not in inline 4" - - set ok 1 - gdb_test_multiple "next" "next step 4" { - -re -wrap "(if \\(t != NULL|\} // get_alias_set)" { - send_gdb "next\n" - exp_continue - } - -re -wrap "return x;" { - set ok 0 - send_gdb "next\n" - exp_continue - } - -re -wrap "return 0.*" { - if { $ok } { - pass $gdb_test_name - } else { - if { $have_kfail } { - setup_kfail "*-*-*" symtab/25507 - } - fail $gdb_test_name - } - } - } + gdb_test "next" ".*" "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" + if ![test_compiler_info gcc*] { return } @@ -190,22 +95,79 @@ proc do_test { use_header } { gdb_test "step" ".*if \\(t->x != i\\).*" "step 2" gdb_test "bt" "\\s*\\#0\\s+\[^\r\]*tree_check\[^\r\]*${hdrfile}:.*" \ "in inline 1 pass 2" - gdb_test "step" ".*TREE_TYPE.*" "step 3" + gdb_test "step" ".*return x.*" "step 3" + gdb_test "bt" "\\s*\\#0\\s+\[^\r\]*tree_check\[^\r\]*${hdrfile}:.*" \ + "return from inline 1 pass 2" + gdb_test "step" ".*TREE_TYPE.*" "step 4" gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \ "not in inline 2 pass 2" - gdb_test "step" ".*if \\(t->x != i\\).*" "step 4" + gdb_test "step" ".*if \\(t->x != i\\).*" "step 5" gdb_test "bt" "\\s*\\#0\\s+\[^\r\]*tree_check\[^\r\]*${hdrfile}:.*" \ "in inline 2 pass 2" - gdb_test "step" ".*TREE_TYPE.*" "step 5" + gdb_test "step" ".*return x.*" "step 6" + gdb_test "bt" "\\s*\\#0\\s+\[^\r\]*tree_check\[^\r\]*${hdrfile}:.*" \ + "return from inline 2 pass 2" + gdb_test "step" ".*TREE_TYPE.*" "step 7" gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \ "not in inline 3 pass 2" - gdb_test "step" ".*if \\(t->x != i\\).*" "step 6" + gdb_test "step" ".*if \\(t->x != i\\).*" "step 8" gdb_test "bt" "\\s*\\#0\\s+\[^\r\]*tree_check\[^\r\]*${hdrfile}:.*" \ "in inline 3 pass 2" - gdb_test "step" "return 0.*" "step 7" + gdb_test "step" ".*return x.*" "step 9" + gdb_test "bt" "\\s*\\#0\\s+\[^\r\]*tree_check\[^\r\]*${hdrfile}:.*" \ + "return from inline 3 pass 2" + gdb_test "step" "return 0.*" "step 10" gdb_test "bt" \ "\\s*\\#0\\s+(main|get_alias_set)\[^\r\]*${srcfile}:.*" \ "not in inline 4 pass 2" + + clean_restart ${executable} + + if ![runto_main] { + fail "can't run to main pass 3" + return + } + + gdb_test "bt" "\\s*\\#0\\s+main.*" "in main pass 3" + gdb_test "step" ".*" "step into get_alias_set pass 3" + gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \ + "in get_alias_set pass 3" + gdb_test "step" ".*TREE_TYPE.*" "step 1 pass 3" + gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \ + "not in inline 1 pass 3" + gdb_test "step" ".*if \\(t->x != i\\).*" "step 2 pass 3" + gdb_test "bt" "\\s*\\#0\\s+\[^\r\]*tree_check\[^\r\]*${hdrfile}:.*" \ + "in inline 1 pass 3" + gdb_test "p t->x = 2" ".* = 2.*" "change value pass 3" + gdb_test "step" ".*abort.*" "step 4, pass 3" + gdb_test "bt" "\\s*\\#0\\s+\[^\r\]*tree_check\[^\r\]*${hdrfile}:.*" \ + "abort from inline 1 pass 3" + + clean_restart ${executable} + + if ![runto_main] { + fail "can't run to main pass 4" + return + } + + gdb_test "bt" "\\s*\\#0\\s+main.*" "in main pass 4" + gdb_test "skip tree_check" ".*" "skip tree_check pass 4" + gdb_test "step" ".*" "step into get_alias_set pass 4" + gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \ + "in get_alias_set pass 4" + gdb_test "step" ".*TREE_TYPE.*" "step 1 pass 4" + gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \ + "not in inline 1 pass 4" + gdb_test "step" ".*TREE_TYPE.*" "step 2 pass 4" + gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \ + "not in inline 2 pass 4" + gdb_test "step" ".*TREE_TYPE.*" "step 3 pass 4" + gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \ + "not in inline 3 pass 4" + gdb_test "step" "return 0.*" "step 4 pass 4" + gdb_test "bt" \ + "\\s*\\#0\\s+(main|get_alias_set)\[^\r\]*${srcfile}:.*" \ + "not in inline 4 pass 4" } } -- 1.9.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3 2/3] Fix range end handling of inlined subroutines 2021-09-05 19:15 [PATCH v3 2/3] Fix range end handling of inlined subroutines Bernd Edlinger @ 2021-09-08 20:53 ` Bruno Larsen 2021-09-09 10:42 ` Bernd Edlinger 2021-09-10 9:11 ` Andrew Burgess 1 sibling, 1 reply; 5+ messages in thread From: Bruno Larsen @ 2021-09-08 20:53 UTC (permalink / raw) To: Bernd Edlinger, gdb-patches On 9/5/21 4:15 PM, Bernd Edlinger wrote: > Currently there is a problem when debugging > optimized code when the inferior stops at an inline > sub-range end PC. It is unclear if that location > is from the inline function or from the calling > function. Therefore the call stack is often > wrong. > > This patch detects the "weak" line table entries > which are likely part of the previous inline block, > and if we have such a location, it assumes the > location belongs to the previous block. > > Additionally it may happen that the infrun machinery > steps from one inline range to another inline range > of the same inline function. That can look like > jumping back and forth from the calling program > to the inline function, while really the inline > function just jumps from a hot to a cold section > of the code, i.e. error handling. > > Additionally it may happen that one inline sub-range > is empty or the inline is completely empty. But > filtering that information away is not the right > solution, since although there is no actual code > from the inline, it is still possible that variables > from an inline function can be inspected here. > > Conceptually this patch uses a heuristic to work around > a deficiency in the dwarf-4 and dwarf-5 rnglists structure. > There should be a location view number for each inline > sub-range begin PC and end PC, similar to the DW_AT_GNU_entry_view > which is the location view for the inline entry point. > > gdb: > 2021-01-01 Bernd Edlinger <bernd.edlinger@hotmail.de> > > * block.c (blockvector_for_pc_sect): For weak line table values, > use the inline frame at PC - 1 if it is deeper. > * dwarf2/read.c (dwarf2_rnglists_process, > dwarf2_ranges_process): Don't ignore empty ranges here. > (dwarf2_ranges_read): Ignore empty ranges here. > (dwarf2_get_pc_bounds): Allow empty inlined subroutines. > (partial_die_info::read): Likewise. > (dwarf_finish_line): Add new parameter end_sequence. > (lnp_state_machine::m_last_file, lnp_state_machine::m_last_address, > lnp_state_machine::m_stmt_at_address): Remove data items. > (lnp_state_machine::record_line): Do not filter line entries here, > except for line == 0. Pass end_sequence to dwarf_finish_line. > * infcmd.c (prepare_one_step): Ignore empty inline ranges. > * infrun.c (process_event_stop_test): Don't stop in !is_stmt lines > when stepping into inlines. Don't stop at the call site again, when > stepping within inlined subroutines with multiple ranges. > Don't refresh the step info the when executing !is_stmt lines of > of a different inlined subroutine. > * symtab.c (find_pc_sect_line): Handle is_weak. > > gdb/testsuite: > 2021-01-01 Bernd Edlinger <bernd.edlinger@hotmail.de> > > * gdb.base/empty-inline.exp: New test. > * gdb.base/empty-inline.c: New test. > * gdb.cp/empty-inline.exp: New test. > * gdb.cp/empty-inline.cc: New test. > * gdb.cp/step-and-next-inline.exp: Remove KFAILs, enhance test. > * gdb.cp/step-and-next-inline.cc: Work around failure with > gcc-9.3.0 from ubuntu 20.04. > --- > gdb/block.c | 15 ++- > gdb/dwarf2/read.c | 84 +++---------- > gdb/infcmd.c | 3 +- > gdb/infrun.c | 33 ++++- > gdb/symtab.c | 18 +-- > gdb/testsuite/gdb.base/empty-inline.c | 43 +++++++ > gdb/testsuite/gdb.base/empty-inline.exp | 50 ++++++++ > gdb/testsuite/gdb.cp/empty-inline.cc | 33 +++++ > gdb/testsuite/gdb.cp/empty-inline.exp | 55 ++++++++ > gdb/testsuite/gdb.cp/step-and-next-inline.cc | 6 + > gdb/testsuite/gdb.cp/step-and-next-inline.exp | 174 ++++++++++---------------- > 11 files changed, 329 insertions(+), 185 deletions(-) > create mode 100644 gdb/testsuite/gdb.base/empty-inline.c > create mode 100644 gdb/testsuite/gdb.base/empty-inline.exp > create mode 100644 gdb/testsuite/gdb.cp/empty-inline.cc > create mode 100644 gdb/testsuite/gdb.cp/empty-inline.exp > I also have a few questions about this patch. they are as follows: diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c index 5385a3a..358266a 100644 --- a/gdb/dwarf2/read.c +++ b/gdb/dwarf2/read.c @@ -13838,10 +13838,6 @@ class process_die_scope return false; } - /* Empty range entries have no effect. */ - if (range_beginning == range_end) - continue; - /* Only DW_RLE_offset_pair needs the base address added. */ if (rlet == DW_RLE_offset_pair) { @@ -13957,10 +13953,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; @@ -14001,6 +13993,10 @@ class process_die_scope retval = dwarf2_ranges_process (offset, cu, tag, [&] (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; ....... Why have you removed the range check on the other functions(dwarf2_rnglists_process and dwarf2_ranges_process), and added one for dwarf2_ranges_read? I don't see how this change is related to the rest of the patch, or why the ifs can be removed on those parts >still on the same file void @@ -21097,37 +21083,17 @@ class lnp_state_machine else if (m_op_index == 0 || end_sequence) { fe->included_p = true; - if (m_record_lines_p) - { - /* When we switch files we insert an end maker in the first file, - switch to the second file and add a new line entry. The - problem is that the end marker inserted in the first file will - discard any previous line entries at the same address. If the - line entries in the first file are marked as is-stmt, while - the new line in the second file is non-stmt, then this means - the end marker will discard is-stmt lines so we can have a - non-stmt line. This means that there are less addresses at - which the user can insert a breakpoint. - - To improve this we track the last address in m_last_address, - and whether we have seen an is-stmt at this address. Then - when switching files, if we have seen a stmt at the current - address, and we are switching to create a non-stmt line, then - discard the new line. */ - bool file_changed - = m_last_subfile != m_cu->get_builder ()->get_current_subfile (); - bool ignore_this_line - = ((file_changed && !end_sequence && m_last_address == m_address - && !m_is_stmt && m_stmt_at_address) - || (!end_sequence && m_line == 0)); - - if ((file_changed && !ignore_this_line) || end_sequence) + if (m_record_lines_p && (end_sequence || m_line != 0)) + { + if (m_last_subfile != m_cu->get_builder ()->get_current_subfile () + || 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 || m_is_stmt); } - if (!end_sequence && !ignore_this_line) + if (!end_sequence) { bool is_stmt = producer_is_codewarrior (m_cu) || m_is_stmt; @@ -21146,15 +21112,6 @@ class lnp_state_machine } } } - - /* Track whether we have seen any m_is_stmt true at m_address in case we - have multiple line table entries all at m_address. */ - if (m_last_address != m_address) - { - m_stmt_at_address = false; - m_last_address = m_address; - } - m_stmt_at_address |= m_is_stmt; } Why can the check for overriding an "is_stmt" be removed? Is this related to how inlined functions work? I am not very familiar with gdb code yet, so I can definitely be missing something here. diff --git a/gdb/symtab.c b/gdb/symtab.c index fa3f422..5e95d8a 100644 --- a/gdb/symtab.c +++ b/gdb/symtab.c @@ -3269,7 +3269,11 @@ struct symtab_and_line save prev if it represents the end of a function (i.e. line number 0) instead of a real line. */ - if (prev && prev->line && (!best || prev->pc > best->pc)) + if (prev && prev->line && (!best || prev->pc > best->pc + || (prev->pc == best->pc + && (best->pc == pc + ? !best->is_stmt + : best->is_weak)))) { best = prev; best_symtab = iter_s; @@ -3287,7 +3291,7 @@ struct symtab_and_line while (tmp > first && (tmp - 1)->pc == tmp->pc && (tmp - 1)->line != 0 && !tmp->is_stmt) --tmp; - if (tmp->is_stmt) + if (tmp->is_stmt && (tmp->pc == pc || !tmp->is_weak)) best = tmp; } @@ -3310,18 +3314,14 @@ struct symtab_and_line We used to return alt->line - 1 here, but that could be anywhere; if we don't have line number info for this PC, don't make some up. */ - val.pc = pc; - } - else if (best->line == 0) - { - /* If our best fit is in a range of PC's for which no line - number info is available (line number is zero) then we didn't - find any valid line information. */ + if (notcurrent) + pc++; val.pc = pc; } else { val.is_stmt = best->is_stmt; + val.is_weak = best->is_weak; val.symtab = best_symtab; val.line = best->line; val.pc = best->pc; This last hunk is also not obvious to me. Can we assume that if best->line == 0 is true, !best_symtab will be true? ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3 2/3] Fix range end handling of inlined subroutines 2021-09-08 20:53 ` Bruno Larsen @ 2021-09-09 10:42 ` Bernd Edlinger 0 siblings, 0 replies; 5+ messages in thread From: Bernd Edlinger @ 2021-09-09 10:42 UTC (permalink / raw) To: Bruno Larsen, gdb-patches On 9/8/21 10:53 PM, Bruno Larsen wrote: > On 9/5/21 4:15 PM, Bernd Edlinger wrote: >> Currently there is a problem when debugging >> optimized code when the inferior stops at an inline >> sub-range end PC. It is unclear if that location >> is from the inline function or from the calling >> function. Therefore the call stack is often >> wrong. >> >> This patch detects the "weak" line table entries >> which are likely part of the previous inline block, >> and if we have such a location, it assumes the >> location belongs to the previous block. >> >> Additionally it may happen that the infrun machinery >> steps from one inline range to another inline range >> of the same inline function. That can look like >> jumping back and forth from the calling program >> to the inline function, while really the inline >> function just jumps from a hot to a cold section >> of the code, i.e. error handling. >> >> Additionally it may happen that one inline sub-range >> is empty or the inline is completely empty. But >> filtering that information away is not the right >> solution, since although there is no actual code >> from the inline, it is still possible that variables >> from an inline function can be inspected here. >> >> Conceptually this patch uses a heuristic to work around >> a deficiency in the dwarf-4 and dwarf-5 rnglists structure. >> There should be a location view number for each inline >> sub-range begin PC and end PC, similar to the DW_AT_GNU_entry_view >> which is the location view for the inline entry point. >> >> gdb: >> 2021-01-01 Bernd Edlinger <bernd.edlinger@hotmail.de> >> >> * block.c (blockvector_for_pc_sect): For weak line table values, >> use the inline frame at PC - 1 if it is deeper. >> * dwarf2/read.c (dwarf2_rnglists_process, >> dwarf2_ranges_process): Don't ignore empty ranges here. >> (dwarf2_ranges_read): Ignore empty ranges here. >> (dwarf2_get_pc_bounds): Allow empty inlined subroutines. >> (partial_die_info::read): Likewise. >> (dwarf_finish_line): Add new parameter end_sequence. >> (lnp_state_machine::m_last_file, lnp_state_machine::m_last_address, >> lnp_state_machine::m_stmt_at_address): Remove data items. >> (lnp_state_machine::record_line): Do not filter line entries here, >> except for line == 0. Pass end_sequence to dwarf_finish_line. >> * infcmd.c (prepare_one_step): Ignore empty inline ranges. >> * infrun.c (process_event_stop_test): Don't stop in !is_stmt lines >> when stepping into inlines. Don't stop at the call site again, when >> stepping within inlined subroutines with multiple ranges. >> Don't refresh the step info the when executing !is_stmt lines of >> of a different inlined subroutine. >> * symtab.c (find_pc_sect_line): Handle is_weak. >> >> gdb/testsuite: >> 2021-01-01 Bernd Edlinger <bernd.edlinger@hotmail.de> >> >> * gdb.base/empty-inline.exp: New test. >> * gdb.base/empty-inline.c: New test. >> * gdb.cp/empty-inline.exp: New test. >> * gdb.cp/empty-inline.cc: New test. >> * gdb.cp/step-and-next-inline.exp: Remove KFAILs, enhance test. >> * gdb.cp/step-and-next-inline.cc: Work around failure with >> gcc-9.3.0 from ubuntu 20.04. >> --- >> gdb/block.c | 15 ++- >> gdb/dwarf2/read.c | 84 +++---------- >> gdb/infcmd.c | 3 +- >> gdb/infrun.c | 33 ++++- >> gdb/symtab.c | 18 +-- >> gdb/testsuite/gdb.base/empty-inline.c | 43 +++++++ >> gdb/testsuite/gdb.base/empty-inline.exp | 50 ++++++++ >> gdb/testsuite/gdb.cp/empty-inline.cc | 33 +++++ >> gdb/testsuite/gdb.cp/empty-inline.exp | 55 ++++++++ >> gdb/testsuite/gdb.cp/step-and-next-inline.cc | 6 + >> gdb/testsuite/gdb.cp/step-and-next-inline.exp | 174 ++++++++++---------------- >> 11 files changed, 329 insertions(+), 185 deletions(-) >> create mode 100644 gdb/testsuite/gdb.base/empty-inline.c >> create mode 100644 gdb/testsuite/gdb.base/empty-inline.exp >> create mode 100644 gdb/testsuite/gdb.cp/empty-inline.cc >> create mode 100644 gdb/testsuite/gdb.cp/empty-inline.exp >> > > > I also have a few questions about this patch. they are as follows: > > diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c > index 5385a3a..358266a 100644 > --- a/gdb/dwarf2/read.c > +++ b/gdb/dwarf2/read.c > @@ -13838,10 +13838,6 @@ class process_die_scope > return false; > } > > - /* Empty range entries have no effect. */ > - if (range_beginning == range_end) > - continue; > - > /* Only DW_RLE_offset_pair needs the base address added. */ > if (rlet == DW_RLE_offset_pair) > { > @@ -13957,10 +13953,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; > > @@ -14001,6 +13993,10 @@ class process_die_scope > retval = dwarf2_ranges_process (offset, cu, tag, > [&] (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; > ....... > > Why have you removed the range check on the other functions(dwarf2_rnglists_process and dwarf2_ranges_process), and added one for dwarf2_ranges_read? I don't see how this change is related to the rest of the patch, or why the ifs can be removed on those parts > Note: with this hunk reverted I see: FAIL: gdb.cp/step-and-next-inline.exp: no_header: abort from inline 1 pass 3 FAIL: gdb.cp/step-and-next-inline.exp: use_header: abort from inline 1 pass 3 There are two invocations of dwarf2_ranges_process. One here, and another one there: std::vector<blockrange> blockvec; dwarf2_ranges_process (ranges_offset, cu, die->tag, [&] (CORE_ADDR start, CORE_ADDR end) { start += baseaddr; end += baseaddr; start = gdbarch_adjust_dwarf2_addr (gdbarch, start); end = gdbarch_adjust_dwarf2_addr (gdbarch, end); cu->get_builder ()->record_block_range (block, start, end - 1); blockvec.emplace_back (start, end); if (entry_pc == start && blockvec.size () > 1) std::swap (blockvec[0], blockvec[blockvec.size () - 1]); }); I want the empty ranges to be recorded in cu->get_builder ()->record_block_range and also the condition entry_pc == start && blockvec.size () > 1 is possibly satisfied in an empty block range, although I have not been able to find a simple test case for this, but I know for sure it may happen. The overall idea of this patch is to move the decision what to do with empty ranges to a later stage, when we have access to the line table, and know better if it can be ignored or not. >> still on the same file > void > @@ -21097,37 +21083,17 @@ class lnp_state_machine > else if (m_op_index == 0 || end_sequence) > { > fe->included_p = true; > - if (m_record_lines_p) > - { > - /* When we switch files we insert an end maker in the first file, > - switch to the second file and add a new line entry. The > - problem is that the end marker inserted in the first file will > - discard any previous line entries at the same address. If the > - line entries in the first file are marked as is-stmt, while > - the new line in the second file is non-stmt, then this means > - the end marker will discard is-stmt lines so we can have a > - non-stmt line. This means that there are less addresses at > - which the user can insert a breakpoint. > - > - To improve this we track the last address in m_last_address, > - and whether we have seen an is-stmt at this address. Then > - when switching files, if we have seen a stmt at the current > - address, and we are switching to create a non-stmt line, then > - discard the new line. */ > - bool file_changed > - = m_last_subfile != m_cu->get_builder ()->get_current_subfile (); > - bool ignore_this_line > - = ((file_changed && !end_sequence && m_last_address == m_address > - && !m_is_stmt && m_stmt_at_address) > - || (!end_sequence && m_line == 0)); > - > - if ((file_changed && !ignore_this_line) || end_sequence) > + if (m_record_lines_p && (end_sequence || m_line != 0)) > + { > + if (m_last_subfile != m_cu->get_builder ()->get_current_subfile () > + || 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 || m_is_stmt); > } > > - if (!end_sequence && !ignore_this_line) > + if (!end_sequence) > { > bool is_stmt = producer_is_codewarrior (m_cu) || m_is_stmt; > > @@ -21146,15 +21112,6 @@ class lnp_state_machine > } > } > } > - > - /* Track whether we have seen any m_is_stmt true at m_address in case we > - have multiple line table entries all at m_address. */ > - if (m_last_address != m_address) > - { > - m_stmt_at_address = false; > - m_last_address = m_address; > - } > - m_stmt_at_address |= m_is_stmt; > } > > Why can the check for overriding an "is_stmt" be removed? Is this related to how inlined functions work? I am not very familiar with gdb code yet, so I can definitely be missing something here. > Yes, this is related. Previously this used to ignore some line table entries (see "ignore_this_line"), and not to record those line entries at all. But at this time we don't know yet if a subroutine range ends here or not. So the decision what to do with those line table entries is moved to a later point when we have access to the block structure. > diff --git a/gdb/symtab.c b/gdb/symtab.c > index fa3f422..5e95d8a 100644 > --- a/gdb/symtab.c > +++ b/gdb/symtab.c > @@ -3269,7 +3269,11 @@ struct symtab_and_line > save prev if it represents the end of a function (i.e. line number > 0) instead of a real line. */ > > - if (prev && prev->line && (!best || prev->pc > best->pc)) > + if (prev && prev->line && (!best || prev->pc > best->pc > + || (prev->pc == best->pc > + && (best->pc == pc > + ? !best->is_stmt > + : best->is_weak)))) > { > best = prev; > best_symtab = iter_s; > @@ -3287,7 +3291,7 @@ struct symtab_and_line > while (tmp > first && (tmp - 1)->pc == tmp->pc > && (tmp - 1)->line != 0 && !tmp->is_stmt) > --tmp; > - if (tmp->is_stmt) > + if (tmp->is_stmt && (tmp->pc == pc || !tmp->is_weak)) > best = tmp; > } > > @@ -3310,18 +3314,14 @@ struct symtab_and_line > We used to return alt->line - 1 here, but that could be > anywhere; if we don't have line number info for this PC, > don't make some up. */ > - val.pc = pc; > - } > - else if (best->line == 0) > - { > - /* If our best fit is in a range of PC's for which no line > - number info is available (line number is zero) then we didn't > - find any valid line information. */ > + if (notcurrent) > + pc++; > val.pc = pc; > } > else > { > val.is_stmt = best->is_stmt; > + val.is_weak = best->is_weak; > val.symtab = best_symtab; > val.line = best->line; > val.pc = best->pc; > > This last hunk is also not obvious to me. Can we assume that if best->line == 0 is true, !best_symtab will be true? > Yes. For the same reason why we don't have to check that best != NULL. See here: if (prev && prev->line && (!best || prev->pc > best->pc || (prev->pc == best->pc && (best->pc == pc ? !best->is_stmt : best->is_weak)))) { best = prev; best_symtab = iter_s; so best_symtab is only set to something when at the same time best is set to prev, but the condition is if (prev && prev->line && ...). The only other place where bset is assigned is here: if (!best->is_stmt) { struct linetable_entry *tmp = best; while (tmp > first && (tmp - 1)->pc == tmp->pc && (tmp - 1)->line != 0 && !tmp->is_stmt) --tmp; if (tmp->is_stmt && (tmp->pc == pc || !tmp->is_weak)) best = tmp; and again we check for (tmp - 1)->line != 0 before decrementing tmp. So indeed we know that best->line != 0 if best_symtab != NULL. Thanks Bernd. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3 2/3] Fix range end handling of inlined subroutines 2021-09-05 19:15 [PATCH v3 2/3] Fix range end handling of inlined subroutines Bernd Edlinger 2021-09-08 20:53 ` Bruno Larsen @ 2021-09-10 9:11 ` Andrew Burgess 2021-09-12 7:47 ` Bernd Edlinger 1 sibling, 1 reply; 5+ messages in thread From: Andrew Burgess @ 2021-09-10 9:11 UTC (permalink / raw) To: Bernd Edlinger; +Cc: gdb-patches * Bernd Edlinger <bernd.edlinger@hotmail.de> [2021-09-05 21:15:52 +0200]: > Currently there is a problem when debugging > optimized code when the inferior stops at an inline > sub-range end PC. It is unclear if that location > is from the inline function or from the calling > function. Therefore the call stack is often > wrong. > > This patch detects the "weak" line table entries > which are likely part of the previous inline block, > and if we have such a location, it assumes the > location belongs to the previous block. > > Additionally it may happen that the infrun machinery > steps from one inline range to another inline range > of the same inline function. That can look like > jumping back and forth from the calling program > to the inline function, while really the inline > function just jumps from a hot to a cold section > of the code, i.e. error handling. > > Additionally it may happen that one inline sub-range > is empty or the inline is completely empty. But > filtering that information away is not the right > solution, since although there is no actual code > from the inline, it is still possible that variables > from an inline function can be inspected here. > > Conceptually this patch uses a heuristic to work around > a deficiency in the dwarf-4 and dwarf-5 rnglists structure. > There should be a location view number for each inline > sub-range begin PC and end PC, similar to the DW_AT_GNU_entry_view > which is the location view for the inline entry point. I know that we've discussed this before, and that we disagree on whether there is a GCC bug that is contributing to this problem or not, but I find it weird that you have not linked to: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94474 This is the still that patch that aims to address this issue, right? Please don't misunderstand me, I am absolutely not saying that we shouldn't work around GCC bugs in GDB. And as you point out, there might also be deficiencies in the DWARF spec that impact on this issue too. I think that the description of this fix needs to touch on all these issues. Finally, I really think the description needs to include actual examples of DWARF output along with an explanation of how your patch solves the problem. I've started looking at this series of yours several times, and each time I run out of time and give up, mostly because I have to start from first principles trying to figure out what's going on, I'm trying to answer the question _why_ is this the right way to solve this problem, and I'm just not getting that from your commit messages. Thanks, Andrew > > gdb: > 2021-01-01 Bernd Edlinger <bernd.edlinger@hotmail.de> > > * block.c (blockvector_for_pc_sect): For weak line table values, > use the inline frame at PC - 1 if it is deeper. > * dwarf2/read.c (dwarf2_rnglists_process, > dwarf2_ranges_process): Don't ignore empty ranges here. > (dwarf2_ranges_read): Ignore empty ranges here. > (dwarf2_get_pc_bounds): Allow empty inlined subroutines. > (partial_die_info::read): Likewise. > (dwarf_finish_line): Add new parameter end_sequence. > (lnp_state_machine::m_last_file, lnp_state_machine::m_last_address, > lnp_state_machine::m_stmt_at_address): Remove data items. > (lnp_state_machine::record_line): Do not filter line entries here, > except for line == 0. Pass end_sequence to dwarf_finish_line. > * infcmd.c (prepare_one_step): Ignore empty inline ranges. > * infrun.c (process_event_stop_test): Don't stop in !is_stmt lines > when stepping into inlines. Don't stop at the call site again, when > stepping within inlined subroutines with multiple ranges. > Don't refresh the step info the when executing !is_stmt lines of > of a different inlined subroutine. > * symtab.c (find_pc_sect_line): Handle is_weak. > > gdb/testsuite: > 2021-01-01 Bernd Edlinger <bernd.edlinger@hotmail.de> > > * gdb.base/empty-inline.exp: New test. > * gdb.base/empty-inline.c: New test. > * gdb.cp/empty-inline.exp: New test. > * gdb.cp/empty-inline.cc: New test. > * gdb.cp/step-and-next-inline.exp: Remove KFAILs, enhance test. > * gdb.cp/step-and-next-inline.cc: Work around failure with > gcc-9.3.0 from ubuntu 20.04. > --- > gdb/block.c | 15 ++- > gdb/dwarf2/read.c | 84 +++---------- > gdb/infcmd.c | 3 +- > gdb/infrun.c | 33 ++++- > gdb/symtab.c | 18 +-- > gdb/testsuite/gdb.base/empty-inline.c | 43 +++++++ > gdb/testsuite/gdb.base/empty-inline.exp | 50 ++++++++ > gdb/testsuite/gdb.cp/empty-inline.cc | 33 +++++ > gdb/testsuite/gdb.cp/empty-inline.exp | 55 ++++++++ > gdb/testsuite/gdb.cp/step-and-next-inline.cc | 6 + > gdb/testsuite/gdb.cp/step-and-next-inline.exp | 174 ++++++++++---------------- > 11 files changed, 329 insertions(+), 185 deletions(-) > create mode 100644 gdb/testsuite/gdb.base/empty-inline.c > create mode 100644 gdb/testsuite/gdb.base/empty-inline.exp > create mode 100644 gdb/testsuite/gdb.cp/empty-inline.cc > create mode 100644 gdb/testsuite/gdb.cp/empty-inline.exp > > From 7112251fb65740d7cafa97034ba2b72c0fced721 Mon Sep 17 00:00:00 2001 > From: Bernd Edlinger <bernd.edlinger@hotmail.de> > Date: Tue, 3 Nov 2020 18:41:43 +0100 > Subject: [PATCH v3 2/3] Fix range end handling of inlined subroutines > > Currently there is a problem when debugging > optimized code when the inferior stops at an inline > sub-range end PC. It is unclear if that location > is from the inline function or from the calling > function. Therefore the call stack is often > wrong. > > This patch detects the "weak" line table entries > which are likely part of the previous inline block, > and if we have such a location, it assumes the > location belongs to the previous block. > > Additionally it may happen that the infrun machinery > steps from one inline range to another inline range > of the same inline function. That can look like > jumping back and forth from the calling program > to the inline function, while really the inline > function just jumps from a hot to a cold section > of the code, i.e. error handling. > > Additionally it may happen that one inline sub-range > is empty or the inline is completely empty. But > filtering that information away is not the right > solution, since although there is no actual code > from the inline, it is still possible that variables > from an inline function can be inspected here. > > Conceptually this patch uses a heuristic to work around > a deficiency in the dwarf-4 and dwarf-5 rnglists structure. > There should be a location view number for each inline > sub-range begin PC and end PC, similar to the DW_AT_GNU_entry_view > which is the location view for the inline entry point. > > gdb: > 2021-01-01 Bernd Edlinger <bernd.edlinger@hotmail.de> > > * block.c (blockvector_for_pc_sect): For weak line table values, > use the inline frame at PC - 1 if it is deeper. > * dwarf2/read.c (dwarf2_rnglists_process, > dwarf2_ranges_process): Don't ignore empty ranges here. > (dwarf2_ranges_read): Ignore empty ranges here. > (dwarf2_get_pc_bounds): Allow empty inlined subroutines. > (partial_die_info::read): Likewise. > (dwarf_finish_line): Add new parameter end_sequence. > (lnp_state_machine::m_last_file, lnp_state_machine::m_last_address, > lnp_state_machine::m_stmt_at_address): Remove data items. > (lnp_state_machine::record_line): Do not filter line entries here, > except for line == 0. Pass end_sequence to dwarf_finish_line. > * infcmd.c (prepare_one_step): Ignore empty inline ranges. > * infrun.c (process_event_stop_test): Don't stop in !is_stmt lines > when stepping into inlines. Don't stop at the call site again, when > stepping within inlined subroutines with multiple ranges. > Don't refresh the step info the when executing !is_stmt lines of > of a different inlined subroutine. > * symtab.c (find_pc_sect_line): Handle is_weak. > > gdb/testsuite: > 2021-01-01 Bernd Edlinger <bernd.edlinger@hotmail.de> > > * gdb.base/empty-inline.exp: New test. > * gdb.base/empty-inline.c: New test. > * gdb.cp/empty-inline.exp: New test. > * gdb.cp/empty-inline.cc: New test. > * gdb.cp/step-and-next-inline.exp: Remove KFAILs, enhance test. > * gdb.cp/step-and-next-inline.cc: Work around failure with > gcc-9.3.0 from ubuntu 20.04. > --- > gdb/block.c | 15 ++- > gdb/dwarf2/read.c | 84 +++---------- > gdb/infcmd.c | 3 +- > gdb/infrun.c | 33 ++++- > gdb/symtab.c | 18 +-- > gdb/testsuite/gdb.base/empty-inline.c | 43 +++++++ > gdb/testsuite/gdb.base/empty-inline.exp | 50 ++++++++ > gdb/testsuite/gdb.cp/empty-inline.cc | 33 +++++ > gdb/testsuite/gdb.cp/empty-inline.exp | 55 ++++++++ > gdb/testsuite/gdb.cp/step-and-next-inline.cc | 6 + > gdb/testsuite/gdb.cp/step-and-next-inline.exp | 174 ++++++++++---------------- > 11 files changed, 329 insertions(+), 185 deletions(-) > create mode 100644 gdb/testsuite/gdb.base/empty-inline.c > create mode 100644 gdb/testsuite/gdb.base/empty-inline.exp > create mode 100644 gdb/testsuite/gdb.cp/empty-inline.cc > create mode 100644 gdb/testsuite/gdb.cp/empty-inline.exp > > diff --git a/gdb/block.c b/gdb/block.c > index 4cb9573..2a0ce6f 100644 > --- a/gdb/block.c > +++ b/gdb/block.c > @@ -205,7 +205,20 @@ struct symbol * > return NULL; > > if (pblock) > - *pblock = b; > + { > + struct symtab_and_line sal = find_pc_sect_line (pc, section, 0); > + if (sal.line != 0 && sal.pc == pc && sal.is_weak) > + { > + const struct block *b2 = find_block_in_blockvector (bl, pc - 1); > + const struct block *b0 = b; > + while (BLOCK_SUPERBLOCK (b0) && !BLOCK_FUNCTION (b0)) > + b0 = BLOCK_SUPERBLOCK (b0); > + if (contained_in (b2, b0)) > + b = b2; > + } > + *pblock = b; > + } > + > return bl; > } > > diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c > index 5385a3a..358266a 100644 > --- a/gdb/dwarf2/read.c > +++ b/gdb/dwarf2/read.c > @@ -13838,10 +13838,6 @@ class process_die_scope > return false; > } > > - /* Empty range entries have no effect. */ > - if (range_beginning == range_end) > - continue; > - > /* Only DW_RLE_offset_pair needs the base address added. */ > if (rlet == DW_RLE_offset_pair) > { > @@ -13957,10 +13953,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; > > @@ -14001,6 +13993,10 @@ class process_die_scope > retval = dwarf2_ranges_process (offset, cu, tag, > [&] (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; > @@ -14109,8 +14105,8 @@ class process_die_scope > return PC_BOUNDS_NOT_PRESENT; > } > > - /* partial_die_info::read has also the strict LOW < HIGH requirement. */ > - if (high <= low) > + /* partial_die_info::read has also the same low < high requirement. */ > + if (low > high || (low == high && die->tag != DW_TAG_inlined_subroutine)) > return PC_BOUNDS_INVALID; > > /* When using the GNU linker, .gnu.linkonce. sections are used to > @@ -19439,8 +19435,9 @@ struct type * > sect_offset_str (sect_off), > objfile_name (objfile)); > } > - /* dwarf2_get_pc_bounds has also the strict low < high requirement. */ > - else if (lowpc >= highpc) > + /* dwarf2_get_pc_bounds has also the same low < high requirement. */ > + else if (lowpc > highpc > + || (lowpc == highpc && tag != DW_TAG_inlined_subroutine)) > { > struct objfile *objfile = per_objfile->objfile; > struct gdbarch *gdbarch = objfile->arch (); > @@ -20891,21 +20888,9 @@ class lnp_state_machine > > /* Additional bits of state we need to track. */ > > - /* The last file that we called dwarf2_start_subfile for. > - This is only used for TLLs. */ > - unsigned int m_last_file = 0; > /* The last file a line number was recorded for. */ > struct subfile *m_last_subfile = NULL; > > - /* The address of the last line entry. */ > - CORE_ADDR m_last_address; > - > - /* Set to true when a previous line at the same address (using > - m_last_address) had m_is_stmt true. This is reset to false when a > - line entry at a new address (m_address different to m_last_address) is > - processed. */ > - bool m_stmt_at_address = false; > - > /* When true, record the lines we decode. */ > bool m_currently_recording_lines = false; > > @@ -21057,7 +21042,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; > @@ -21070,7 +21055,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 > @@ -21097,37 +21083,17 @@ class lnp_state_machine > else if (m_op_index == 0 || end_sequence) > { > fe->included_p = true; > - if (m_record_lines_p) > - { > - /* When we switch files we insert an end maker in the first file, > - switch to the second file and add a new line entry. The > - problem is that the end marker inserted in the first file will > - discard any previous line entries at the same address. If the > - line entries in the first file are marked as is-stmt, while > - the new line in the second file is non-stmt, then this means > - the end marker will discard is-stmt lines so we can have a > - non-stmt line. This means that there are less addresses at > - which the user can insert a breakpoint. > - > - To improve this we track the last address in m_last_address, > - and whether we have seen an is-stmt at this address. Then > - when switching files, if we have seen a stmt at the current > - address, and we are switching to create a non-stmt line, then > - discard the new line. */ > - bool file_changed > - = m_last_subfile != m_cu->get_builder ()->get_current_subfile (); > - bool ignore_this_line > - = ((file_changed && !end_sequence && m_last_address == m_address > - && !m_is_stmt && m_stmt_at_address) > - || (!end_sequence && m_line == 0)); > - > - if ((file_changed && !ignore_this_line) || end_sequence) > + if (m_record_lines_p && (end_sequence || m_line != 0)) > + { > + if (m_last_subfile != m_cu->get_builder ()->get_current_subfile () > + || 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 || m_is_stmt); > } > > - if (!end_sequence && !ignore_this_line) > + if (!end_sequence) > { > bool is_stmt = producer_is_codewarrior (m_cu) || m_is_stmt; > > @@ -21146,15 +21112,6 @@ class lnp_state_machine > } > } > } > - > - /* Track whether we have seen any m_is_stmt true at m_address in case we > - have multiple line table entries all at m_address. */ > - if (m_last_address != m_address) > - { > - m_stmt_at_address = false; > - m_last_address = m_address; > - } > - m_stmt_at_address |= m_is_stmt; > } > > lnp_state_machine::lnp_state_machine (struct dwarf2_cu *cu, gdbarch *arch, > @@ -21174,9 +21131,6 @@ class lnp_state_machine > m_address = gdbarch_adjust_dwarf2_line (arch, 0, 0); > m_is_stmt = lh->default_is_stmt; > m_discriminator = 0; > - > - m_last_address = m_address; > - m_stmt_at_address = false; > } > > void > diff --git a/gdb/infcmd.c b/gdb/infcmd.c > index c183b60..e1d9bb1 100644 > --- a/gdb/infcmd.c > +++ b/gdb/infcmd.c > @@ -976,7 +976,8 @@ enum async_reply_reason > if (SYMBOL_CLASS (sym) == LOC_BLOCK) > { > const block *block = SYMBOL_BLOCK_VALUE (sym); > - if (BLOCK_END (block) < tp->control.step_range_end) > + if (BLOCK_END (block) < tp->control.step_range_end > + && BLOCK_END (block) > tp->control.step_range_start) > tp->control.step_range_end = BLOCK_END (block); > } > } > diff --git a/gdb/infrun.c b/gdb/infrun.c > index 694bbe6..37d44ae 100644 > --- a/gdb/infrun.c > +++ b/gdb/infrun.c > @@ -7106,6 +7106,31 @@ struct wait_one_event > return; > } > > + /* Handle the case when subroutines have multiple ranges. > + When we step from one part to the next part of the same subroutine, > + all subroutine levels are skipped again which begin here. > + Compensate for this by removing all skipped subroutines, > + which were already executing from the user's perspective. */ > + > + if (frame_id_eq (get_stack_frame_id (get_current_frame ()), > + ecs->event_thread->control.step_stack_frame_id) > + && inline_skipped_frames (ecs->event_thread) > + && ecs->event_thread->control.step_frame_id.artificial_depth > 0 > + && ecs->event_thread->control.step_frame_id.code_addr_p) > + { > + const struct block *prev, *curr; > + int depth = 0; > + prev = block_for_pc (ecs->event_thread->control.step_frame_id.code_addr); > + curr = block_for_pc (ecs->event_thread->stop_pc ()); > + while (curr && block_inlined_p (curr) && !contained_in (prev, curr)) > + { > + depth ++; > + curr = BLOCK_SUPERBLOCK (curr); > + } > + while (inline_skipped_frames (ecs->event_thread) > depth) > + step_into_inline_frame (ecs->event_thread); > + } > + > /* Look for "calls" to inlined functions, part one. If the inline > frame machinery detected some skipped call sites, we have entered > a new inline function. */ > @@ -7167,6 +7192,8 @@ struct wait_one_event > infrun_debug_printf ("stepping through inlined function"); > > if (ecs->event_thread->control.step_over_calls == STEP_OVER_ALL > + || ecs->event_thread->stop_pc () != stop_pc_sal.pc > + || !stop_pc_sal.is_stmt > || inline_frame_is_marked_for_skip (false, ecs->event_thread)) > keep_going (ecs); > else > @@ -7192,8 +7219,8 @@ struct wait_one_event > end_stepping_range (ecs); > return; > } > - else if (frame_id_eq (get_frame_id (get_current_frame ()), > - ecs->event_thread->control.step_frame_id)) > + else if (frame_id_eq (get_stack_frame_id (get_current_frame ()), > + ecs->event_thread->control.step_stack_frame_id)) > { > /* We are not at the start of a statement, and we have not changed > frame. > @@ -7237,7 +7264,7 @@ struct wait_one_event > ecs->event_thread->control.step_range_end = stop_pc_sal.end; > ecs->event_thread->control.may_range_step = 1; > if (refresh_step_info) > - set_step_info (ecs->event_thread, frame, stop_pc_sal); > + set_step_info (ecs->event_thread, get_current_frame (), stop_pc_sal); > > infrun_debug_printf ("keep going"); > keep_going (ecs); > diff --git a/gdb/symtab.c b/gdb/symtab.c > index fa3f422..5e95d8a 100644 > --- a/gdb/symtab.c > +++ b/gdb/symtab.c > @@ -3269,7 +3269,11 @@ struct symtab_and_line > save prev if it represents the end of a function (i.e. line number > 0) instead of a real line. */ > > - if (prev && prev->line && (!best || prev->pc > best->pc)) > + if (prev && prev->line && (!best || prev->pc > best->pc > + || (prev->pc == best->pc > + && (best->pc == pc > + ? !best->is_stmt > + : best->is_weak)))) > { > best = prev; > best_symtab = iter_s; > @@ -3287,7 +3291,7 @@ struct symtab_and_line > while (tmp > first && (tmp - 1)->pc == tmp->pc > && (tmp - 1)->line != 0 && !tmp->is_stmt) > --tmp; > - if (tmp->is_stmt) > + if (tmp->is_stmt && (tmp->pc == pc || !tmp->is_weak)) > best = tmp; > } > > @@ -3310,18 +3314,14 @@ struct symtab_and_line > We used to return alt->line - 1 here, but that could be > anywhere; if we don't have line number info for this PC, > don't make some up. */ > - val.pc = pc; > - } > - else if (best->line == 0) > - { > - /* If our best fit is in a range of PC's for which no line > - number info is available (line number is zero) then we didn't > - find any valid line information. */ > + if (notcurrent) > + pc++; > val.pc = pc; > } > else > { > val.is_stmt = best->is_stmt; > + val.is_weak = best->is_weak; > val.symtab = best_symtab; > val.line = best->line; > val.pc = best->pc; > diff --git a/gdb/testsuite/gdb.base/empty-inline.c b/gdb/testsuite/gdb.base/empty-inline.c > new file mode 100644 > index 0000000..46944f8 > --- /dev/null > +++ b/gdb/testsuite/gdb.base/empty-inline.c > @@ -0,0 +1,43 @@ > +/* This testcase is part of GDB, the GNU debugger. > + > + Copyright 2021 Free Software Foundation, Inc. > + > + This program is free software; you can redistribute it and/or modify > + it under the terms of the GNU General Public License as published by > + the Free Software Foundation; either version 3 of the License, or > + (at your option) any later version. > + > + This program is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + GNU General Public License for more details. > + > + You should have received a copy of the GNU General Public License > + along with this program. If not, see <http://www.gnu.org/licenses/>. */ > + > +static int test0 (void) > +{ > + asm (""); > + return 1; /* line 21 */ > +} > + > +int __attribute__((noinline, noclone)) > +#ifdef __CET__ > + __attribute__((nocf_check)) > +#endif > +test1 (int x) > +{ > + asm (""); > + return x+1; /* line 31 */ > +} > + > +int > +#ifdef __CET__ > + __attribute__((nocf_check)) > +#endif > +main() > +{ > + test1 (test0 ()); /* line 40 */ > + test1 (test0 ()); /* line 41 */ > + return 0; > +} > diff --git a/gdb/testsuite/gdb.base/empty-inline.exp b/gdb/testsuite/gdb.base/empty-inline.exp > new file mode 100644 > index 0000000..5f3e04a > --- /dev/null > +++ b/gdb/testsuite/gdb.base/empty-inline.exp > @@ -0,0 +1,50 @@ > +# Copyright 2021 Free Software Foundation, Inc. > + > +# This program is free software; you can redistribute it and/or modify > +# it under the terms of the GNU General Public License as published by > +# the Free Software Foundation; either version 3 of the License, or > +# (at your option) any later version. > +# > +# This program is distributed in the hope that it will be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > +# > +# You should have received a copy of the GNU General Public License > +# along with this program. If not, see <http://www.gnu.org/licenses/>. > + > +standard_testfile .c > + > +if [get_compiler_info] { > + return -1 > +} > + > +if { [test_compiler_info gcc*] && ![supports_statement_frontiers] } { > + return -1 > +} > + > +global srcfile testfile > + > +set options {debug nowarnings optimize=-O2} > +if { [supports_statement_frontiers] } { > + lappend options additional_flags=-gstatement-frontiers > +} > + > +if { [prepare_for_testing "failed to prepare" $binfile \ > + $srcfile $options] } { > + return -1 > +} > + > +if ![runto_main] { > + fail "can't run to main" > + return -1 > +} > + > +gdb_test "frame 0" "\\s*\\#0\\s+main.*${srcfile}:40.*" "in main" > +gdb_test "step" ".*test0.*${srcfile}:21.*" "step into test0" > +gdb_test "frame 1" "\\s*\\#1\\s+main.*${srcfile}:40.*" "frame1" > +gdb_test "step" ".*test1.*${srcfile}:31.*" "step into test1" > +gdb_test "frame 1" "\\s*\\#1.*in main.*${srcfile}:40.*" "frame2" > +gdb_test "step" ".*main.*${srcfile}:41.*" "step back to main" > +gdb_test "next" ".*return 0;.*" "step over test0+1" > +gdb_test "frame 0" "\\s*\\#0\\s+main.*${srcfile}:42.*" "in main again" > diff --git a/gdb/testsuite/gdb.cp/empty-inline.cc b/gdb/testsuite/gdb.cp/empty-inline.cc > new file mode 100644 > index 0000000..d23f45c > --- /dev/null > +++ b/gdb/testsuite/gdb.cp/empty-inline.cc > @@ -0,0 +1,33 @@ > +/* This testcase is part of GDB, the GNU debugger. > + > + Copyright 2021 Free Software Foundation, Inc. > + > + This program is free software; you can redistribute it and/or modify > + it under the terms of the GNU General Public License as published by > + the Free Software Foundation; either version 3 of the License, or > + (at your option) any later version. > + > + This program is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + GNU General Public License for more details. > + > + You should have received a copy of the GNU General Public License > + along with this program. If not, see <http://www.gnu.org/licenses/>. */ > + > +/* PR 25987 */ > +struct MyClass; > +struct ptr { > + MyClass* get() { return t; } /* line 21 */ > + MyClass* t; > +}; > +struct MyClass { void call(); }; > +void MyClass::call() { > + *(volatile char*)(nullptr) = 1; /* line 26 */ > +} > +static void intermediate(ptr p) { > + p.get()->call(); /* line 29 */ > +} > +int main() { > + intermediate(ptr{new MyClass}); > +} > diff --git a/gdb/testsuite/gdb.cp/empty-inline.exp b/gdb/testsuite/gdb.cp/empty-inline.exp > new file mode 100644 > index 0000000..98f4058 > --- /dev/null > +++ b/gdb/testsuite/gdb.cp/empty-inline.exp > @@ -0,0 +1,55 @@ > +# Copyright 2021 Free Software Foundation, Inc. > + > +# This program is free software; you can redistribute it and/or modify > +# it under the terms of the GNU General Public License as published by > +# the Free Software Foundation; either version 3 of the License, or > +# (at your option) any later version. > +# > +# This program is distributed in the hope that it will be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > +# > +# You should have received a copy of the GNU General Public License > +# along with this program. If not, see <http://www.gnu.org/licenses/>. > + > +# PR 25987 > +standard_testfile .cc > + > +if {[skip_cplus_tests]} { > + return -1 > +} > + > +if {![supports_statement_frontiers]} { > + return -1 > +} > + > +set options {c++ debug nowarnings optimize=-Og} > +lappend options additional_flags=-gstatement-frontiers > +if { [prepare_for_testing "failed to prepare" $testfile \ > + $srcfile $options] } { > + return -1 > +} > + > +if ![runto_main] { > + fail "can't run to main" > + return > +} > + > +gdb_test "bt" "\\s*\\#0\\s+main.*" "in main" > +#break at the empty inline function ptr::get > +gdb_test "b get" ".*" "break at get" > +gdb_test "c" ".*" "continue to get" > +#call frame 1 is at line 29 > +gdb_test "bt" [multi_line "\\s*\\#0\\s+ptr::get\[^\r\]*${srcfile}:21" \ > + "\\s*\\#1\\s+intermediate\[^\r\]*${srcfile}:29" \ > + ".*"] \ > + "at get" > +#print a local value here > +gdb_test "p t" ".*\\\$1 = \\(MyClass \\*\\) 0x.*" "print t" > +gdb_test "c" ".*SIGSEGV.*" "continue to SIGSEGV" > +#call frame 1 is at line 29 > +gdb_test "bt" [multi_line "\\s*\\#0\\s+MyClass::call\[^\r\]*${srcfile}:26" \ > + "\\s*\\#1\\s+0x\[^\r\]*${srcfile}:29" \ > + ".*"] \ > + "at call" > diff --git a/gdb/testsuite/gdb.cp/step-and-next-inline.cc b/gdb/testsuite/gdb.cp/step-and-next-inline.cc > index 26b29d0..d64d155 100644 > --- a/gdb/testsuite/gdb.cp/step-and-next-inline.cc > +++ b/gdb/testsuite/gdb.cp/step-and-next-inline.cc > @@ -46,6 +46,9 @@ tree_check (tree *t, int i) > #endif /* USE_NEXT_INLINE_H */ > > int __attribute__((noinline, noclone)) > +#ifdef __CET__ > + __attribute__((nocf_check)) > +#endif > get_alias_set (tree *t) > { > if (t != NULL > @@ -59,6 +62,9 @@ get_alias_set (tree *t) > tree xx; > > int > +#ifdef __CET__ > + __attribute__((nocf_check)) > +#endif > main() > { > get_alias_set (&xx); > diff --git a/gdb/testsuite/gdb.cp/step-and-next-inline.exp b/gdb/testsuite/gdb.cp/step-and-next-inline.exp > index 13d801b..5af1e38 100644 > --- a/gdb/testsuite/gdb.cp/step-and-next-inline.exp > +++ b/gdb/testsuite/gdb.cp/step-and-next-inline.exp > @@ -20,7 +20,7 @@ if [get_compiler_info "c++"] { > return -1 > } > > -if {[test_compiler_info gcc*] && ![supports_statement_frontiers] } { > +if { [test_compiler_info gcc*] && ![supports_statement_frontiers] } { > return -1 > } > > @@ -29,13 +29,6 @@ if {[test_compiler_info gcc*] && ![supports_statement_frontiers] } { > proc do_test { use_header } { > global srcfile testfile > > - if { $use_header } { > - # This test will not pass due to poor debug information > - # generated by GCC (at least upto 10.x). See > - # https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94474 > - return > - } > - > set options {c++ debug nowarnings optimize=-O2} > if { [supports_statement_frontiers] } { > lappend options additional_flags=-gstatement-frontiers > @@ -67,109 +60,21 @@ 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. > - set have_kfail [expr [test_compiler_info gcc*] && !$use_header] > - > - set ok 1 > - gdb_test_multiple "next" "next step 1" { > - -re -wrap "if \\(t->x != i\\)" { > - set ok 0 > - send_gdb "next\n" > - exp_continue > - } > - -re -wrap ".*TREE_TYPE.* != 1" { > - if { $ok } { > - pass $gdb_test_name > - } else { > - if { $have_kfail } { > - setup_kfail "*-*-*" symtab/25507 > - } > - fail $gdb_test_name > - } > - } > - } > + gdb_test "next" ".*TREE_TYPE.*" "next step 1" > gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \ > "not in inline 2" > - > - set ok 1 > - gdb_test_multiple "next" "next step 2" { > - -re -wrap "return x;" { > - set ok 0 > - send_gdb "next\n" > - exp_continue > - } > - -re -wrap ".*TREE_TYPE.* != 2" { > - if { $ok } { > - pass $gdb_test_name > - } else { > - if { $have_kfail } { > - setup_kfail "*-*-*" symtab/25507 > - } > - fail $gdb_test_name > - } > - } > - } > + gdb_test "next" ".*TREE_TYPE.*" "next step 2" > gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \ > "not in inline 3" > - > - set ok 1 > - gdb_test_multiple "next" "next step 3" { > - -re -wrap "return x;" { > - set ok 0 > - send_gdb "next\n" > - exp_continue > - } > - -re -wrap ".*TREE_TYPE.* != 3\\)" { > - if { $ok } { > - pass $gdb_test_name > - } else { > - if { $have_kfail } { > - setup_kfail "*-*-*" symtab/25507 > - } > - fail $gdb_test_name > - } > - } > - } > + gdb_test "next" ".*TREE_TYPE.*" "next step 3" > gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \ > "not in inline 4" > - > - set ok 1 > - gdb_test_multiple "next" "next step 4" { > - -re -wrap "(if \\(t != NULL|\} // get_alias_set)" { > - send_gdb "next\n" > - exp_continue > - } > - -re -wrap "return x;" { > - set ok 0 > - send_gdb "next\n" > - exp_continue > - } > - -re -wrap "return 0.*" { > - if { $ok } { > - pass $gdb_test_name > - } else { > - if { $have_kfail } { > - setup_kfail "*-*-*" symtab/25507 > - } > - fail $gdb_test_name > - } > - } > - } > + gdb_test "next" ".*" "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" > + if ![test_compiler_info gcc*] { > return > } > > @@ -190,22 +95,79 @@ proc do_test { use_header } { > gdb_test "step" ".*if \\(t->x != i\\).*" "step 2" > gdb_test "bt" "\\s*\\#0\\s+\[^\r\]*tree_check\[^\r\]*${hdrfile}:.*" \ > "in inline 1 pass 2" > - gdb_test "step" ".*TREE_TYPE.*" "step 3" > + gdb_test "step" ".*return x.*" "step 3" > + gdb_test "bt" "\\s*\\#0\\s+\[^\r\]*tree_check\[^\r\]*${hdrfile}:.*" \ > + "return from inline 1 pass 2" > + gdb_test "step" ".*TREE_TYPE.*" "step 4" > gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \ > "not in inline 2 pass 2" > - gdb_test "step" ".*if \\(t->x != i\\).*" "step 4" > + gdb_test "step" ".*if \\(t->x != i\\).*" "step 5" > gdb_test "bt" "\\s*\\#0\\s+\[^\r\]*tree_check\[^\r\]*${hdrfile}:.*" \ > "in inline 2 pass 2" > - gdb_test "step" ".*TREE_TYPE.*" "step 5" > + gdb_test "step" ".*return x.*" "step 6" > + gdb_test "bt" "\\s*\\#0\\s+\[^\r\]*tree_check\[^\r\]*${hdrfile}:.*" \ > + "return from inline 2 pass 2" > + gdb_test "step" ".*TREE_TYPE.*" "step 7" > gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \ > "not in inline 3 pass 2" > - gdb_test "step" ".*if \\(t->x != i\\).*" "step 6" > + gdb_test "step" ".*if \\(t->x != i\\).*" "step 8" > gdb_test "bt" "\\s*\\#0\\s+\[^\r\]*tree_check\[^\r\]*${hdrfile}:.*" \ > "in inline 3 pass 2" > - gdb_test "step" "return 0.*" "step 7" > + gdb_test "step" ".*return x.*" "step 9" > + gdb_test "bt" "\\s*\\#0\\s+\[^\r\]*tree_check\[^\r\]*${hdrfile}:.*" \ > + "return from inline 3 pass 2" > + gdb_test "step" "return 0.*" "step 10" > gdb_test "bt" \ > "\\s*\\#0\\s+(main|get_alias_set)\[^\r\]*${srcfile}:.*" \ > "not in inline 4 pass 2" > + > + clean_restart ${executable} > + > + if ![runto_main] { > + fail "can't run to main pass 3" > + return > + } > + > + gdb_test "bt" "\\s*\\#0\\s+main.*" "in main pass 3" > + gdb_test "step" ".*" "step into get_alias_set pass 3" > + gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \ > + "in get_alias_set pass 3" > + gdb_test "step" ".*TREE_TYPE.*" "step 1 pass 3" > + gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \ > + "not in inline 1 pass 3" > + gdb_test "step" ".*if \\(t->x != i\\).*" "step 2 pass 3" > + gdb_test "bt" "\\s*\\#0\\s+\[^\r\]*tree_check\[^\r\]*${hdrfile}:.*" \ > + "in inline 1 pass 3" > + gdb_test "p t->x = 2" ".* = 2.*" "change value pass 3" > + gdb_test "step" ".*abort.*" "step 4, pass 3" > + gdb_test "bt" "\\s*\\#0\\s+\[^\r\]*tree_check\[^\r\]*${hdrfile}:.*" \ > + "abort from inline 1 pass 3" > + > + clean_restart ${executable} > + > + if ![runto_main] { > + fail "can't run to main pass 4" > + return > + } > + > + gdb_test "bt" "\\s*\\#0\\s+main.*" "in main pass 4" > + gdb_test "skip tree_check" ".*" "skip tree_check pass 4" > + gdb_test "step" ".*" "step into get_alias_set pass 4" > + gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \ > + "in get_alias_set pass 4" > + gdb_test "step" ".*TREE_TYPE.*" "step 1 pass 4" > + gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \ > + "not in inline 1 pass 4" > + gdb_test "step" ".*TREE_TYPE.*" "step 2 pass 4" > + gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \ > + "not in inline 2 pass 4" > + gdb_test "step" ".*TREE_TYPE.*" "step 3 pass 4" > + gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \ > + "not in inline 3 pass 4" > + gdb_test "step" "return 0.*" "step 4 pass 4" > + gdb_test "bt" \ > + "\\s*\\#0\\s+(main|get_alias_set)\[^\r\]*${srcfile}:.*" \ > + "not in inline 4 pass 4" > } > } > > -- > 1.9.1 > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3 2/3] Fix range end handling of inlined subroutines 2021-09-10 9:11 ` Andrew Burgess @ 2021-09-12 7:47 ` Bernd Edlinger 0 siblings, 0 replies; 5+ messages in thread From: Bernd Edlinger @ 2021-09-12 7:47 UTC (permalink / raw) To: Andrew Burgess; +Cc: gdb-patches On 9/10/21 11:11 AM, Andrew Burgess wrote: > * Bernd Edlinger <bernd.edlinger@hotmail.de> [2021-09-05 21:15:52 +0200]: > >> Currently there is a problem when debugging >> optimized code when the inferior stops at an inline >> sub-range end PC. It is unclear if that location >> is from the inline function or from the calling >> function. Therefore the call stack is often >> wrong. >> >> This patch detects the "weak" line table entries >> which are likely part of the previous inline block, >> and if we have such a location, it assumes the >> location belongs to the previous block. >> >> Additionally it may happen that the infrun machinery >> steps from one inline range to another inline range >> of the same inline function. That can look like >> jumping back and forth from the calling program >> to the inline function, while really the inline >> function just jumps from a hot to a cold section >> of the code, i.e. error handling. >> >> Additionally it may happen that one inline sub-range >> is empty or the inline is completely empty. But >> filtering that information away is not the right >> solution, since although there is no actual code >> from the inline, it is still possible that variables >> from an inline function can be inspected here. >> >> Conceptually this patch uses a heuristic to work around >> a deficiency in the dwarf-4 and dwarf-5 rnglists structure. >> There should be a location view number for each inline >> sub-range begin PC and end PC, similar to the DW_AT_GNU_entry_view >> which is the location view for the inline entry point. > > I know that we've discussed this before, and that we disagree on > whether there is a GCC bug that is contributing to this problem or > not, but I find it weird that you have not linked to: > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94474 > > This is the still that patch that aims to address this issue, right? Yes and no, at that time the issues with inline functions from header files were resolved, but differently as this patch here tries to do. The patch that was in discussion at that time tried to extend that solution: silently remove some breakpoints at the end of the inline function, when everything is in the same source file. Then with your patch 1313c56ef9a ("gdb: Preserve is-stmt lines when switch between files") that was no longer possible. The idea of that patch was that it is under no circumstance acceptable to remove any possible breakpoints from the line table. Then I tried to work out a gcc-patch which changes the default of -gstatement-frontiers and/or -gvariable-location-views, but realized that it would break too many things in the gcc testsuite, So I looked again at the abandoned patch and tried a different strategy, that is inspired by your patch, that I do no longer try to delete the line breakpoints where the inline function returns. So the patch has some stuff in common with the original version, but it tries to preserve the line breakpoints, and I also discovered something that you can do at those breakpoint positions at the end of an inline frame: You can print rhe result that was returned from the subroutine, so for instance: gdb.cp/empty-inline.cc struct ptr { MyClass* get() { return t; } /* line 21 */ MyClass* t; }; the test case does set a breakpoint at "get" and there we have an empty inline function, but something unexpected is possible here, at this point we are able to print the return value "t" now: gdb.cp/empty-inline: #print a local value here gdb_test "p t" ".*\\\$1 = \\(MyClass \\*\\) 0x.*" "print t" So I think it is clear that while there may be corner cases where the debug info is weird, it is impossible to look at the return value of an identity function, and if a function returns something that is likely in a register only after the instruction, but the return statement usually does not need any code by itself. At a later time we will probably have the view numbers where each inline range begins and ends, but we will probably live for very long with old toolchains and need a reasonable way to handle those. > > Please don't misunderstand me, I am absolutely not saying that we > shouldn't work around GCC bugs in GDB. And as you point out, there > might also be deficiencies in the DWARF spec that impact on this issue > too. I think that the description of this fix needs to touch on all > these issues. > As I see it now, the DWARF spec is wrong to forbid break points at the return statement of an inline function, this is a useful feature when you want to see the return value. Should I add that to the commit message? > Finally, I really think the description needs to include actual > examples of DWARF output along with an explanation of how your patch > solves the problem. > What exactly is unclear? > I've started looking at this series of yours several times, and each > time I run out of time and give up, mostly because I have to start > from first principles trying to figure out what's going on, I'm trying > to answer the question _why_ is this the right way to solve this > problem, and I'm just not getting that from your commit messages. > I would also be open for suggestions where comments are missing. Please send me a note, when you dig into the patch, and get stuck, at some point, that is clearly an indication that something can be improved, or maybe a comment is missing somewhere. Thanks Bernd. > Thanks, > Andrew > > > > >> >> gdb: >> 2021-01-01 Bernd Edlinger <bernd.edlinger@hotmail.de> >> >> * block.c (blockvector_for_pc_sect): For weak line table values, >> use the inline frame at PC - 1 if it is deeper. >> * dwarf2/read.c (dwarf2_rnglists_process, >> dwarf2_ranges_process): Don't ignore empty ranges here. >> (dwarf2_ranges_read): Ignore empty ranges here. >> (dwarf2_get_pc_bounds): Allow empty inlined subroutines. >> (partial_die_info::read): Likewise. >> (dwarf_finish_line): Add new parameter end_sequence. >> (lnp_state_machine::m_last_file, lnp_state_machine::m_last_address, >> lnp_state_machine::m_stmt_at_address): Remove data items. >> (lnp_state_machine::record_line): Do not filter line entries here, >> except for line == 0. Pass end_sequence to dwarf_finish_line. >> * infcmd.c (prepare_one_step): Ignore empty inline ranges. >> * infrun.c (process_event_stop_test): Don't stop in !is_stmt lines >> when stepping into inlines. Don't stop at the call site again, when >> stepping within inlined subroutines with multiple ranges. >> Don't refresh the step info the when executing !is_stmt lines of >> of a different inlined subroutine. >> * symtab.c (find_pc_sect_line): Handle is_weak. >> >> gdb/testsuite: >> 2021-01-01 Bernd Edlinger <bernd.edlinger@hotmail.de> >> >> * gdb.base/empty-inline.exp: New test. >> * gdb.base/empty-inline.c: New test. >> * gdb.cp/empty-inline.exp: New test. >> * gdb.cp/empty-inline.cc: New test. >> * gdb.cp/step-and-next-inline.exp: Remove KFAILs, enhance test. >> * gdb.cp/step-and-next-inline.cc: Work around failure with >> gcc-9.3.0 from ubuntu 20.04. >> --- >> gdb/block.c | 15 ++- >> gdb/dwarf2/read.c | 84 +++---------- >> gdb/infcmd.c | 3 +- >> gdb/infrun.c | 33 ++++- >> gdb/symtab.c | 18 +-- >> gdb/testsuite/gdb.base/empty-inline.c | 43 +++++++ >> gdb/testsuite/gdb.base/empty-inline.exp | 50 ++++++++ >> gdb/testsuite/gdb.cp/empty-inline.cc | 33 +++++ >> gdb/testsuite/gdb.cp/empty-inline.exp | 55 ++++++++ >> gdb/testsuite/gdb.cp/step-and-next-inline.cc | 6 + >> gdb/testsuite/gdb.cp/step-and-next-inline.exp | 174 ++++++++++---------------- >> 11 files changed, 329 insertions(+), 185 deletions(-) >> create mode 100644 gdb/testsuite/gdb.base/empty-inline.c >> create mode 100644 gdb/testsuite/gdb.base/empty-inline.exp >> create mode 100644 gdb/testsuite/gdb.cp/empty-inline.cc >> create mode 100644 gdb/testsuite/gdb.cp/empty-inline.exp >> > >> From 7112251fb65740d7cafa97034ba2b72c0fced721 Mon Sep 17 00:00:00 2001 >> From: Bernd Edlinger <bernd.edlinger@hotmail.de> >> Date: Tue, 3 Nov 2020 18:41:43 +0100 >> Subject: [PATCH v3 2/3] Fix range end handling of inlined subroutines >> >> Currently there is a problem when debugging >> optimized code when the inferior stops at an inline >> sub-range end PC. It is unclear if that location >> is from the inline function or from the calling >> function. Therefore the call stack is often >> wrong. >> >> This patch detects the "weak" line table entries >> which are likely part of the previous inline block, >> and if we have such a location, it assumes the >> location belongs to the previous block. >> >> Additionally it may happen that the infrun machinery >> steps from one inline range to another inline range >> of the same inline function. That can look like >> jumping back and forth from the calling program >> to the inline function, while really the inline >> function just jumps from a hot to a cold section >> of the code, i.e. error handling. >> >> Additionally it may happen that one inline sub-range >> is empty or the inline is completely empty. But >> filtering that information away is not the right >> solution, since although there is no actual code >> from the inline, it is still possible that variables >> from an inline function can be inspected here. >> >> Conceptually this patch uses a heuristic to work around >> a deficiency in the dwarf-4 and dwarf-5 rnglists structure. >> There should be a location view number for each inline >> sub-range begin PC and end PC, similar to the DW_AT_GNU_entry_view >> which is the location view for the inline entry point. >> >> gdb: >> 2021-01-01 Bernd Edlinger <bernd.edlinger@hotmail.de> >> >> * block.c (blockvector_for_pc_sect): For weak line table values, >> use the inline frame at PC - 1 if it is deeper. >> * dwarf2/read.c (dwarf2_rnglists_process, >> dwarf2_ranges_process): Don't ignore empty ranges here. >> (dwarf2_ranges_read): Ignore empty ranges here. >> (dwarf2_get_pc_bounds): Allow empty inlined subroutines. >> (partial_die_info::read): Likewise. >> (dwarf_finish_line): Add new parameter end_sequence. >> (lnp_state_machine::m_last_file, lnp_state_machine::m_last_address, >> lnp_state_machine::m_stmt_at_address): Remove data items. >> (lnp_state_machine::record_line): Do not filter line entries here, >> except for line == 0. Pass end_sequence to dwarf_finish_line. >> * infcmd.c (prepare_one_step): Ignore empty inline ranges. >> * infrun.c (process_event_stop_test): Don't stop in !is_stmt lines >> when stepping into inlines. Don't stop at the call site again, when >> stepping within inlined subroutines with multiple ranges. >> Don't refresh the step info the when executing !is_stmt lines of >> of a different inlined subroutine. >> * symtab.c (find_pc_sect_line): Handle is_weak. >> >> gdb/testsuite: >> 2021-01-01 Bernd Edlinger <bernd.edlinger@hotmail.de> >> >> * gdb.base/empty-inline.exp: New test. >> * gdb.base/empty-inline.c: New test. >> * gdb.cp/empty-inline.exp: New test. >> * gdb.cp/empty-inline.cc: New test. >> * gdb.cp/step-and-next-inline.exp: Remove KFAILs, enhance test. >> * gdb.cp/step-and-next-inline.cc: Work around failure with >> gcc-9.3.0 from ubuntu 20.04. >> --- >> gdb/block.c | 15 ++- >> gdb/dwarf2/read.c | 84 +++---------- >> gdb/infcmd.c | 3 +- >> gdb/infrun.c | 33 ++++- >> gdb/symtab.c | 18 +-- >> gdb/testsuite/gdb.base/empty-inline.c | 43 +++++++ >> gdb/testsuite/gdb.base/empty-inline.exp | 50 ++++++++ >> gdb/testsuite/gdb.cp/empty-inline.cc | 33 +++++ >> gdb/testsuite/gdb.cp/empty-inline.exp | 55 ++++++++ >> gdb/testsuite/gdb.cp/step-and-next-inline.cc | 6 + >> gdb/testsuite/gdb.cp/step-and-next-inline.exp | 174 ++++++++++---------------- >> 11 files changed, 329 insertions(+), 185 deletions(-) >> create mode 100644 gdb/testsuite/gdb.base/empty-inline.c >> create mode 100644 gdb/testsuite/gdb.base/empty-inline.exp >> create mode 100644 gdb/testsuite/gdb.cp/empty-inline.cc >> create mode 100644 gdb/testsuite/gdb.cp/empty-inline.exp >> >> diff --git a/gdb/block.c b/gdb/block.c >> index 4cb9573..2a0ce6f 100644 >> --- a/gdb/block.c >> +++ b/gdb/block.c >> @@ -205,7 +205,20 @@ struct symbol * >> return NULL; >> >> if (pblock) >> - *pblock = b; >> + { >> + struct symtab_and_line sal = find_pc_sect_line (pc, section, 0); >> + if (sal.line != 0 && sal.pc == pc && sal.is_weak) >> + { >> + const struct block *b2 = find_block_in_blockvector (bl, pc - 1); >> + const struct block *b0 = b; >> + while (BLOCK_SUPERBLOCK (b0) && !BLOCK_FUNCTION (b0)) >> + b0 = BLOCK_SUPERBLOCK (b0); >> + if (contained_in (b2, b0)) >> + b = b2; >> + } >> + *pblock = b; >> + } >> + >> return bl; >> } >> >> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c >> index 5385a3a..358266a 100644 >> --- a/gdb/dwarf2/read.c >> +++ b/gdb/dwarf2/read.c >> @@ -13838,10 +13838,6 @@ class process_die_scope >> return false; >> } >> >> - /* Empty range entries have no effect. */ >> - if (range_beginning == range_end) >> - continue; >> - >> /* Only DW_RLE_offset_pair needs the base address added. */ >> if (rlet == DW_RLE_offset_pair) >> { >> @@ -13957,10 +13953,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; >> >> @@ -14001,6 +13993,10 @@ class process_die_scope >> retval = dwarf2_ranges_process (offset, cu, tag, >> [&] (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; >> @@ -14109,8 +14105,8 @@ class process_die_scope >> return PC_BOUNDS_NOT_PRESENT; >> } >> >> - /* partial_die_info::read has also the strict LOW < HIGH requirement. */ >> - if (high <= low) >> + /* partial_die_info::read has also the same low < high requirement. */ >> + if (low > high || (low == high && die->tag != DW_TAG_inlined_subroutine)) >> return PC_BOUNDS_INVALID; >> >> /* When using the GNU linker, .gnu.linkonce. sections are used to >> @@ -19439,8 +19435,9 @@ struct type * >> sect_offset_str (sect_off), >> objfile_name (objfile)); >> } >> - /* dwarf2_get_pc_bounds has also the strict low < high requirement. */ >> - else if (lowpc >= highpc) >> + /* dwarf2_get_pc_bounds has also the same low < high requirement. */ >> + else if (lowpc > highpc >> + || (lowpc == highpc && tag != DW_TAG_inlined_subroutine)) >> { >> struct objfile *objfile = per_objfile->objfile; >> struct gdbarch *gdbarch = objfile->arch (); >> @@ -20891,21 +20888,9 @@ class lnp_state_machine >> >> /* Additional bits of state we need to track. */ >> >> - /* The last file that we called dwarf2_start_subfile for. >> - This is only used for TLLs. */ >> - unsigned int m_last_file = 0; >> /* The last file a line number was recorded for. */ >> struct subfile *m_last_subfile = NULL; >> >> - /* The address of the last line entry. */ >> - CORE_ADDR m_last_address; >> - >> - /* Set to true when a previous line at the same address (using >> - m_last_address) had m_is_stmt true. This is reset to false when a >> - line entry at a new address (m_address different to m_last_address) is >> - processed. */ >> - bool m_stmt_at_address = false; >> - >> /* When true, record the lines we decode. */ >> bool m_currently_recording_lines = false; >> >> @@ -21057,7 +21042,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; >> @@ -21070,7 +21055,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 >> @@ -21097,37 +21083,17 @@ class lnp_state_machine >> else if (m_op_index == 0 || end_sequence) >> { >> fe->included_p = true; >> - if (m_record_lines_p) >> - { >> - /* When we switch files we insert an end maker in the first file, >> - switch to the second file and add a new line entry. The >> - problem is that the end marker inserted in the first file will >> - discard any previous line entries at the same address. If the >> - line entries in the first file are marked as is-stmt, while >> - the new line in the second file is non-stmt, then this means >> - the end marker will discard is-stmt lines so we can have a >> - non-stmt line. This means that there are less addresses at >> - which the user can insert a breakpoint. >> - >> - To improve this we track the last address in m_last_address, >> - and whether we have seen an is-stmt at this address. Then >> - when switching files, if we have seen a stmt at the current >> - address, and we are switching to create a non-stmt line, then >> - discard the new line. */ >> - bool file_changed >> - = m_last_subfile != m_cu->get_builder ()->get_current_subfile (); >> - bool ignore_this_line >> - = ((file_changed && !end_sequence && m_last_address == m_address >> - && !m_is_stmt && m_stmt_at_address) >> - || (!end_sequence && m_line == 0)); >> - >> - if ((file_changed && !ignore_this_line) || end_sequence) >> + if (m_record_lines_p && (end_sequence || m_line != 0)) >> + { >> + if (m_last_subfile != m_cu->get_builder ()->get_current_subfile () >> + || 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 || m_is_stmt); >> } >> >> - if (!end_sequence && !ignore_this_line) >> + if (!end_sequence) >> { >> bool is_stmt = producer_is_codewarrior (m_cu) || m_is_stmt; >> >> @@ -21146,15 +21112,6 @@ class lnp_state_machine >> } >> } >> } >> - >> - /* Track whether we have seen any m_is_stmt true at m_address in case we >> - have multiple line table entries all at m_address. */ >> - if (m_last_address != m_address) >> - { >> - m_stmt_at_address = false; >> - m_last_address = m_address; >> - } >> - m_stmt_at_address |= m_is_stmt; >> } >> >> lnp_state_machine::lnp_state_machine (struct dwarf2_cu *cu, gdbarch *arch, >> @@ -21174,9 +21131,6 @@ class lnp_state_machine >> m_address = gdbarch_adjust_dwarf2_line (arch, 0, 0); >> m_is_stmt = lh->default_is_stmt; >> m_discriminator = 0; >> - >> - m_last_address = m_address; >> - m_stmt_at_address = false; >> } >> >> void >> diff --git a/gdb/infcmd.c b/gdb/infcmd.c >> index c183b60..e1d9bb1 100644 >> --- a/gdb/infcmd.c >> +++ b/gdb/infcmd.c >> @@ -976,7 +976,8 @@ enum async_reply_reason >> if (SYMBOL_CLASS (sym) == LOC_BLOCK) >> { >> const block *block = SYMBOL_BLOCK_VALUE (sym); >> - if (BLOCK_END (block) < tp->control.step_range_end) >> + if (BLOCK_END (block) < tp->control.step_range_end >> + && BLOCK_END (block) > tp->control.step_range_start) >> tp->control.step_range_end = BLOCK_END (block); >> } >> } >> diff --git a/gdb/infrun.c b/gdb/infrun.c >> index 694bbe6..37d44ae 100644 >> --- a/gdb/infrun.c >> +++ b/gdb/infrun.c >> @@ -7106,6 +7106,31 @@ struct wait_one_event >> return; >> } >> >> + /* Handle the case when subroutines have multiple ranges. >> + When we step from one part to the next part of the same subroutine, >> + all subroutine levels are skipped again which begin here. >> + Compensate for this by removing all skipped subroutines, >> + which were already executing from the user's perspective. */ >> + >> + if (frame_id_eq (get_stack_frame_id (get_current_frame ()), >> + ecs->event_thread->control.step_stack_frame_id) >> + && inline_skipped_frames (ecs->event_thread) >> + && ecs->event_thread->control.step_frame_id.artificial_depth > 0 >> + && ecs->event_thread->control.step_frame_id.code_addr_p) >> + { >> + const struct block *prev, *curr; >> + int depth = 0; >> + prev = block_for_pc (ecs->event_thread->control.step_frame_id.code_addr); >> + curr = block_for_pc (ecs->event_thread->stop_pc ()); >> + while (curr && block_inlined_p (curr) && !contained_in (prev, curr)) >> + { >> + depth ++; >> + curr = BLOCK_SUPERBLOCK (curr); >> + } >> + while (inline_skipped_frames (ecs->event_thread) > depth) >> + step_into_inline_frame (ecs->event_thread); >> + } >> + >> /* Look for "calls" to inlined functions, part one. If the inline >> frame machinery detected some skipped call sites, we have entered >> a new inline function. */ >> @@ -7167,6 +7192,8 @@ struct wait_one_event >> infrun_debug_printf ("stepping through inlined function"); >> >> if (ecs->event_thread->control.step_over_calls == STEP_OVER_ALL >> + || ecs->event_thread->stop_pc () != stop_pc_sal.pc >> + || !stop_pc_sal.is_stmt >> || inline_frame_is_marked_for_skip (false, ecs->event_thread)) >> keep_going (ecs); >> else >> @@ -7192,8 +7219,8 @@ struct wait_one_event >> end_stepping_range (ecs); >> return; >> } >> - else if (frame_id_eq (get_frame_id (get_current_frame ()), >> - ecs->event_thread->control.step_frame_id)) >> + else if (frame_id_eq (get_stack_frame_id (get_current_frame ()), >> + ecs->event_thread->control.step_stack_frame_id)) >> { >> /* We are not at the start of a statement, and we have not changed >> frame. >> @@ -7237,7 +7264,7 @@ struct wait_one_event >> ecs->event_thread->control.step_range_end = stop_pc_sal.end; >> ecs->event_thread->control.may_range_step = 1; >> if (refresh_step_info) >> - set_step_info (ecs->event_thread, frame, stop_pc_sal); >> + set_step_info (ecs->event_thread, get_current_frame (), stop_pc_sal); >> >> infrun_debug_printf ("keep going"); >> keep_going (ecs); >> diff --git a/gdb/symtab.c b/gdb/symtab.c >> index fa3f422..5e95d8a 100644 >> --- a/gdb/symtab.c >> +++ b/gdb/symtab.c >> @@ -3269,7 +3269,11 @@ struct symtab_and_line >> save prev if it represents the end of a function (i.e. line number >> 0) instead of a real line. */ >> >> - if (prev && prev->line && (!best || prev->pc > best->pc)) >> + if (prev && prev->line && (!best || prev->pc > best->pc >> + || (prev->pc == best->pc >> + && (best->pc == pc >> + ? !best->is_stmt >> + : best->is_weak)))) >> { >> best = prev; >> best_symtab = iter_s; >> @@ -3287,7 +3291,7 @@ struct symtab_and_line >> while (tmp > first && (tmp - 1)->pc == tmp->pc >> && (tmp - 1)->line != 0 && !tmp->is_stmt) >> --tmp; >> - if (tmp->is_stmt) >> + if (tmp->is_stmt && (tmp->pc == pc || !tmp->is_weak)) >> best = tmp; >> } >> >> @@ -3310,18 +3314,14 @@ struct symtab_and_line >> We used to return alt->line - 1 here, but that could be >> anywhere; if we don't have line number info for this PC, >> don't make some up. */ >> - val.pc = pc; >> - } >> - else if (best->line == 0) >> - { >> - /* If our best fit is in a range of PC's for which no line >> - number info is available (line number is zero) then we didn't >> - find any valid line information. */ >> + if (notcurrent) >> + pc++; >> val.pc = pc; >> } >> else >> { >> val.is_stmt = best->is_stmt; >> + val.is_weak = best->is_weak; >> val.symtab = best_symtab; >> val.line = best->line; >> val.pc = best->pc; >> diff --git a/gdb/testsuite/gdb.base/empty-inline.c b/gdb/testsuite/gdb.base/empty-inline.c >> new file mode 100644 >> index 0000000..46944f8 >> --- /dev/null >> +++ b/gdb/testsuite/gdb.base/empty-inline.c >> @@ -0,0 +1,43 @@ >> +/* This testcase is part of GDB, the GNU debugger. >> + >> + Copyright 2021 Free Software Foundation, Inc. >> + >> + This program is free software; you can redistribute it and/or modify >> + it under the terms of the GNU General Public License as published by >> + the Free Software Foundation; either version 3 of the License, or >> + (at your option) any later version. >> + >> + This program is distributed in the hope that it will be useful, >> + but WITHOUT ANY WARRANTY; without even the implied warranty of >> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + GNU General Public License for more details. >> + >> + You should have received a copy of the GNU General Public License >> + along with this program. If not, see <http://www.gnu.org/licenses/>. */ >> + >> +static int test0 (void) >> +{ >> + asm (""); >> + return 1; /* line 21 */ >> +} >> + >> +int __attribute__((noinline, noclone)) >> +#ifdef __CET__ >> + __attribute__((nocf_check)) >> +#endif >> +test1 (int x) >> +{ >> + asm (""); >> + return x+1; /* line 31 */ >> +} >> + >> +int >> +#ifdef __CET__ >> + __attribute__((nocf_check)) >> +#endif >> +main() >> +{ >> + test1 (test0 ()); /* line 40 */ >> + test1 (test0 ()); /* line 41 */ >> + return 0; >> +} >> diff --git a/gdb/testsuite/gdb.base/empty-inline.exp b/gdb/testsuite/gdb.base/empty-inline.exp >> new file mode 100644 >> index 0000000..5f3e04a >> --- /dev/null >> +++ b/gdb/testsuite/gdb.base/empty-inline.exp >> @@ -0,0 +1,50 @@ >> +# Copyright 2021 Free Software Foundation, Inc. >> + >> +# This program is free software; you can redistribute it and/or modify >> +# it under the terms of the GNU General Public License as published by >> +# the Free Software Foundation; either version 3 of the License, or >> +# (at your option) any later version. >> +# >> +# This program is distributed in the hope that it will be useful, >> +# but WITHOUT ANY WARRANTY; without even the implied warranty of >> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> +# GNU General Public License for more details. >> +# >> +# You should have received a copy of the GNU General Public License >> +# along with this program. If not, see <http://www.gnu.org/licenses/>. >> + >> +standard_testfile .c >> + >> +if [get_compiler_info] { >> + return -1 >> +} >> + >> +if { [test_compiler_info gcc*] && ![supports_statement_frontiers] } { >> + return -1 >> +} >> + >> +global srcfile testfile >> + >> +set options {debug nowarnings optimize=-O2} >> +if { [supports_statement_frontiers] } { >> + lappend options additional_flags=-gstatement-frontiers >> +} >> + >> +if { [prepare_for_testing "failed to prepare" $binfile \ >> + $srcfile $options] } { >> + return -1 >> +} >> + >> +if ![runto_main] { >> + fail "can't run to main" >> + return -1 >> +} >> + >> +gdb_test "frame 0" "\\s*\\#0\\s+main.*${srcfile}:40.*" "in main" >> +gdb_test "step" ".*test0.*${srcfile}:21.*" "step into test0" >> +gdb_test "frame 1" "\\s*\\#1\\s+main.*${srcfile}:40.*" "frame1" >> +gdb_test "step" ".*test1.*${srcfile}:31.*" "step into test1" >> +gdb_test "frame 1" "\\s*\\#1.*in main.*${srcfile}:40.*" "frame2" >> +gdb_test "step" ".*main.*${srcfile}:41.*" "step back to main" >> +gdb_test "next" ".*return 0;.*" "step over test0+1" >> +gdb_test "frame 0" "\\s*\\#0\\s+main.*${srcfile}:42.*" "in main again" >> diff --git a/gdb/testsuite/gdb.cp/empty-inline.cc b/gdb/testsuite/gdb.cp/empty-inline.cc >> new file mode 100644 >> index 0000000..d23f45c >> --- /dev/null >> +++ b/gdb/testsuite/gdb.cp/empty-inline.cc >> @@ -0,0 +1,33 @@ >> +/* This testcase is part of GDB, the GNU debugger. >> + >> + Copyright 2021 Free Software Foundation, Inc. >> + >> + This program is free software; you can redistribute it and/or modify >> + it under the terms of the GNU General Public License as published by >> + the Free Software Foundation; either version 3 of the License, or >> + (at your option) any later version. >> + >> + This program is distributed in the hope that it will be useful, >> + but WITHOUT ANY WARRANTY; without even the implied warranty of >> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + GNU General Public License for more details. >> + >> + You should have received a copy of the GNU General Public License >> + along with this program. If not, see <http://www.gnu.org/licenses/>. */ >> + >> +/* PR 25987 */ >> +struct MyClass; >> +struct ptr { >> + MyClass* get() { return t; } /* line 21 */ >> + MyClass* t; >> +}; >> +struct MyClass { void call(); }; >> +void MyClass::call() { >> + *(volatile char*)(nullptr) = 1; /* line 26 */ >> +} >> +static void intermediate(ptr p) { >> + p.get()->call(); /* line 29 */ >> +} >> +int main() { >> + intermediate(ptr{new MyClass}); >> +} >> diff --git a/gdb/testsuite/gdb.cp/empty-inline.exp b/gdb/testsuite/gdb.cp/empty-inline.exp >> new file mode 100644 >> index 0000000..98f4058 >> --- /dev/null >> +++ b/gdb/testsuite/gdb.cp/empty-inline.exp >> @@ -0,0 +1,55 @@ >> +# Copyright 2021 Free Software Foundation, Inc. >> + >> +# This program is free software; you can redistribute it and/or modify >> +# it under the terms of the GNU General Public License as published by >> +# the Free Software Foundation; either version 3 of the License, or >> +# (at your option) any later version. >> +# >> +# This program is distributed in the hope that it will be useful, >> +# but WITHOUT ANY WARRANTY; without even the implied warranty of >> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> +# GNU General Public License for more details. >> +# >> +# You should have received a copy of the GNU General Public License >> +# along with this program. If not, see <http://www.gnu.org/licenses/>. >> + >> +# PR 25987 >> +standard_testfile .cc >> + >> +if {[skip_cplus_tests]} { >> + return -1 >> +} >> + >> +if {![supports_statement_frontiers]} { >> + return -1 >> +} >> + >> +set options {c++ debug nowarnings optimize=-Og} >> +lappend options additional_flags=-gstatement-frontiers >> +if { [prepare_for_testing "failed to prepare" $testfile \ >> + $srcfile $options] } { >> + return -1 >> +} >> + >> +if ![runto_main] { >> + fail "can't run to main" >> + return >> +} >> + >> +gdb_test "bt" "\\s*\\#0\\s+main.*" "in main" >> +#break at the empty inline function ptr::get >> +gdb_test "b get" ".*" "break at get" >> +gdb_test "c" ".*" "continue to get" >> +#call frame 1 is at line 29 >> +gdb_test "bt" [multi_line "\\s*\\#0\\s+ptr::get\[^\r\]*${srcfile}:21" \ >> + "\\s*\\#1\\s+intermediate\[^\r\]*${srcfile}:29" \ >> + ".*"] \ >> + "at get" >> +#print a local value here >> +gdb_test "p t" ".*\\\$1 = \\(MyClass \\*\\) 0x.*" "print t" >> +gdb_test "c" ".*SIGSEGV.*" "continue to SIGSEGV" >> +#call frame 1 is at line 29 >> +gdb_test "bt" [multi_line "\\s*\\#0\\s+MyClass::call\[^\r\]*${srcfile}:26" \ >> + "\\s*\\#1\\s+0x\[^\r\]*${srcfile}:29" \ >> + ".*"] \ >> + "at call" >> diff --git a/gdb/testsuite/gdb.cp/step-and-next-inline.cc b/gdb/testsuite/gdb.cp/step-and-next-inline.cc >> index 26b29d0..d64d155 100644 >> --- a/gdb/testsuite/gdb.cp/step-and-next-inline.cc >> +++ b/gdb/testsuite/gdb.cp/step-and-next-inline.cc >> @@ -46,6 +46,9 @@ tree_check (tree *t, int i) >> #endif /* USE_NEXT_INLINE_H */ >> >> int __attribute__((noinline, noclone)) >> +#ifdef __CET__ >> + __attribute__((nocf_check)) >> +#endif >> get_alias_set (tree *t) >> { >> if (t != NULL >> @@ -59,6 +62,9 @@ get_alias_set (tree *t) >> tree xx; >> >> int >> +#ifdef __CET__ >> + __attribute__((nocf_check)) >> +#endif >> main() >> { >> get_alias_set (&xx); >> diff --git a/gdb/testsuite/gdb.cp/step-and-next-inline.exp b/gdb/testsuite/gdb.cp/step-and-next-inline.exp >> index 13d801b..5af1e38 100644 >> --- a/gdb/testsuite/gdb.cp/step-and-next-inline.exp >> +++ b/gdb/testsuite/gdb.cp/step-and-next-inline.exp >> @@ -20,7 +20,7 @@ if [get_compiler_info "c++"] { >> return -1 >> } >> >> -if {[test_compiler_info gcc*] && ![supports_statement_frontiers] } { >> +if { [test_compiler_info gcc*] && ![supports_statement_frontiers] } { >> return -1 >> } >> >> @@ -29,13 +29,6 @@ if {[test_compiler_info gcc*] && ![supports_statement_frontiers] } { >> proc do_test { use_header } { >> global srcfile testfile >> >> - if { $use_header } { >> - # This test will not pass due to poor debug information >> - # generated by GCC (at least upto 10.x). See >> - # https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94474 >> - return >> - } >> - >> set options {c++ debug nowarnings optimize=-O2} >> if { [supports_statement_frontiers] } { >> lappend options additional_flags=-gstatement-frontiers >> @@ -67,109 +60,21 @@ 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. >> - set have_kfail [expr [test_compiler_info gcc*] && !$use_header] >> - >> - set ok 1 >> - gdb_test_multiple "next" "next step 1" { >> - -re -wrap "if \\(t->x != i\\)" { >> - set ok 0 >> - send_gdb "next\n" >> - exp_continue >> - } >> - -re -wrap ".*TREE_TYPE.* != 1" { >> - if { $ok } { >> - pass $gdb_test_name >> - } else { >> - if { $have_kfail } { >> - setup_kfail "*-*-*" symtab/25507 >> - } >> - fail $gdb_test_name >> - } >> - } >> - } >> + gdb_test "next" ".*TREE_TYPE.*" "next step 1" >> gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \ >> "not in inline 2" >> - >> - set ok 1 >> - gdb_test_multiple "next" "next step 2" { >> - -re -wrap "return x;" { >> - set ok 0 >> - send_gdb "next\n" >> - exp_continue >> - } >> - -re -wrap ".*TREE_TYPE.* != 2" { >> - if { $ok } { >> - pass $gdb_test_name >> - } else { >> - if { $have_kfail } { >> - setup_kfail "*-*-*" symtab/25507 >> - } >> - fail $gdb_test_name >> - } >> - } >> - } >> + gdb_test "next" ".*TREE_TYPE.*" "next step 2" >> gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \ >> "not in inline 3" >> - >> - set ok 1 >> - gdb_test_multiple "next" "next step 3" { >> - -re -wrap "return x;" { >> - set ok 0 >> - send_gdb "next\n" >> - exp_continue >> - } >> - -re -wrap ".*TREE_TYPE.* != 3\\)" { >> - if { $ok } { >> - pass $gdb_test_name >> - } else { >> - if { $have_kfail } { >> - setup_kfail "*-*-*" symtab/25507 >> - } >> - fail $gdb_test_name >> - } >> - } >> - } >> + gdb_test "next" ".*TREE_TYPE.*" "next step 3" >> gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \ >> "not in inline 4" >> - >> - set ok 1 >> - gdb_test_multiple "next" "next step 4" { >> - -re -wrap "(if \\(t != NULL|\} // get_alias_set)" { >> - send_gdb "next\n" >> - exp_continue >> - } >> - -re -wrap "return x;" { >> - set ok 0 >> - send_gdb "next\n" >> - exp_continue >> - } >> - -re -wrap "return 0.*" { >> - if { $ok } { >> - pass $gdb_test_name >> - } else { >> - if { $have_kfail } { >> - setup_kfail "*-*-*" symtab/25507 >> - } >> - fail $gdb_test_name >> - } >> - } >> - } >> + gdb_test "next" ".*" "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" >> + if ![test_compiler_info gcc*] { >> return >> } >> >> @@ -190,22 +95,79 @@ proc do_test { use_header } { >> gdb_test "step" ".*if \\(t->x != i\\).*" "step 2" >> gdb_test "bt" "\\s*\\#0\\s+\[^\r\]*tree_check\[^\r\]*${hdrfile}:.*" \ >> "in inline 1 pass 2" >> - gdb_test "step" ".*TREE_TYPE.*" "step 3" >> + gdb_test "step" ".*return x.*" "step 3" >> + gdb_test "bt" "\\s*\\#0\\s+\[^\r\]*tree_check\[^\r\]*${hdrfile}:.*" \ >> + "return from inline 1 pass 2" >> + gdb_test "step" ".*TREE_TYPE.*" "step 4" >> gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \ >> "not in inline 2 pass 2" >> - gdb_test "step" ".*if \\(t->x != i\\).*" "step 4" >> + gdb_test "step" ".*if \\(t->x != i\\).*" "step 5" >> gdb_test "bt" "\\s*\\#0\\s+\[^\r\]*tree_check\[^\r\]*${hdrfile}:.*" \ >> "in inline 2 pass 2" >> - gdb_test "step" ".*TREE_TYPE.*" "step 5" >> + gdb_test "step" ".*return x.*" "step 6" >> + gdb_test "bt" "\\s*\\#0\\s+\[^\r\]*tree_check\[^\r\]*${hdrfile}:.*" \ >> + "return from inline 2 pass 2" >> + gdb_test "step" ".*TREE_TYPE.*" "step 7" >> gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \ >> "not in inline 3 pass 2" >> - gdb_test "step" ".*if \\(t->x != i\\).*" "step 6" >> + gdb_test "step" ".*if \\(t->x != i\\).*" "step 8" >> gdb_test "bt" "\\s*\\#0\\s+\[^\r\]*tree_check\[^\r\]*${hdrfile}:.*" \ >> "in inline 3 pass 2" >> - gdb_test "step" "return 0.*" "step 7" >> + gdb_test "step" ".*return x.*" "step 9" >> + gdb_test "bt" "\\s*\\#0\\s+\[^\r\]*tree_check\[^\r\]*${hdrfile}:.*" \ >> + "return from inline 3 pass 2" >> + gdb_test "step" "return 0.*" "step 10" >> gdb_test "bt" \ >> "\\s*\\#0\\s+(main|get_alias_set)\[^\r\]*${srcfile}:.*" \ >> "not in inline 4 pass 2" >> + >> + clean_restart ${executable} >> + >> + if ![runto_main] { >> + fail "can't run to main pass 3" >> + return >> + } >> + >> + gdb_test "bt" "\\s*\\#0\\s+main.*" "in main pass 3" >> + gdb_test "step" ".*" "step into get_alias_set pass 3" >> + gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \ >> + "in get_alias_set pass 3" >> + gdb_test "step" ".*TREE_TYPE.*" "step 1 pass 3" >> + gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \ >> + "not in inline 1 pass 3" >> + gdb_test "step" ".*if \\(t->x != i\\).*" "step 2 pass 3" >> + gdb_test "bt" "\\s*\\#0\\s+\[^\r\]*tree_check\[^\r\]*${hdrfile}:.*" \ >> + "in inline 1 pass 3" >> + gdb_test "p t->x = 2" ".* = 2.*" "change value pass 3" >> + gdb_test "step" ".*abort.*" "step 4, pass 3" >> + gdb_test "bt" "\\s*\\#0\\s+\[^\r\]*tree_check\[^\r\]*${hdrfile}:.*" \ >> + "abort from inline 1 pass 3" >> + >> + clean_restart ${executable} >> + >> + if ![runto_main] { >> + fail "can't run to main pass 4" >> + return >> + } >> + >> + gdb_test "bt" "\\s*\\#0\\s+main.*" "in main pass 4" >> + gdb_test "skip tree_check" ".*" "skip tree_check pass 4" >> + gdb_test "step" ".*" "step into get_alias_set pass 4" >> + gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \ >> + "in get_alias_set pass 4" >> + gdb_test "step" ".*TREE_TYPE.*" "step 1 pass 4" >> + gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \ >> + "not in inline 1 pass 4" >> + gdb_test "step" ".*TREE_TYPE.*" "step 2 pass 4" >> + gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \ >> + "not in inline 2 pass 4" >> + gdb_test "step" ".*TREE_TYPE.*" "step 3 pass 4" >> + gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \ >> + "not in inline 3 pass 4" >> + gdb_test "step" "return 0.*" "step 4 pass 4" >> + gdb_test "bt" \ >> + "\\s*\\#0\\s+(main|get_alias_set)\[^\r\]*${srcfile}:.*" \ >> + "not in inline 4 pass 4" >> } >> } >> >> -- >> 1.9.1 >> > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-09-12 7:47 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-09-05 19:15 [PATCH v3 2/3] Fix range end handling of inlined subroutines Bernd Edlinger 2021-09-08 20:53 ` Bruno Larsen 2021-09-09 10:42 ` Bernd Edlinger 2021-09-10 9:11 ` Andrew Burgess 2021-09-12 7:47 ` Bernd Edlinger
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).