From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 1879) id ED7373858439; Thu, 10 Nov 2022 16:34:05 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org ED7373858439 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1668098045; bh=BUn8gN/BWgeg+MYvfUVKeItXTBYYj+SEwAfL6F1EodM=; h=From:To:Subject:Date:From; b=U7j169HMsqnCYA4ZqLeF9UTFB0Tk7b774n94BFwCdmrzq40qNnJoE78rWw+d2DrA6 SrjY4EgL6THcj3/wqj5CTbKzRCQmcNyLIqz5ptHbDucLyrl8dktrgsvdQ234qiDXpg GSr2wZoM4YGbn1bgstU/Pu59NowGJ/3x8R1IVYFg= Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Simon Marchi To: gdb-cvs@sourceware.org Subject: [binutils-gdb] gdb: remove manual frame_info reinflation code in backtrace_command_1 X-Act-Checkin: binutils-gdb X-Git-Author: Simon Marchi X-Git-Refname: refs/heads/master X-Git-Oldrev: 04e2ac7b2a7c5fbc0afcf151aeb8a26415ad0fac X-Git-Newrev: 73cafdbd1ddd6ebfa21d737e3b69ae5aafd70c23 Message-Id: <20221110163405.ED7373858439@sourceware.org> Date: Thu, 10 Nov 2022 16:34:05 +0000 (GMT) List-Id: https://sourceware.org/git/gitweb.cgi?p=3Dbinutils-gdb.git;h=3D73cafdbd1ddd= 6ebfa21d737e3b69ae5aafd70c23 commit 73cafdbd1ddd6ebfa21d737e3b69ae5aafd70c23 Author: Simon Marchi Date: Mon Oct 24 15:57:26 2022 -0400 gdb: remove manual frame_info reinflation code in backtrace_command_1 =20 With the following patch applied (gdb: use frame_id_p instead of comparing to null_frame_id in frame_info_ptr::reinflate), I would get: =20 $ ./gdb -q -nx --data-directory=3Ddata-directory testsuite/outputs/= gdb.base/bt-selected-frame/bt-selected-frame -ex "b breakpt" -ex r -ex "bt = full" Reading symbols from testsuite/outputs/gdb.base/bt-selected-frame/b= t-selected-frame... Breakpoint 1 at 0x1131: file /home/smarchi/src/binutils-gdb/gdb/tes= tsuite/gdb.base/bt-selected-frame.c, line 22. Starting program: /home/smarchi/build/binutils-gdb/gdb/testsuite/ou= tputs/gdb.base/bt-selected-frame/bt-selected-frame [Thread debugging using libthread_db enabled] Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db= .so.1". =20 Breakpoint 1, breakpt () at /home/smarchi/src/binutils-gdb/gdb/test= suite/gdb.base/bt-selected-frame.c:22 22 } #0 breakpt () at /home/smarchi/src/binutils-gdb/gdb/testsuite/gdb.= base/bt-selected-frame.c:22 No locals. /home/smarchi/src/binutils-gdb/gdb/frame-info.c:42: internal-error:= reinflate: Assertion `frame_id_p (m_cached_id)' failed. =20 This is because the code in backtrace_command_1 to manually reinflate `fi` steps overs frame_info_ptr's toes. =20 When calling =20 fi.prepare_reinflate (); =20 `fi` gets properly filled with the cached frame id. But when this happens: =20 fi =3D frame_find_by_id (frame_id); =20 `fi` gets replaced by a brand new frame_info_ptr that doesn't have a cached frame id. Then this is called without a cached frame id: =20 fi.reinflate (); =20 That doesn't cause any problem currently, since =20 - the gdb_assert in the reinflate method doesn't actually do anything (the following patch fixes that) - `fi.m_ptr` will always be non-nullptr, since we just got it from frame_find_by_id, so reinflate will not do anything, it won't try to use m_cached_id =20 Fix that by removing the code to manually re-fetch the frame. That should be taken care of by frame_info_ptr::reinflate. =20 Note that the old code checked if we successfully re-inflated the frame or not, and if not it did emit a warning. The equivalent in frame_info_ptr::reinflate asserts that the frame has been successfully re-inflated. It's not clear if / when this can happen, but if it can happen, we'll need to find a solution to this problem globally (everywhere a frame_info_ptr can be re-inflated), not just here. So I propose to leave it like this, until it does become a problem. =20 Reviewed-By: Bruno Larsen Change-Id: I07b783d94e2853e0a2d058fe7deaf04eddf24835 Diff: --- gdb/stack.c | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/gdb/stack.c b/gdb/stack.c index 4e2342c2a8d..5f29566fcfe 100644 --- a/gdb/stack.c +++ b/gdb/stack.c @@ -2082,20 +2082,7 @@ backtrace_command_1 (const frame_print_options &fp_o= pts, =20 print_frame_info (fp_opts, fi, 1, LOCATION, 1, 0); if ((flags & PRINT_LOCALS) !=3D 0) - { - struct frame_id frame_id =3D get_frame_id (fi); - - print_frame_local_vars (fi, false, NULL, NULL, 1, gdb_stdout); - - /* print_frame_local_vars invalidates FI. */ - fi =3D frame_find_by_id (frame_id); - if (fi =3D=3D NULL) - { - trailing =3D NULL; - warning (_("Unable to restore previously selected frame.")); - break; - } - } + print_frame_local_vars (fi, false, NULL, NULL, 1, gdb_stdout); =20 /* Save the last frame to check for error conditions. */ fi.reinflate ();