From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=simark.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=simark.ca 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) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=simark.ca; s=mail; t=1670429235; bh=VdBvNPqWlJpRkO0PqOgpkH3NcgIzHF9Dc0UpT8aYLIs=; h=Date:Subject:To:References:From:In-Reply-To:From; b=sY1apQRUdoDRH6EiPw9U9ZL2eOpCO8WlRNTaD6T9dEGNCdqhvRi7nuex2I7kNm9DB 9Bzeib0DrBPf6D5iJx3SP90PYeuBuJCUSrMtReeYWZIk6sFW5IfJLvY5AKQaiOfNkO ForsrqtS9A1ZWjPz7iBCDnIry5qwJKuK85Ieh6ww= 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> From: Simon Marchi In-Reply-To: <20220406185524.30713-1-jan.vrany@labware.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-11.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,NICE_REPLY_A,SPF_HELO_PASS,SPF_PASS,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 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