From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 1043F38356B1 for ; Fri, 22 Jul 2022 15:32:01 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 1043F38356B1 Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-54-Nq-CN-ggOOiKlbnGGBGW5w-1; Fri, 22 Jul 2022 11:31:59 -0400 X-MC-Unique: Nq-CN-ggOOiKlbnGGBGW5w-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 3A23C2999B30; Fri, 22 Jul 2022 15:31:59 +0000 (UTC) Received: from [10.97.116.26] (ovpn-116-26.gru2.redhat.com [10.97.116.26]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 3AE9F1121314; Fri, 22 Jul 2022 15:31:58 +0000 (UTC) Message-ID: Date: Fri, 22 Jul 2022 12:31:56 -0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.11.0 Subject: Re: [PATCH] gdb/reverse: Fix stepping over recursive functions To: Pedro Alves , gdb-patches@sourceware.org References: <20220525180247.29731-1-blarsen@redhat.com> <4883e160-bb4a-6578-1bfd-308646144aef@palves.net> From: Bruno Larsen In-Reply-To: <4883e160-bb4a-6578-1bfd-308646144aef@palves.net> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.3 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-13.1 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_NONE, TXREP 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.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 22 Jul 2022 15:32:02 -0000 On 7/7/22 15:48, Pedro Alves wrote: > I also don't understand why do we need all these different frame checks as above. Breakpoints > can have a frame associated, so that when a breakpoint hits, the breakpoint.c stuff checks > whether the hit happened in the right frame, and if not, the breakpoint is ignored / set to > be auto-stepped-over, and BPSTAT_WHAT_STEP_RESUME isn't reached. > > The actual problem seems to me that we create the step-resume breakpoint in > question without giving it a frame. If I do that instead of your fix, like so: > > diff --git c/gdb/infrun.c w/gdb/infrun.c > index 02c98b50c8c..1360b8da70e 100644 > --- c/gdb/infrun.c > +++ w/gdb/infrun.c > @@ -7118,8 +7118,8 @@ process_event_stop_test (struct execution_control_state *ecs) > symtab_and_line sr_sal; > sr_sal.pc = ecs->stop_func_start; > sr_sal.pspace = get_frame_program_space (frame); > - insert_step_resume_breakpoint_at_sal (gdbarch, > - sr_sal, null_frame_id); > + insert_step_resume_breakpoint_at_sal > + (gdbarch, sr_sal, get_stack_frame_id (frame)); > } > } > else > > and then run your updated testcase, it passes. I also see no regressions in gdb.reverse/ tests. > > WDYT? Are you seeing something that would break with this approach? Hi Pedro I have spent a while attempting to use your suggestion, but I did find something that breaks with this patch. gdb.reverse/solib-precsave.exp shows 10 new failures. Here's a snippet before your change: record restore /home/blarsen/git-pool/git-repos/gdb-local-build/gdb/testsuite/outputs/gdb.reverse/solib-precsave/solib.precsave Warning: 'record restore', an alias for the command 'record full restore', is deprecated. Use 'record full restore'. [New LWP 22073] Core was generated by `/home/blarsen/git-pool/git-repos/gdb-local-build/gdb/testsuite/outputs/gdb.rever'. Program terminated with signal SIGTRAP, Trace/breakpoint trap. #0 main () at /home/blarsen/git-pool/git-repos/gdb-local-build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.reverse/solib-reverse.c:27 27 char* cptr = "String 1"; Restored records from core file /home/blarsen/git-pool/git-repos/gdb-local-build/gdb/testsuite/outputs/gdb.reverse/solib-precsave/solib.precsave. #0 main () at /home/blarsen/git-pool/git-repos/gdb-local-build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.reverse/solib-reverse.c:27 27 char* cptr = "String 1"; (gdb) PASS: gdb.reverse/solib-precsave.exp: reload core file until 46 main () at /home/blarsen/git-pool/git-repos/gdb-local-build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.reverse/solib-reverse.c:46 46 return 0; /* end part one */ (gdb) PASS: gdb.reverse/solib-precsave.exp: run until end part one reverse-step 44 shr1 ("message 3\n"); /* shr1 three */ (gdb) PASS: gdb.reverse/solib-precsave.exp: reverse-step third shr1 reverse-step 43 shr1 ("message 2\n"); /* shr1 two */ (gdb) PASS: gdb.reverse/solib-precsave.exp: reverse-step second shr1 reverse-step 42 shr1 ("message 1\n"); /* shr1 one */ (gdb) PASS: gdb.reverse/solib-precsave.exp: reverse-step first shr1 reverse-step 40 b[0] = 6; b[1] = 9; /* generic statement, end part two */ (gdb) PASS: gdb.reverse/solib-precsave.exp: reverse-step generic until 46 main () at /home/blarsen/git-pool/git-repos/gdb-local-build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.reverse/solib-reverse.c:46 46 return 0; /* end part one */ (gdb) PASS: gdb.reverse/solib-precsave.exp: forward to end part one reverse-next 44 shr1 ("message 3\n"); /* shr1 three */ (gdb) PASS: gdb.reverse/solib-precsave.exp: reverse-next third shr1 and after your change: record restore /home/blarsen/git-pool/git-repos/gdb-local-build/gdb/testsuite/outputs/gdb.reverse/solib-precsave/solib.precsave Warning: 'record restore', an alias for the command 'record full restore', is deprecated. Use 'record full restore'. [New LWP 21721] Core was generated by `/home/blarsen/git-pool/git-repos/gdb-local-build/gdb/testsuite/outputs/gdb.rever'. Program terminated with signal SIGTRAP, Trace/breakpoint trap. #0 main () at /home/blarsen/git-pool/git-repos/gdb-local-build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.reverse/solib-reverse.c:27 27 char* cptr = "String 1"; Restored records from core file /home/blarsen/git-pool/git-repos/gdb-local-build/gdb/testsuite/outputs/gdb.reverse/solib-precsave/solib.precsave. #0 main () at /home/blarsen/git-pool/git-repos/gdb-local-build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.reverse/solib-reverse.c:27 27 char* cptr = "String 1"; (gdb) PASS: gdb.reverse/solib-precsave.exp: reload core file until 46 main () at /home/blarsen/git-pool/git-repos/gdb-local-build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.reverse/solib-reverse.c:46 46 return 0; /* end part one */ (gdb) PASS: gdb.reverse/solib-precsave.exp: run until end part one reverse-step 44 shr1 ("message 3\n"); /* shr1 three */ (gdb) PASS: gdb.reverse/solib-precsave.exp: reverse-step third shr1 reverse-step 43 shr1 ("message 2\n"); /* shr1 two */ (gdb) PASS: gdb.reverse/solib-precsave.exp: reverse-step second shr1 reverse-step 42 shr1 ("message 1\n"); /* shr1 one */ (gdb) PASS: gdb.reverse/solib-precsave.exp: reverse-step first shr1 reverse-step 40 b[0] = 6; b[1] = 9; /* generic statement, end part two */ (gdb) PASS: gdb.reverse/solib-precsave.exp: reverse-step generic until 46 main () at /home/blarsen/git-pool/git-repos/gdb-local-build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.reverse/solib-reverse.c:46 46 return 0; /* end part one */ (gdb) PASS: gdb.reverse/solib-precsave.exp: forward to end part one reverse-next No more reverse-execution history. main () at /home/blarsen/git-pool/git-repos/gdb-local-build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.reverse/solib-reverse.c:27 27 char* cptr = "String 1"; (gdb) FAIL: gdb.reverse/solib-precsave.exp: reverse-next third shr1 And a few more reverse-next fails after this. I haven't made much progress in debugging this issue because it seems that the whole record restore has somewhat bit-rotted, as trying to restore a recording from another GDB session results in internal errors, so I wonder if this test is still relevant, or if we can add these failures as "FIXME" since the whole feature is having issues. DO you have any thoughts on this? Cheers! Bruno Larsen