From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id on6/Aki6kGNFySEAWB0awg (envelope-from ) for ; Wed, 07 Dec 2022 11:07:36 -0500 Received: by simark.ca (Postfix, from userid 112) id F0EFF1E126; Wed, 7 Dec 2022 11:07:35 -0500 (EST) Authentication-Results: simark.ca; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.a=rsa-sha256 header.s=default header.b=YjW8Bi13; dkim-atps=neutral X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-9.0 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,NICE_REPLY_A, RCVD_IN_DNSWL_HI,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 915321E11E for ; Wed, 7 Dec 2022 11:07:35 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id DE9A838FC733 for ; Wed, 7 Dec 2022 16:07:34 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org DE9A838FC733 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1670429254; bh=1v4tYsHyESrh9SShTW9BHq1ZUUoxTkHAtjhIDdXOFdA=; h=Date:Subject:To:References:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=YjW8Bi13gFk0E3Yb1DbP60xHODxBDqrDMsNgzf5kdLPayPSLUpTOu4CN3w84pIXoc ct+ldnNGWRxJ7ag54vc1UQVOiHaDEESBhzNxxEhUUugrQsjyl6LkLt1KU0SHvVp4pL 8/t5atuo5J/8dLoz6OGFzXa++hC886bEs7NZLM5A= Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id 7B63B382FAFF for ; Wed, 7 Dec 2022 16:07:15 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 7B63B382FAFF Received: from [172.16.0.64] (192-222-180-24.qc.cable.ebox.net [192.222.180.24]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id E015B1E112; Wed, 7 Dec 2022 11:07:14 -0500 (EST) Message-ID: <4d4952bf-0e70-253f-578d-8bdeec8bfafc@simark.ca> Date: Wed, 7 Dec 2022 11:07:12 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.4.1 Subject: Re: [PATCH v2] gdb: skip objfiles with no BFD in DWARF unwinder Content-Language: fr To: Jan Vrany , gdb-patches@sourceware.org References: <7b267fea-452e-446e-3800-1cbacd50b689@palves.net> <20220406185524.30713-1-jan.vrany@labware.com> In-Reply-To: <20220406185524.30713-1-jan.vrany@labware.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit 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: , From: Simon Marchi via Gdb-patches Reply-To: Simon Marchi Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org Sender: "Gdb-patches" On 4/6/22 14:55, Jan Vrany via Gdb-patches wrote: > Changes since v1: > > * use with_test_prefix as Lancelot suggested > * rewrote test as Pedro suggested > * updated commit message > > -- >8 -- > While playing with JIT reader I experienced GDB to crash on null-pointer > dereference when stepping through non-jitted code. > > The problem was that dwarf2_frame_find_fde () assumed that all objfiles > have BFD but that's not always true. To address this problem, this > commit skips such objfiles. > > To test the fix we put breakpoint in jit_function_add (). The JIT reader > does not know how unwind this function so unwinding eventually falls > back to DWARF unwinder which in turn iterates over objfiles. Since the > the code is jitted, it is guaranteed it would eventually process JIT > objfile. > --- > gdb/dwarf2/frame.c | 3 +++ > gdb/objfiles.h | 4 +++- > gdb/testsuite/gdb.base/jit-reader.exp | 13 +++++++++++++ > 3 files changed, 19 insertions(+), 1 deletion(-) > > diff --git a/gdb/dwarf2/frame.c b/gdb/dwarf2/frame.c > index 5878d72f922..514ae8c694f 100644 > --- a/gdb/dwarf2/frame.c > +++ b/gdb/dwarf2/frame.c > @@ -1565,6 +1565,9 @@ dwarf2_frame_find_fde (CORE_ADDR *pc, dwarf2_per_objfile **out_per_objfile) > CORE_ADDR offset; > CORE_ADDR seek_pc; > > + if (objfile->obfd == nullptr) > + continue; > + > comp_unit *unit = find_comp_unit (objfile); > if (unit == NULL) > { > diff --git a/gdb/objfiles.h b/gdb/objfiles.h > index 8bd76705688..429dea1da4c 100644 > --- a/gdb/objfiles.h > +++ b/gdb/objfiles.h > @@ -636,7 +636,9 @@ struct objfile > struct compunit_symtab *compunit_symtabs = nullptr; > > /* The object file's BFD. Can be null if the objfile contains only > - minimal symbols, e.g. the run time common symbols for SunOS4. */ > + minimal symbols (e.g. the run time common symbols for SunOS4) or > + if the objfile is a dynamic objfile (e.g. created by JIT reader > + API). */ > > bfd *obfd; > > diff --git a/gdb/testsuite/gdb.base/jit-reader.exp b/gdb/testsuite/gdb.base/jit-reader.exp > index d94360cd7d9..756b400906c 100644 > --- a/gdb/testsuite/gdb.base/jit-reader.exp > +++ b/gdb/testsuite/gdb.base/jit-reader.exp > @@ -271,6 +271,19 @@ proc jit_reader_test {} { > "#1 ${any} in main ${any}" \ > ] > } > + > + with_test_prefix "test dwarf unwinder" { > + # Check that the DWARF unwinder does not crash in presence of > + # JIT objfiles. > + gdb_test "up" > + gdb_breakpoint "*function_add" temporary > + gdb_test "cont" ".*Temporary breakpoint ${any} in jit_function_add .*" > + gdb_test "bt" \ > + [multi_line \ > + "#0 ${any} in jit_function_add ${any}" \ > + "#1 ${any} in main ${any}" \ > + ] > + } This test function is getting a tad long. In general, I find that it makes it difficult to know if some part of the test depends on the state left by a previous portion of the test, if adding something in the middle will break something later (or silently make a test not test what it intended to test anymore). I therefore like to split things up in small independent functions that each start with a clean GDB. This still LGTM though, not a blocker, but I thought I'd mention it for future changes. Approved-By: Simon Marchi Simon