From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out1.suse.de (smtp-out1.suse.de [IPv6:2001:67c:2178:6::1c]) by sourceware.org (Postfix) with ESMTPS id DFCD13858C60 for ; Wed, 15 Feb 2023 09:39:58 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org DFCD13858C60 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.de Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id 5DB2722590; Wed, 15 Feb 2023 09:39:57 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1676453997; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=yNX9sCvoswpVbMj+gNva3J81BcGNDElNniU+oTBfCBw=; b=I06/MhOsCGa99sKE2aOzrwHI9rIiQQ0KtcNbfyW/wLnc4OKNCJze7xNyneuNbPQBqrOMmI uaniED/tc95z8snALe/WbWIdKX162f+pWrkzD/IQUF2ndmkA/ynNqhIOg0vOH8X8Q2CNnK dXAIL02PtEIwlu3w/G3gbxOd+cp7vNo= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1676453997; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=yNX9sCvoswpVbMj+gNva3J81BcGNDElNniU+oTBfCBw=; b=YeLb01BSRb5T7DCtTV/ceyQ1nrQIdBEJzYve3Hu+JgLOy+VoNG+Ws81RznwdL3gUjEDQAb Myahj0/TeZ4LIxCQ== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 3FCDF13483; Wed, 15 Feb 2023 09:39:57 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id 43hfDm2o7GMbfAAAMHmgww (envelope-from ); Wed, 15 Feb 2023 09:39:57 +0000 Content-Type: multipart/mixed; boundary="------------Py9HgK0UJkQrkmJYbvZI5fVb" Message-ID: <8b8d6d16-ed28-bebb-9136-85fc81968fa5@suse.de> Date: Wed, 15 Feb 2023 10:39:59 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.7.1 Subject: Re: [PATCH] gdb: fix dealloc function not being called for frame 0 Content-Language: en-US To: Simon Marchi , Simon Marchi , gdb-patches@sourceware.org References: <20230209195037.100368-1-simon.marchi@efficios.com> <30ee85a3-d6aa-c69e-2fe6-3c6d53a11a90@simark.ca> From: Tom de Vries In-Reply-To: <30ee85a3-d6aa-c69e-2fe6-3c6d53a11a90@simark.ca> X-Spam-Status: No, score=-12.3 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,KAM_NUMSUBJECT,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,TXREP,WEIRD_PORT 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: This is a multi-part message in MIME format. --------------Py9HgK0UJkQrkmJYbvZI5fVb Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 2/10/23 04:10, Simon Marchi wrote: > >> @@ -2105,7 +2106,23 @@ reinit_frame_cache (void) >> invalidate_selected_frame (); >> >> /* Invalidate cache. */ >> - sentinel_frame = NULL; >> + if (sentinel_frame != nullptr) >> + { >> + /* If frame 0's id is not computed, it is not in the frame stash, so its >> + dealloc functions will not be called when emptying the frash stash. >> + Call frame_info_del manually in that case. */ >> + frame_info *current_frame = sentinel_frame->prev; >> + if (current_frame != nullptr) >> + { >> + gdb_assert (current_frame->this_id.p != frame_id_status::COMPUTING); > > Well, this gdb_assert happens to cause a failure in > gdb.server/server-kill.exp: > > (gdb) PASS: gdb.server/server-kill.exp: kill_pid_of=inferior: test_unwind_syms: p server_pid > Executing on target: kill -9 1567851 (timeout = 300) > builtin_spawn -ignore SIGHUP kill -9 1567851 > bt > /home/simark/src/binutils-gdb/gdb/frame.c:2117: internal-error: reinit_frame_cache: Assertion `current_frame->this_id.p != frame_id_status::COMPUTING' failed. > A problem internal to GDB has been detected, further debugging may prove unreliable. > FAIL: gdb.server/server-kill.exp: kill_pid_of=inferior: test_unwind_syms: bt (GDB internal error) > > This happens because the remote connection breaks while computing a > frame id. The stack looks roughly like this (many frames removed for > readability): > > 0x55cd4623396c internal_error_loc(char const*, int, char const*, ...) > /home/simark/src/binutils-gdb/gdbsupport/errors.cc:58 > 0x55cd4434f5a1 reinit_frame_cache() > /home/simark/src/binutils-gdb/gdb/frame.c:2117 > 0x55cd457e1bce switch_to_no_thread() > /home/simark/src/binutils-gdb/gdb/thread.c:1330 > 0x55cd44618270 switch_to_inferior_no_thread(inferior*) > /home/simark/src/binutils-gdb/gdb/inferior.c:671 > 0x55cd451467c9 remote_unpush_target > /home/simark/src/binutils-gdb/gdb/remote.c:5903 > 0x55cd45183b88 unpush_and_perror > /home/simark/src/binutils-gdb/gdb/remote.c:9612 > 0x55cd451840ce remote_target::readchar(int) > /home/simark/src/binutils-gdb/gdb/remote.c:9652 > 0x55cd45188e91 remote_target::getpkt_or_notif_sane_1(std::__debug::vector > >*, int, int, int*) > /home/simark/src/binutils-gdb/gdb/remote.c:10118 > 0x55cd4518a2fe remote_target::getpkt_sane(std::__debug::vector > >*, int) > /home/simark/src/binutils-gdb/gdb/remote.c:10220 > 0x55cd451889d6 remote_target::getpkt(std::__debug::vector > >*, int) > /home/simark/src/binutils-gdb/gdb/remote.c:10062 > 0x55cd45181137 remote_target::remote_read_bytes_1(unsigned long, unsigned char*, unsigned long, int, unsigned long*) > /home/simark/src/binutils-gdb/gdb/remote.c:9379 > 0x55cd45182772 remote_target::remote_read_bytes(unsigned long, unsigned char*, unsigned long, int, unsigned long*) > /home/simark/src/binutils-gdb/gdb/remote.c:9503 > 0x55cd4519cd2d remote_target::xfer_partial(target_object, char const*, unsigned char*, unsigned char const*, unsigned long, unsigned long, unsigned long*) > /home/simark/src/binutils-gdb/gdb/remote.c:11421 > ... > 0x55cd4433d812 compute_frame_id > /home/simark/src/binutils-gdb/gdb/frame.c:606 > > So, I will remove that gdb_assert and simplify the code to: > > /* If frame 0's id is not computed, it is not in the frame stash, so its > dealloc functions will not be called when emptying the frash stash. > Call frame_info_del manually in that case. */ > frame_info *current_frame = sentinel_frame->prev; > if (current_frame != nullptr > && current_frame->this_id.p == frame_id_status::NOT_COMPUTED) > frame_info_del (current_frame); > > With that change, the test passes again. > > If the state of the id of the current frame is "COMPUTING", I think it's > better not to call frame_info_del and the dealloc functions, because the > state of the per-unwinder cache object is maybe not in a state expected > that the dealloc functions expect. If something goes wrong during the > computation of the frame id, I think it's best to let the unwinder make > sure to clean up anything it has started. Hi Simon, I've: - applied the initial patch - verified that it fixes the gdb.btrace/record_goto.exp test-case - verified that it breaks the gdb.server/server-kill.exp test-case - applied your proposed change (attached in patch form for clarity) - verified that it fixes the gdb.server/server-kill.exp test-case - verified that it still fixes the gdb.btrace/record_goto.exp test-case - done a complete build and test run, no issues found (in other words, I'm back to just one FAIL, in gdb.server/monitor-exit-quit.exp, PR29833) Thanks, - Tom --------------Py9HgK0UJkQrkmJYbvZI5fVb Content-Type: text/x-patch; charset=UTF-8; name="0001-fixup.patch" Content-Disposition: attachment; filename="0001-fixup.patch" Content-Transfer-Encoding: base64 RnJvbSBhNjNmZTA5NDM4ZGViYTI4NjQ2NGFjNzk2ODU1ODc4OTBlYThhZTM5IE1vbiBTZXAg MTcgMDA6MDA6MDAgMjAwMQpGcm9tOiBTaW1vbiBNYXJjaGkgPHNpbW9uLm1hcmNoaUBlZmZp Y2lvcy5jb20+CkRhdGU6IFdlZCwgMTUgRmViIDIwMjMgMDk6NDM6MzUgKzAxMDAKU3ViamVj dDogW1BBVENIXSBmaXh1cAoKLS0tCiBnZGIvZnJhbWUuYyB8IDEwICsrKy0tLS0tLS0KIDEg ZmlsZSBjaGFuZ2VkLCAzIGluc2VydGlvbnMoKyksIDcgZGVsZXRpb25zKC0pCgpkaWZmIC0t Z2l0IGEvZ2RiL2ZyYW1lLmMgYi9nZGIvZnJhbWUuYwppbmRleCA2OTczMDk3OTcwMy4uNjYy OWY0NDRhZmUgMTAwNjQ0Ci0tLSBhL2dkYi9mcmFtZS5jCisrKyBiL2dkYi9mcmFtZS5jCkBA IC0yMTEyLDEzICsyMTEyLDkgQEAgcmVpbml0X2ZyYW1lX2NhY2hlICh2b2lkKQogCSBkZWFs bG9jIGZ1bmN0aW9ucyB3aWxsIG5vdCBiZSBjYWxsZWQgd2hlbiBlbXB0eWluZyB0aGUgZnJh c2ggc3Rhc2guCiAJIENhbGwgZnJhbWVfaW5mb19kZWwgbWFudWFsbHkgaW4gdGhhdCBjYXNl LiAgKi8KICAgICAgIGZyYW1lX2luZm8gKmN1cnJlbnRfZnJhbWUgPSBzZW50aW5lbF9mcmFt ZS0+cHJldjsKLSAgICAgIGlmIChjdXJyZW50X2ZyYW1lICE9IG51bGxwdHIpCi0JewotCSAg Z2RiX2Fzc2VydCAoY3VycmVudF9mcmFtZS0+dGhpc19pZC5wICE9IGZyYW1lX2lkX3N0YXR1 czo6Q09NUFVUSU5HKTsKLQotCSAgaWYgKGN1cnJlbnRfZnJhbWUtPnRoaXNfaWQucCA9PSBm cmFtZV9pZF9zdGF0dXM6Ok5PVF9DT01QVVRFRCkKLQkgICAgZnJhbWVfaW5mb19kZWwgKGN1 cnJlbnRfZnJhbWUpOwotCX0KKyAgICAgIGlmIChjdXJyZW50X2ZyYW1lICE9IG51bGxwdHIK KwkgICYmIGN1cnJlbnRfZnJhbWUtPnRoaXNfaWQucCA9PSBmcmFtZV9pZF9zdGF0dXM6Ok5P VF9DT01QVVRFRCkKKwlmcmFtZV9pbmZvX2RlbCAoY3VycmVudF9mcmFtZSk7CiAKICAgICAg IHNlbnRpbmVsX2ZyYW1lID0gbnVsbHB0cjsKICAgICB9CgpiYXNlLWNvbW1pdDogYjZkYWE3 N2QwZDkxMjE2MjY2YmNjYmUxODJjYzdkYzY4OTU5MTY5MgotLSAKMi4zNS4zCgo= --------------Py9HgK0UJkQrkmJYbvZI5fVb--