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.129.124]) by sourceware.org (Postfix) with ESMTPS id 36226384F015 for ; Thu, 9 Jun 2022 18:55:23 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 36226384F015 Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-661-bNjiGeMDPQaTmy8Dgr5s-A-1; Thu, 09 Jun 2022 14:55:12 -0400 X-MC-Unique: bNjiGeMDPQaTmy8Dgr5s-A-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.rdu2.redhat.com [10.11.54.2]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id BA279801756; Thu, 9 Jun 2022 18:55:11 +0000 (UTC) Received: from [10.97.116.29] (ovpn-116-29.gru2.redhat.com [10.97.116.29]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 2449340D2827; Thu, 9 Jun 2022 18:55:10 +0000 (UTC) Message-ID: Date: Thu, 9 Jun 2022 15:55:09 -0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.9.1 Subject: Re: [PATCH v3 02/14] Change gdb.base/skip-solib.exp deal with lack of epilogue information To: Pedro Alves , gdb-patches@sourceware.org References: <20220526151041.23223-1-blarsen@redhat.com> <20220526151041.23223-3-blarsen@redhat.com> <23a15847-a4f5-d25b-3477-a03b5942eb21@redhat.com> From: Bruno Larsen In-Reply-To: X-Scanned-By: MIMEDefang 2.84 on 10.11.54.2 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: 8bit X-Spam-Status: No, score=-11.9 required=5.0 tests=BAYES_00, BODY_8BITS, 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, 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.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 09 Jun 2022 18:55:24 -0000 On 6/9/22 15:25, Pedro Alves wrote: > On 2022-06-09 17:27, Bruno Larsen wrote: >> >> On 6/8/22 12:37, Pedro Alves wrote: >>> On 2022-05-26 16:10, Bruno Larsen via Gdb-patches wrote: >>>> When running gdb.base/skip-solib.exp, the backtrace tests could fail if >>>> the compiler did not emit epilogue information for trivial epilogues, >>> >>> I found "emit epilogue information" very confusing FWIW.  When I read that the first time, >>> I thought this was talking about unwind info for the prologue after the stack is destroyed, >>> and I wondered why not fix clang instead. >>> >>> I think the following would be more accurate: >>> >>>   "fail with compilers that associate prologue insns with the function's last >>>    statement line instead of the function's closing brace". >> >> Yeah, I think this is a better explanation, but s/prologue/epilogue, right? > > Right. That was a typo. I meant: > > "fail with compilers that associate epilogue insns with the function's last > statement line instead of the function's closing brace". > >> I tried to make this concept more specific by saying that clang doesn't emit epilogue information, Have I misunderstood or misnamed it? > > To me "epilogue information" sounds like some special DWARF information specific to epilogues. > It reminds of me of special epilogue unwinders that GDB has, like amd64_epilogue_frame_unwind, > i386_epilogue_frame_unwind, rs6000_epilogue_frame_unwind, etc. By not saying "line" anywhere, > I have nothing to connect "epilogue information" to "line info". Once you make the connection, > you understand that there's nothing special about "epilogue information", it's just that clang > and gcc choose different lines numbers for the epilogue instructions. Ah, I see! Then yes, I can change it to what you suggested. Just wanted to confirm my understanding :) > > >>>> diff --git a/gdb/testsuite/gdb.base/skip-inline.exp b/gdb/testsuite/gdb.base/skip-inline.exp >>>> index f6e9926b66c..327ea676140 100644 >>>> --- a/gdb/testsuite/gdb.base/skip-inline.exp >>>> +++ b/gdb/testsuite/gdb.base/skip-inline.exp >>>> @@ -35,16 +35,20 @@ gdb_test "skip function foo" "Function foo will be skipped when stepping\." >>>>   gdb_test "bt" "\\s*\\#0\\s+main.*" "in the main" >>>>   gdb_test "step" ".*" "step into baz, since foo will be skipped" >>>>   gdb_test "bt" "\\s*\\#0\\s+baz.*" "in the baz, since foo was skipped" >>>> -gdb_test "step" ".*" "step in the baz" >>>> -gdb_test "bt" "\\s*\\#0\\s+baz.*" "still in the baz" >>>> -gdb_test "step" ".*" "step back to main" >>>> +gdb_step_until_regexp ".*x = 0; x = baz \\(foo \\(\\)\\).*" >>> >>> FWIW, I'd remove the "regexp" from the function's name, just call it gdb_step_until. >> >> Alright, but this sounds more like a review of the first patch than this one. > > It only stood out to me when I saw it in use, as in the function name is long > and regexp stuck out like a sore thumb here. :-) > >>> Or even with a simple "just main" program, like so: >>> >>>   (gdb) list >>>   1       int >>>   2       main () >>>   3       { >>>   4         return 0; >>>   5       } >>> >>> we get this with gcc: >>> >>>   (gdb) info line 4 >>>   Line 4 of "main.c" starts at address 0x1131 and ends at 0x1136 . >>>   (gdb) info line 5 >>>   Line 5 of "main.c" starts at address 0x1136 and ends at 0x1138. >>>   (gdb) >>> >>> and this with clang: >>> >>>   (gdb) info line 4 >>>   Line 4 of "main.c" starts at address 0x40111d and ends at 0x40111f. >>>   (gdb) info line 5 >>>   Line number 5 is out of range for "main.c". >>>   (gdb) >>> >>> I think that we can use that to write a caching proc like: >>> >>>   # Return true if the compiler emits line information associating prologue insns with >>>   # the function's closing brace.  Return false if not, meaning the prologue >>>   # associates prologue instructions with function's last line with a statement. >>> >>>   gdb_caching_proc have_prologue_line_info { >>>       use gdb_simple_compile the simple main program from above >>>             use "info line 5", and return false if we get "out of range", otherwise return true. >>>   } >>> >> >> This sounds like a good idea, I would only change the default behavior to returning false, unless we saw specifically "start at address.*and ends at". > > ... > >> This would make GDB changes more pronounced, as GCC would start having different results. > > Note sure what you mean here. > I meant that if something changes in GDB and the epilogue detection fails, making gcc's test result different would be easier to detect than making clang's results different. Cheers! Bruno Larsen