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 1164C3858C52 for ; Tue, 17 Jan 2023 14:30:07 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 1164C3858C52 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1673965806; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=fj/tdFNG1brCvnKMr3yKosdSNAUV/TWfE+CssWdS8YA=; b=Ri0APyWvwmjTqblVWfdekpimTKQvX4j6vIqC0N/ToQh/SGKKtIn1OSvGz8Eya7TG4FJtqF FqwD/lc14Yd+hcq+o+c07nHcKDmJjde+9J0zaxx7AwitbhpZbL0Q5JcWhxSn1yOkRTsHrD WIbhErfBdaKFgOKoekEyyXrovje81g8= Received: from mail-qt1-f197.google.com (mail-qt1-f197.google.com [209.85.160.197]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-142-mAX7jhhtPf-N2C6eehiVaA-1; Tue, 17 Jan 2023 09:29:54 -0500 X-MC-Unique: mAX7jhhtPf-N2C6eehiVaA-1 Received: by mail-qt1-f197.google.com with SMTP id a13-20020ac84d8d000000b003b63a85ae73so1180941qtw.21 for ; Tue, 17 Jan 2023 06:29:42 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:from:references:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=fj/tdFNG1brCvnKMr3yKosdSNAUV/TWfE+CssWdS8YA=; b=RgT/Y+cfNmHOw4ozMpGNMFG2JYdfhe9mqh8Yu2pcodVUF1hJON0YDBJrMfo3gFqRD/ JA7sMeA1HMXzbofEBKvitbK5rbvkNnL0iIhHKNpIwvQiP07IGe3KTtXZ7zkDonq/Lpw3 Ep6dT4uZhiz9U5VRPx3BokuWAEXEddAs6GKABzY+wpfCzkEu3bv6oS63Rg3BArIXAxnu l//lDpZOyhQJOATeBuW318TC30KyhgWwci4SiuJeEntZUdypBjeTksOW0+OBMnfTNM03 V24Chzt/p3rHXUhlP66IivtgzRE3Le2i+3iW8w48/PE0VB+8VoRSNu4BV7ATEMtZ7iPP 6Dkg== X-Gm-Message-State: AFqh2kp1EEvzohIPacJF0pmijxfrZrB1IGv7Ef3G1SJApwsfx62kr9hM YDo+3pF+LCMDylVM16BFQM56+daix0DaIq/l/5FKN4YItDo1cIHyiOd0aP55Pph9DQpdZ9ScI2f Hi1daKRb+vtJn1sKKhDYgTQ== X-Received: by 2002:ac8:68e:0:b0:3a7:ec99:56e4 with SMTP id f14-20020ac8068e000000b003a7ec9956e4mr39179648qth.39.1673965782131; Tue, 17 Jan 2023 06:29:42 -0800 (PST) X-Google-Smtp-Source: AMrXdXtRikoFTEOKDIwFbNsBLHTNzaSUu+VRc9dT9XDBokafN2kJt83Q8a29hLkrghYex7AsVpsenQ== X-Received: by 2002:ac8:68e:0:b0:3a7:ec99:56e4 with SMTP id f14-20020ac8068e000000b003a7ec9956e4mr39179611qth.39.1673965781715; Tue, 17 Jan 2023 06:29:41 -0800 (PST) Received: from [10.43.2.105] (nat-pool-brq-t.redhat.com. [213.175.37.10]) by smtp.gmail.com with ESMTPSA id f26-20020ac86eda000000b003b63c55c299sm1112153qtv.81.2023.01.17.06.29.40 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 17 Jan 2023 06:29:41 -0800 (PST) Message-ID: <78b464a1-e32e-c3da-85e4-7bfc322cc29f@redhat.com> Date: Tue, 17 Jan 2023 15:29:39 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.6.0 Subject: Re: [PATCH 2/2 version 2] fix for gdb.reverse/finish-precsave.exp and gdb.reverse/finish-reverse.exp To: Carl Love , Ulrich Weigand , "will_schmidt@vnet.ibm.com" , gdb-patches@sourceware.org References: <53878e37c6e57de1d04d9c9960c5d0a74324ee6e.camel@us.ibm.com> <50474aa92ba82eff05cdc8f49001eae56be29670.camel@us.ibm.com> <89331c26795e3f7743e1e068dce43b3c2dd53008.camel@us.ibm.com> <071f24ecf9b3a2bbbe8fee7db77492eb55c5f3ff.camel@us.ibm.com> <1d9b21914354bef6a290ac30673741e722e11757.camel@de.ibm.com> <3e3c9c40f07ab01c79fe10915e76ffa187c42ad9.camel@us.ibm.com> <122f5d2d3db9ef1979b0f8da927d005f32bba82c.camel@us.ibm.com> <011768e8-2b76-f8ed-1174-fbaa020b15e7@redhat.com> From: Bruno Larsen In-Reply-To: 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=-12.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,RCVD_IN_MSPIKE_H2,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 List-Id: On 16/01/2023 17:37, Carl Love wrote: > GDB maintainers: > > Version 2: Addressed various comments from Bruno Larsen. > > This patch fixes the issues with the reverse-finish command on > PowerPC. The reverse-finish command now correctly stops at the first > instruction in the source code line of the caller. > > The patch adds tests for calling a function via the GEP to the new test > gdb.reverse/finish-reverse-next.exp. > > Version 2 patch changes have been re-verified on PowerPC and X86 with > no regressions. > > Please let me know if you have any comments on the patch. Thanks. > > Carl > > --------------------------------------------------------------- > PowerPC: fix for gdb.reverse/finish-precsave.exp and gdb.reverse/finish-reverse.exp > > PR record/29927 - reverse-finish requires two reverse next instructions to > reach previous source line > > PowerPC uses two entry points called the local entry point (LEP) and the > global entry point (GEP). Normally the LEP is used when calling a > function. However, if the table of contents (TOC) value in register 2 is > not valid the GEP is called to setup the TOC before execution continues at > the LEP. When executing in reverse, the function finish_backward sets the > break point at the alternate entry point (GEP). However if the forward > execution enters via the normal entry point (LEP), the reverse execution > never sees the break point at the GEP of the function. Reverse execution > continues until the next break point is encountered or the end of the > recorded log is reached causing gdb to stop at the wrong place. > > This patch adds a new address to struct execution_control_state to hold the > address of the alternate function start address, known as the GEP on > PowerPC. The finish_backwards function is updated. If the stopping point > is between the two entry points (the LEP and GEP on PowerPC), the stepping > range is set to execute back to the alternate entry point (GEP on PowerPC). > Otherwise, a breakpoint is inserted at the normal entry point (LEP on > PowerPC). > > Function process_event_stop_test checks uses a stepping range to stop > execution in the caller at the first instruction of the source code line. > Note, on systems that only support one entry point, the address of the two > entry points are the same. > > Test finish-reverse-next.exp is updated to include tests for the > reverse-finish command when the function is entered via the normal entry > point (i.e. the LEP) and the alternate entry point (i.e. the GEP). > > The patch has been tested on X86 and PowerPC with no regressions. I will reiterate that I don't know much about PPC, so the best I can do is check for style and tests, but apart from a few minor nits inlined, it looks ok Tested-By: Bruno Larsen > --- > gdb/infcmd.c | 40 +++++--- > gdb/infrun.c | 16 +++- > .../gdb.reverse/finish-reverse-next.c | 41 +++++++- > .../gdb.reverse/finish-reverse-next.exp | 96 ++++++++++++++++--- > 4 files changed, 161 insertions(+), 32 deletions(-) > > diff --git a/gdb/infcmd.c b/gdb/infcmd.c > index 9c42efeae8d..6aaed34b1b6 100644 > --- a/gdb/infcmd.c > +++ b/gdb/infcmd.c > @@ -1722,22 +1722,25 @@ finish_backward (struct finish_command_fsm *sm) > sal = find_pc_line (func_addr, 0); > > frame_info_ptr frame = get_selected_frame (nullptr); > + struct gdbarch *gdbarch = get_frame_arch (frame); > + CORE_ADDR alt_entry_point = sal.pc; > + CORE_ADDR entry_point = alt_entry_point; > > - if (sal.pc != pc) > + if (gdbarch_skip_entrypoint_p (gdbarch)) > { > - struct gdbarch *gdbarch = get_frame_arch (frame); > - > - /* Set a step-resume at the function's entry point. Once that's > - hit, we'll do one more step backwards. */ > - symtab_and_line sr_sal; > - sr_sal.pc = sal.pc; > - sr_sal.pspace = get_frame_program_space (frame); > - insert_step_resume_breakpoint_at_sal (gdbarch, > - sr_sal, null_frame_id); > + /* Some architectures, like PowerPC use local and global entry points. > + There is only one Entry Point (GEP = LEP) for other architectures. > + The GEP is an alternate entry point. The LEP is the normal entry > + point. The value of entry_point was initialized to the alternate > + entry point (GEP). It will be adjusted if the normal entry point > + (LEP) was used. */ > + entry_point = gdbarch_skip_entrypoint (gdbarch, entry_point); > } > - else > + > + if (alt_entry_point <= pc && pc <= entry_point) > { > - /* We are exactly at the function entry point. Note that this > + /* We are exactly at the function entry point, or between the entry > + point on platforms that have two (like PowerPC). Note that this > can only happen at frame #0. > > When setting a step range, need to call set_step_info > @@ -1746,8 +1749,17 @@ finish_backward (struct finish_command_fsm *sm) > > /* Return using a step range so we will keep stepping back > to the first instruction in the source code line. */ > - tp->control.step_range_start = sal.pc; > - tp->control.step_range_end = sal.pc; > + tp->control.step_range_start = alt_entry_point; > + tp->control.step_range_end = entry_point; > + } > + else > + { > + symtab_and_line sr_sal; > + /* Set a step-resume at the function's entry point. */ > + sr_sal.pc = entry_point; > + sr_sal.pspace = get_frame_program_space (frame); > + insert_step_resume_breakpoint_at_sal (gdbarch, > + sr_sal, null_frame_id); > } > proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT); > } > diff --git a/gdb/infrun.c b/gdb/infrun.c > index 86e5ef1ed12..b69f84824a3 100644 > --- a/gdb/infrun.c > +++ b/gdb/infrun.c > @@ -1868,6 +1868,7 @@ struct execution_control_state > > struct target_waitstatus ws; > int stop_func_filled_in = 0; > + CORE_ADDR stop_func_alt_start = 0; > CORE_ADDR stop_func_start = 0; > CORE_ADDR stop_func_end = 0; > const char *stop_func_name = nullptr; > @@ -4663,6 +4664,12 @@ fill_in_stop_func (struct gdbarch *gdbarch, > &block); > ecs->stop_func_name = gsi == nullptr ? nullptr : gsi->print_name (); > > + /* PowerPC functions have a Local Entry Point and a Global Entry > + Point. There is only one Entry Point (GEP = LEP) for other > + architectures. Save the alternate entry point address (GEP) for > + use later. */ > + ecs->stop_func_alt_start = ecs->stop_func_start; > + > /* The call to find_pc_partial_function, above, will set > stop_func_start and stop_func_end to the start and end > of the range containing the stop pc. If this range > @@ -4679,6 +4686,9 @@ fill_in_stop_func (struct gdbarch *gdbarch, > += gdbarch_deprecated_function_start_offset (gdbarch); > > if (gdbarch_skip_entrypoint_p (gdbarch)) > + /* The PowerPC architecture uses two entry points. Stop at the > + regular entry point (LEP on PowerPC) initially. Will setup a > + breakpoint for the alternate entry point (GEP) later. */ > ecs->stop_func_start > = gdbarch_skip_entrypoint (gdbarch, ecs->stop_func_start); > } > @@ -6754,7 +6764,7 @@ process_event_stop_test (struct execution_control_state *ecs) > > /* Return using a step range so we will keep stepping back to the > first instruction in the source code line. */ > - tp->control.step_range_start = ecs->stop_func_start; > + tp->control.step_range_start = ecs->stop_func_alt_start; > tp->control.step_range_end = ecs->stop_func_start; > keep_going (ecs); > return; > @@ -6891,8 +6901,10 @@ process_event_stop_test (struct execution_control_state *ecs) > (unless it's the function entry point, in which case > keep going back to the call point). */ > CORE_ADDR stop_pc = ecs->event_thread->stop_pc (); > + > if (stop_pc == ecs->event_thread->control.step_range_start > - && stop_pc != ecs->stop_func_start > + && (stop_pc < ecs->stop_func_alt_start > + || stop_pc > ecs->stop_func_start) > && execution_direction == EXEC_REVERSE) > end_stepping_range (ecs); > else > diff --git a/gdb/testsuite/gdb.reverse/finish-reverse-next.c b/gdb/testsuite/gdb.reverse/finish-reverse-next.c > index f90ecbb93cb..6bac7c6117a 100644 > --- a/gdb/testsuite/gdb.reverse/finish-reverse-next.c > +++ b/gdb/testsuite/gdb.reverse/finish-reverse-next.c > @@ -1,4 +1,4 @@ > -/* This testcase is part of GDB, the GNU debugger. > +j/* This testcase is part of GDB, the GNU debugger. Accidental change that breaks the test here. > > Copyright 2012-2023 Free Software Foundation, Inc. > > @@ -24,11 +24,37 @@ > This test verifies the fix for gdb bugzilla: > > https://sourceware.org/bugzilla/show_bug.cgi?id=29927 > -*/ > + > + PowerPC supports two entry points to a function. The normal entry point > + is called the local entry point (LEP). The alternat entry point is called > + the global entry point (GEP). The GEP is only used if the table of > + contents (TOC) value stored in register r2 needs to be setup prior to > + execution starting at the LEP. A function call via a function pointer > + will entry via the GEP. A normal function call will enter via the LEP. > + > + This test has been expanded to include tests to verify the reverse-finish > + command works properly if the function is called via the GEP. The original > + test only verified the reverse-finish command for a normal call that used > + the LEP. */ > > int > function1 (int a, int b) // FUNCTION1 > { > + /* The assembly code for this function when compiled for PowerPC is as > + follows: > + > + 0000000010000758 : > + 10000758: 02 10 40 3c lis r2,4098 <- GEP > + 1000075c: 00 7f 42 38 addi r2,r2,32512 > + 10000760: a6 02 08 7c mflr r0 <- LEP > + 10000764: 10 00 01 f8 std r0,16(r1) > + .... > + > + When the function is called on PowerPC with function1 (a, b) the call > + enters at the Local Entry Point (LEP). When the function is called via > + a function pointer, the Global Entry Point (GEP) for function1 is used. > + The GEP sets up register 2 before reaching the LEP. > + */ > int ret = 0; > > ret = a + b; > @@ -39,10 +65,19 @@ int > main(int argc, char* argv[]) > { > int a, b; > + int (*funp) (int, int) = &function1; > + > + /* Call function via Local Entry Point (LEP). */ > > a = 1; > b = 5; > > - function1 (a, b); // CALL FUNCTION > + function1 (a, b); // CALL VIA LEP > + > + /* Call function via Global Entry Point (GEP). */ > + a = 10; > + b = 50; > + > + funp (a, b); // CALL VIA GEP > return 0; > } > diff --git a/gdb/testsuite/gdb.reverse/finish-reverse-next.exp b/gdb/testsuite/gdb.reverse/finish-reverse-next.exp > index 63305c109e1..240b7214ed2 100644 > --- a/gdb/testsuite/gdb.reverse/finish-reverse-next.exp > +++ b/gdb/testsuite/gdb.reverse/finish-reverse-next.exp > @@ -31,6 +31,16 @@ > # This test verifies the fix for gdb bugzilla: > # https://sourceware.org/bugzilla/show_bug.cgi?id=29927 > > +# PowerPC supports two entry points to a function. The normal entry point > +# is called the local entry point (LEP). The alternat entry point is called > +# the global entry point (GEP). A function call via a function pointer > +# will entry via the GEP. A normal function call will enter via the LEP. > +# > +# This test has been expanded to include tests to verify the reverse-finish > +# command works properly if the function is called via the GEP. The original > +# test only verified the reverse-finish command for a normal call that used > +# the LEP. > + > if ![supports_reverse] { > return > } > @@ -50,30 +60,30 @@ if [supports_process_record] { > } > > > -### TEST 1: reverse finish from the entry point instruction in > -### function1. > +### TEST 1: reverse finish from the entry point instruction (LEP) in > +### function1 when called using the normal entry point (LEP). > > # Set breakpoint at call to function1 in main. > -set bp_FUNCTION [gdb_get_line_number "CALL FUNCTION" $srcfile] > -gdb_breakpoint $srcfile:$bp_FUNCTION temporary > +set bp_LEP_test [gdb_get_line_number "CALL VIA LEP" $srcfile] > +gdb_breakpoint $srcfile:$bp_LEP_test temporary > > # Continue to break point at function1 call in main. > gdb_continue_to_breakpoint \ > "stopped at function1 entry point instruction to stepi into function" \ > - ".*$srcfile:$bp_FUNCTION\r\n.*" > + ".*$srcfile:$bp_LEP_test\r\n.*" > > # stepi until we see "{" indicating we entered function1 > -repeat_cmd_until "stepi" "CALL FUNCTION" "{" "stepi into function1 call" > +repeat_cmd_until "stepi" "CALL VIA LEP" "{" "stepi into function1 call" > > -gdb_test "reverse-finish" ".*function1 \\(a, b\\); // CALL FUNCTION.*" \ > - "reverse-finish function1 " > +gdb_test "reverse-finish" ".*function1 \\(a, b\\); // CALL VIA LEP.*" \ > + "reverse-finish function1 LEP call from LEP " > > # Check to make sure we stopped at the first instruction in the source code > # line. It should only take one reverse next command to get to the previous > # source line. If GDB stops at the last instruction in the source code line > # it will take two reverse next instructions to get to the previous source > # line. > -gdb_test "reverse-next" ".*b = 5;.*" "reverse next at b = 5, call from function" > +gdb_test "reverse-next" ".*b = 5;.*" "reverse next at b = 5, call from LEP" > > # Clear the recorded log. > gdb_test "record stop" "Process record is stopped.*" \ > @@ -84,21 +94,81 @@ gdb_test_no_output "record" "turn on process record for test2" > ### TEST 2: reverse finish from the body of function1. > > # Set breakpoint at call to function1 in main. > -gdb_breakpoint $srcfile:$bp_FUNCTION temporary > +gdb_breakpoint $srcfile:$bp_LEP_test temporary > > # Continue to break point at function1 call in main. > gdb_continue_to_breakpoint \ > "at function1 entry point instruction to step to body of function" \ > - ".*$srcfile:$bp_FUNCTION\r\n.*" > + ".*$srcfile:$bp_LEP_test\r\n.*" duplicate test name here. > > # do a step instruction to get to the body of the function > gdb_test "step" ".*int ret = 0;.*" "step test 1" > > -gdb_test "reverse-finish" ".*function1 \\(a, b\\); // CALL FUNCTION.*" \ > - "reverse-finish function1 call from function body" > +gdb_test "reverse-finish" ".*function1 \\(a, b\\); // CALL VIA LEP.*" \ > + "reverse-finish function1 LEP call from function body" > > # Check to make sure we stopped at the first instruction in the source code > # line. It should only take one reverse next command to get to the previous > # source line. > gdb_test "reverse-next" ".*b = 5;.*" \ > "reverse next at b = 5, from function body" > + > +# Turn off record to clear logs and turn on again > +gdb_test "record stop" "Process record is stopped.*" \ > + "turn off process record for test2" > +gdb_test_no_output "record" "turn on process record for test3" > + > + > +### TEST 3: reverse finish from the alternate entry point instruction (GEP) in > +### function1 when called using the alternate entry point (GEP). > + > +# Set breakpoint at call to funp in main. > +set bp_GEP_test [gdb_get_line_number "CALL VIA GEP" $srcfile] > +gdb_breakpoint $srcfile:$bp_GEP_test temporary > + > +# Continue to break point at funp call in main. > +gdb_continue_to_breakpoint \ > + "stopped at function1 entry point instruction to stepi into function" \ > + ".*$srcfile:$bp_GEP_test\r\n.*" Duplicated test name here too. > + > +# stepi until we see "{" indicating we entered function. > +cmd_until "stepi" "CALL VIA GEP" "{" "stepi into funp call" s/cmd_until/repeat_cmd_until you probably missed because of the test compilation failed. > + > +gdb_test "reverse-finish" ".*funp \\(a, b\\);.*" \ > + "function1 GEP call call from GEP" > + > +# Check to make sure we stopped at the first instruction in the source code > +# line. It should only take one reverse next command to get to the previous > +# source line. If GDB stops at the last instruction in the source code line > +# it will take two reverse next instructions to get to the previous source > +# line. > +gdb_test "reverse-next" ".*b = 50;.*" "reverse next at b = 50, call from GEP" > + > +# Turn off record to clear logs and turn on again > +gdb_test "record stop" "Process record is stopped.*" \ > + "turn off process record for test3" > +gdb_test_no_output "record" "turn on process record for test4" > + > + > +### TEST 4: reverse finish from the body of function 1 when calling using the > +### alternate entrypoint (GEP). > +gdb_breakpoint $srcfile:$bp_GEP_test temporary > + > +# Continue to break point at funp call. > +gdb_continue_to_breakpoint \ > + "at function1 entry point instruction to step to body of function" \ > + ".*$srcfile:$bp_GEP_test\r\n.*" > + > +# Step into body of funp, called via GEP. > +gdb_test "step" ".*int ret = 0;.*" "step test 2" > + > +gdb_test "reverse-finish" ".*funp \\(a, b\\);.*" \ > + "reverse-finish function1 GEP call, from function body " > + > +# Check to make sure we stopped at the first instruction in the source code > +# line. It should only take one reverse next command to get to the previous > +# source line. If GDB stops at the last instruction in the source code line > +# it will take two reverse next instructions to get to the previous source > +# line. > +gdb_test "reverse-next" ".*b = 50;.*" \ > + "reverse next at b = 50 from function body" -- Cheers, Bruno