From patchwork Thu Feb 1 15:21:16 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Toby Lloyd Davies X-Patchwork-Id: 85138 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id ADA0C3858029 for ; Thu, 1 Feb 2024 15:24:51 +0000 (GMT) X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from mail-lf1-x12d.google.com (mail-lf1-x12d.google.com [IPv6:2a00:1450:4864:20::12d]) by sourceware.org (Postfix) with ESMTPS id B811F3858C3A for ; Thu, 1 Feb 2024 15:23:26 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org B811F3858C3A Authentication-Results: sourceware.org; dmarc=pass (p=quarantine dis=none) header.from=undo.io Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=undo.io ARC-Filter: OpenARC Filter v1.0.0 sourceware.org B811F3858C3A Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2a00:1450:4864:20::12d ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1706801009; cv=none; b=KeegM/GSV4hdOlAE5L/+IFx9ojtOyxi4m2Is5tPVwDf5bYZdCFt5G+zrKsetWi+6mjMT6xtB+x+4o4nkO3gChRe1eBx/Q+y8vk4NjWVuaQ3jIT+NkZNpiQ3Y78B9tRl7XJJX5DVH/vGZMgxyOdyjZl5QEpOxX2yBHiUirYu+f2Y= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1706801009; c=relaxed/simple; bh=eW6FJ18vEQcnBewpxlLHTRc5+v8hpM8BAM9TA5rx0NY=; h=DKIM-Signature:From:To:Subject:Date:Message-Id:MIME-Version; b=n/CdvLFWcVFNwqRYXIav3QLpD5nA/K2HN5wD9VOvNlnh3HcoiG7C6bq8+4JzuhE+dAPHINEkRM2KJsKrzEVR7ssb+0yHUA46NSkn9NZQmbMr3fRYRjIN33BX2RXFlEcAyK72ehfaDDG5H1ZBsea+0jNfMIjW2/KJT4yvO9kLtlk= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-lf1-x12d.google.com with SMTP id 2adb3069b0e04-51109060d6aso1496846e87.2 for ; Thu, 01 Feb 2024 07:23:26 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=undo.io; s=google2; t=1706801005; x=1707405805; darn=sourceware.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=BiRjWJjJuCIpmeDlSC0xuaN18GS6GusednUg0c1XF84=; b=Hkz0oBDhG/yTjlRsbuvoAlW+bX4FCnUxjvqDJu9UFuzCSSJYkgJL+zfxKCSDLNA9oQ LudRvCpmLDf0x8gApf2SSV36TuidU7AsUbOHx8m0J4Tt+927Kv7uO2jBo7TCvZVJeApP D30PeiIA3Y/3gLnm3wSb6FJsjW9sNM200Ea7Dqbe7pnn2EgCHJYZQCg7xvxXTpniYAj6 iI7v3BPQ4lZFl4lFW3HPJz6FLvpONRKehjtDF33vzIx649JPyeWrE+80yEtcvH9gxq3S 6IIZGcRRiohTYyBunm5kPOHRqoDs6HJj8AuFajtTFKOtODxdKUpoaXDwv0cCW8Wh+EXa 7zyQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1706801005; x=1707405805; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=BiRjWJjJuCIpmeDlSC0xuaN18GS6GusednUg0c1XF84=; b=afg+NoEH0W0wFP7r5+uo5XQmB96zGIc0Vrn+t2RNr3YurKCQTLIJRQlTfH7f+XjjxQ OpECXSlL7LemFOGIDEbbUhiLc8maiko8kQz7rhzWQdMb/7Q11V84s2sZdV6+3HtvkqwD 63NKR5Os5N+3LQ0vaATxEbardIGq+ocvLaVj/ss1wozW//LIHmQO2Pa+gK6YLwYUlev6 Q5qwHuaymfexV1dDRmUBLJSvf0h/H0kzKjUhH45ERqunmEWxPz/l6QP/5q7nbapHOcfU sBAOw1PlWXFy/DXKgAirG/So4pd3txxXrWmimjqDZSiB9Ljsjefa9wFaiKz8X+t83Sl1 u1Kg== X-Gm-Message-State: AOJu0YzgwMADuUPzfvmv7GRTaT25bQnFjdmijDs16UH+m1NhdlC+o4Kp 6DQ7aRuPTPZzRGnBByu8/b0kuh5t1NCy6mAc7v0cZA9EcK5tFIDEc6yHYzkAdPHtJ1hcJL1aR1T IsLwq2YfVs765VRJO+70zLMekI6V93XlHyKnazjxBAfPkEisiNXQjg0+URsy70LZpbWxjz9Wa+4 rIzC4lF5s+zWsiILZLRCoRQ3GMGEpSN2HsEhM43RG0eBQ= X-Google-Smtp-Source: AGHT+IHM6j4FTe6jm97fDZncFOYeWdBmQ3H4jt8GCOTvJr08Px4w/unuq7pX6v1AdmAV/91mrpLm4Q== X-Received: by 2002:ac2:4dae:0:b0:50e:69df:b067 with SMTP id h14-20020ac24dae000000b0050e69dfb067mr1946429lfe.11.1706801004584; Thu, 01 Feb 2024 07:23:24 -0800 (PST) Received: from redhawk-thinkpad.home (cfbb000029.b.cam.camfibre.uk. [185.219.110.154]) by smtp.gmail.com with ESMTPSA id r2-20020a05600c320200b0040ef04987e7sm4598093wmp.16.2024.02.01.07.23.23 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 01 Feb 2024 07:23:23 -0800 (PST) From: Toby Lloyd Davies To: gdb-patches@sourceware.org Cc: Toby Lloyd Davies Subject: [PATCH 1/3] gdb: Fix missed inline callsite when the frame has changed Date: Thu, 1 Feb 2024 15:21:16 +0000 Message-Id: <20240201152118.598375-2-tlloyddavies@undo.io> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20240201152118.598375-1-tlloyddavies@undo.io> References: <20240201152118.598375-1-tlloyddavies@undo.io> MIME-Version: 1.0 X-Spam-Status: No, score=-11.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org When the program has just returned from a function, gdb could incorrectly step over an inline callsite. This would occur when the line from the innermost inline function did not have is_stmt set. e.g. with the following assembly: foo (); call foo bar (); global_var++; /* start of bar. is_stmt false */ When stepping out of foo, gdb will not stop the program at the callsite of bar. This problem was occurring because the first part of logic that checked for "calls" to inlined functions was guarded by a check that the we were still stepping through the same function. This meant that it wasn't used when the frame had changed due to returning from a function. In this case we would incorrectly use the symtab_and_line from the innermost inline function which would mean we wouldn't stop if this line didn't have is_stmt set. This guard was necessary with the logic for detecting if we had stepped into another inline function being done later than the check for inline callsites. This is because otherwise we could decide to stop at an inline callsite before we detect that we have also stepped into a different inlined function we should step over. So fix the bug by swapping the order of the checks. --- gdb/infrun.c | 50 +++---- .../gdb.dwarf2/dw2-inline-stepping-2.c | 49 +++++++ .../gdb.dwarf2/dw2-inline-stepping-2.exp | 127 ++++++++++++++++++ 3 files changed, 202 insertions(+), 24 deletions(-) create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-2.c create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-2.exp diff --git a/gdb/infrun.c b/gdb/infrun.c index e99a1a83070..a782c115cad 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -8090,12 +8090,31 @@ process_event_stop_test (struct execution_control_state *ecs) return; } - /* Look for "calls" to inlined functions, part one. If the inline + /* Look for "calls" to inlined functions, part one. If we are still + in the same real function we were stepping through, but we have + to go further up to find the exact frame ID, we are stepping + through a more inlined call beyond its call site. */ + + if (get_frame_type (get_current_frame ()) == INLINE_FRAME + && (*curr_frame_id != original_frame_id) + && stepped_in_from (get_current_frame (), + ecs->event_thread->control.step_frame_id)) + { + infrun_debug_printf ("stepping through inlined function"); + + if (ecs->event_thread->control.step_over_calls == STEP_OVER_ALL + || inline_frame_is_marked_for_skip (false, ecs->event_thread)) + keep_going (ecs); + else + end_stepping_range (ecs); + return; + } + + /* Look for "calls" to inlined functions, part two. If the inline frame machinery detected some skipped call sites, we have entered a new inline function. */ - if ((*curr_frame_id == original_frame_id) - && inline_skipped_frames (ecs->event_thread)) + if (inline_skipped_frames (ecs->event_thread)) { infrun_debug_printf ("stepped into inlined function"); @@ -8108,7 +8127,8 @@ process_event_stop_test (struct execution_control_state *ecs) we were previously stepping, go down into the function first. Otherwise stop at the call site. */ - if (call_sal.line == ecs->event_thread->current_line + if (*curr_frame_id == original_frame_id + && call_sal.line == ecs->event_thread->current_line && call_sal.symtab == ecs->event_thread->current_symtab) { step_into_inline_frame (ecs->event_thread); @@ -8127,7 +8147,8 @@ process_event_stop_test (struct execution_control_state *ecs) /* For "next", we should stop at the call site if it is on a different source line. Otherwise continue through the inlined function. */ - if (call_sal.line == ecs->event_thread->current_line + if (*curr_frame_id == original_frame_id + && call_sal.line == ecs->event_thread->current_line && call_sal.symtab == ecs->event_thread->current_symtab) keep_going (ecs); else @@ -8136,25 +8157,6 @@ process_event_stop_test (struct execution_control_state *ecs) } } - /* Look for "calls" to inlined functions, part two. If we are still - in the same real function we were stepping through, but we have - to go further up to find the exact frame ID, we are stepping - through a more inlined call beyond its call site. */ - - if (get_frame_type (get_current_frame ()) == INLINE_FRAME - && (*curr_frame_id != original_frame_id) - && stepped_in_from (get_current_frame (), original_frame_id)) - { - infrun_debug_printf ("stepping through inlined function"); - - if (ecs->event_thread->control.step_over_calls == STEP_OVER_ALL - || inline_frame_is_marked_for_skip (false, ecs->event_thread)) - keep_going (ecs); - else - end_stepping_range (ecs); - return; - } - bool refresh_step_info = true; if ((ecs->event_thread->stop_pc () == stop_pc_sal.pc) && (ecs->event_thread->current_line != stop_pc_sal.line diff --git a/gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-2.c b/gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-2.c new file mode 100644 index 00000000000..9c2b3f95a26 --- /dev/null +++ b/gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-2.c @@ -0,0 +1,49 @@ +/* Copyright 2019-2024 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 . */ + +/* This test relies on bar being inlined into main and foo not being + inlined. */ + +volatile int global_var; + +static inline int __attribute__ ((always_inline)) +bar () +{ + asm ("bar_label: .globl bar_label"); + global_var++; /* bar inc global_var*/ + asm ("bar_label2: .globl bar_label2"); + return global_var; /* bar return global_var */ +} /* bar end */ + +void +foo (void) +{ /* foo prologue */ + asm ("foo_label: .globl foo_label"); + global_var++; /* foo inc global_var */ +} /* foo end */ + +int +main () +{ /* main prologue */ + int ans; + asm ("main_label: .globl main_label"); + global_var = 0; /* main set global_var */ + asm ("main_label2: .globl main_label2"); + foo (); /* main call foo */ + asm ("main_label3: .globl main_label3"); + ans = bar (); /* main call bar */ + asm ("main_label4: .globl main_label4"); + return ans; +} /* main end */ diff --git a/gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-2.exp b/gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-2.exp new file mode 100644 index 00000000000..a97a1bfc44b --- /dev/null +++ b/gdb/testsuite/gdb.dwarf2/dw2-inline-stepping-2.exp @@ -0,0 +1,127 @@ +# Copyright 2019-2024 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 . +# +# This test checks that we stop at an inline callsite when returning +# from a non-inlined function. Specifically when the first line of the +# inline function does not have is_stmt set. + +load_lib dwarf.exp + +# This test can only be run on targets which support DWARF-2 and use gas. +require dwarf2_support + +# The .c files use __attribute__. +require is_c_compiler_gcc + +standard_testfile .c .S + +set asm_file [standard_output_file $srcfile2] +Dwarf::assemble $asm_file { + global srcdir subdir srcfile srcfile2 + declare_labels ranges_label lines_label bar_prog + + lassign [function_range main [list ${srcdir}/${subdir}/$srcfile]] \ + main_start main_len + set main_end "$main_start + $main_len" + lassign [function_range foo [list ${srcdir}/${subdir}/$srcfile]] \ + foo_start foo_len + set foo_end "$foo_start + $foo_len" + + set bar_call_line [gdb_get_line_number "main call bar"] + + cu {} { + compile_unit { + {language @DW_LANG_C} + {name dw2-inline-stepping-2.c} + {low_pc 0 addr} + {stmt_list ${lines_label} DW_FORM_sec_offset} + {ranges ${ranges_label} DW_FORM_sec_offset} + } { + subprogram { + {external 1 flag} + {MACRO_AT_func {foo}} + } + bar_prog: subprogram { + {name bar} + {inline 3 data1} + } + subprogram { + {external 1 flag} + {name main} + {low_pc $main_start addr} + {high_pc "$main_start + $main_len" addr} + } { + inlined_subroutine { + {abstract_origin %$bar_prog} + {low_pc main_label3 addr} + {high_pc main_label4 addr} + {call_file 1 data1} + {call_line $bar_call_line data1} + } + } + } + } + + lines {version 2} lines_label { + include_dir "${srcdir}/${subdir}" + file_name "$srcfile" 1 + + program { + DW_LNE_set_address foo_label + line [gdb_get_line_number "foo inc global_var"] + DW_LNS_copy + DW_LNE_set_address $foo_end + DW_LNE_end_sequence + + DW_LNE_set_address main_label3 + line [gdb_get_line_number "bar inc global_var"] + DW_LNS_negate_stmt + DW_LNS_copy + + DW_LNS_negate_stmt + DW_LNE_set_address bar_label2 + line [gdb_get_line_number "bar return global_var"] + DW_LNS_copy + + DW_LNE_set_address main_label4 + line [gdb_get_line_number "return ans"] + DW_LNS_copy + DW_LNE_set_address $main_end + DW_LNE_end_sequence + } + } + + ranges {is_64 [is_64_target]} { + ranges_label: sequence { + range ${main_start} ${main_end} + range ${foo_start} ${foo_end} + } + } +} + +if { [prepare_for_testing "failed to prepare" ${testfile} \ + [list $srcfile $asm_file] {nodebug}] } { + return -1 +} + +if ![runto_main] { + return -1 +} + +gdb_breakpoint "foo" +gdb_continue_to_breakpoint "foo" + +# We should stop at bar callsite when returning from foo. +gdb_test "step" ".*main call bar.*" "step to bar callsite"