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 477763858C20 for ; Thu, 10 Mar 2022 13:50:02 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 477763858C20 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-558-AG2_6S37N7qkLyY7Gga_Ng-1; Thu, 10 Mar 2022 08:50:00 -0500 X-MC-Unique: AG2_6S37N7qkLyY7Gga_Ng-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id A2FA51006AA5; Thu, 10 Mar 2022 13:49:58 +0000 (UTC) Received: from [10.97.116.153] (ovpn-116-153.gru2.redhat.com [10.97.116.153]) by smtp.corp.redhat.com (Postfix) with ESMTPS id F22C97DE2B; Thu, 10 Mar 2022 13:49:54 +0000 (UTC) Message-ID: <806036b5-31d3-1fd2-044b-b7ba1736d261@redhat.com> Date: Thu, 10 Mar 2022 10:49:51 -0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.7.0 Subject: Re: [PATCH] Updated, fix reverse stepping multiple contiguous PC ranges To: Carl Love , gdb-patches@sourceware.org Cc: Rogerio Alves , luis.machado@arm.com References: <442684e3f81aa1df073960bd45918106acefa2b9.camel@us.ibm.com> <0fbaf3783401f8aa8e76a4d3928efff46485ab8d.camel@us.ibm.com> <3b2ab293-92d9-aa9b-bff2-3b6eaff27d59@redhat.com> <4feed1aaa0d5d9b166c7c0e608918f883137fad1.camel@us.ibm.com> From: Bruno Larsen In-Reply-To: <4feed1aaa0d5d9b166c7c0e608918f883137fad1.camel@us.ibm.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 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=-7.2 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H5, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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: Thu, 10 Mar 2022 13:50:03 -0000 Carl: On 3/9/22 17:52, Carl Love wrote: > Bruno: > > On Wed, 2022-03-09 at 09:23 -0300, Bruno Larsen wrote: >>>> Thanks for looking at this. Since I don't test on aarch64 often, >>>> I am >>>> not sure if I see regressions or racy testcases, but it does fix >>>> the >>>> issue you mentioned, and there doesn't seem to be regressions on >>>> x86_64 hardware. I have a few nits, but the main feedback is: >>>> could >>>> you add a testcase for this, using the dwarf assembler and >>>> manually creating contiguous PC ranges, so we can confirm that >>>> this is not >>>> regressed in the future on any hardware? >>>> >>>> Also, I can't approve a patch, but with the testcase this patch >>>> is >>>> mostly ok by me >>>> >>> >>> I have not writen anything in dwarf assembler in the past. Off the >>> top >>> of my head, don't know how to create such a test case. It does >>> seem >>> that there are the two testcases gdb.reverse/solib-precsave.exp >>> and >>> gdb.reverse/solib-reverse.exp which the patch fixes. I guess the >>> dwarf >>> assembly test would be similar to the C level code. >>> >>> Do you have an example of how to write dwarf assembly or a pointer >>> to a >>> tutorial on writting dwarf? >> >> I have recently worked on gdb.base/until-trailing-insns.exp, that >> uses the dwarf assembler quite a bit to create a line table. I am not >> sure how one would go about making contiguous ranges, but maybe you >> can find something in gdb/testsuite/lib/dwarf.exp to help you. > > I have studied until-trailing-insns.exp, dwarf.exp and the DWARF > Debugging Informat Format Version 5 document. I really don't see how > to write the test case you desire. > > The issue is gdb is supposed to step in reverse one statement at a > time. The two testcases that the patch fix have two statements on the > same line. Specifically from gdb.reverse/solib-reverse.c it has the > source code line: b[0] = 6; b[1] = 9;. It is this line where the > reverse step fails to stop in between the two statements. > > I tried > dumping the DWARF info for a very simple program with two statements on > the same line as in the two test cases that fail to see what GCC > generates. The test program is: > > int main() { > int i,j; > i = 1; j=3; > printf("i=%d, j=%d\n", i, j); > } > > gcc -g -O0 test2.c -o test2 > objdump -g -W -S -d test2 > test2.dump > > The interesting dump information is: > > int main() { > 7fc: 02 00 4c 3c addis r2,r12,2 > 800: 04 77 42 38 addi r2,r2,30468 > 804: a6 02 08 7c mflr r0 > 808: 10 00 01 f8 std r0,16(r1) > 80c: f8 ff e1 fb std r31,-8(r1) > 810: 81 ff 21 f8 stdu r1,-128(r1) > 814: 78 0b 3f 7c mr r31,r1 > int i,j; > i = 1; j=3; > 818: 01 00 20 39 li r9,1 // Store i = 1 > 81c: 68 00 3f 91 stw r9,104(r31) > 820: 03 00 20 39 li r9,3 // Store j = 3 > 824: 6c 00 3f 91 stw r9,108(r31) > printf("i=%d, j=%d\n", i, j); > > Line Number Statements: > [0x00000040] Set column to 12 > [0x00000042] Extended opcode 2: set Address to 0x7fc > [0x0000004d] Special opcode 10: advance Address by 0 to 0x7fc and Line by 5 to 6 > [0x0000004e] Set column to 5 > [0x00000050] Special opcode 105: advance Address by 28 to 0x818 and Line by 2 to 8 > [0x00000051] Set column to 11 > [0x00000053] Special opcode 33: advance Address by 8 to 0x820 and Line by 0 to 8 > [0x00000054] Set column to 3 > [0x00000056] Special opcode 34: advance Address by 8 to 0x828 and Line by 1 to 9 > [0x00000057] Set column to 1 > [0x00000059] Special opcode 146: advance Address by 40 to 0x850 and Line by 1 to 10 > [0x0000005a] Advance PC by 36 to 0x874 > [0x0000005c] Extended opcode 1: End of Sequence > > > Here I can see that "[0x00000053] Special opcode 33: advance Address > by 8 to 0x820 and Line by 0 to 8" refers to address 0x820 in the > binary which is the assembly statement for j=3. Also, it says advance > Line by 0, i.e. it is the same line number as the previous statement > i=1. > > The line entries in the line table, i.e. Line Number, are different. > > In your message, you said to create a test case that has "continuous PC > ranges." I don't see where/how that would be reflected in the DWARF > info as dumped above? I don't see GCC generating information/code with > continuous PC ranges. I have not found anything that would enable me > to do that. Each assembly statement has a unique PC value, but the > DWARF info does track the source Line number separately. The testcase I had in mind could be something simple like this: test.c: 1 int main (){ 2 asm ("main_label: .globl main_label"); 3 int i,j; 4 asm ("i_assignment: .globl i_assignment"); 5 i = 0; /* TAG: assignment line */ 6 asm ("j_assignment: .globl j_assignment"); 7 j = 0; 8 return 0; 9 } test.exp: ... dwarf::assemble{ ... {DW_LNE_set_address i_assignment} {line [gdb_get_line_number "TAG: assignment line"]} {DW_LNS_copy} {DW_LNE_set_address j_assignment} {line [gdb_get_line_number "TAG: assignment line"]} {DW_LNS_copy} ... } To explain what is going on: We use the asm() labels to identify PC addresses for the assignment of variables. We use the comment /* TAG ... */ to identify the line we care about. And then we pretend that those 2 PC addresses came from the same line. Even though they didn't in the actual .c, GDB will think they did because of the dwarf information, guaranteeing that we will continue to test this for regressions. (side note: main_label needs to be there for the dwarf assembler to work correctly.) If I understand the problem correctly, this test and dwarf assembler snippet should be able to reproduce the bug, but I am also no DWARF expert, so I may be missing something as well. > > Clearly GCC is generating DWARF that specifies the statements are in > the same line but with different PC values. I have not found anything > that would help me "making contiguous ranges" that you refere to. I > really don't see how a new DWARF test is going to be different or > better then the existing C tests? Perhaps you could explain more as to > the value of this new test and why the existing two C code tests which > the patch fixes are not sufficient to detect a future gdb regression? > Sorry, I just don't see how to create the test you propose and at this > point, and I am not seeing the value in it. I don't claim to be an > expert on DWARF so it is entirely possible I am missing something here. The value comming from a test like this is that we can have the aarch64 behavior on any CPU architecture. This will give us more test coverage, as I can't easily test on aarch64 hardware for instance, and I imagine a lot of people may just be unable to test it. Also,it is possible that GCC will change this behavior in the future for those specific tests, and we unknowingly stop testing for this regression. > > Thanks for your feedback and help on this. > > Carl > > > -- Cheers! Bruno Larsen