From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id v4qgMhhB6mlk8DYAWB0awg (envelope-from ) for ; Thu, 23 Apr 2026 11:56:08 -0400 Authentication-Results: simark.ca; dkim=pass (1024-bit key; unprotected) header.d=suse.de header.i=@suse.de header.a=rsa-sha256 header.s=susede2_rsa header.b=NY31Dc/6; dkim=pass header.d=suse.de header.i=@suse.de header.a=ed25519-sha256 header.s=susede2_ed25519 header.b=zKQralS8; dkim=pass (1024-bit key) header.d=suse.de header.i=@suse.de header.a=rsa-sha256 header.s=susede2_rsa header.b=pPx4DwPi; dkim=neutral header.d=suse.de header.i=@suse.de header.a=ed25519-sha256 header.s=susede2_ed25519 header.b=hhF7XLpc; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id B1EEE1E0BA; Thu, 23 Apr 2026 11:56:08 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-25) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-2.4 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,RCVD_IN_VALIDITY_CERTIFIED_BLOCKED, RCVD_IN_VALIDITY_RPBL_BLOCKED,RCVD_IN_VALIDITY_SAFE_BLOCKED autolearn=ham autolearn_force=no version=4.0.1 Received: from vm01.sourceware.org (vm01.sourceware.org [38.145.34.32]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange x25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 68D671E067 for ; Thu, 23 Apr 2026 11:56:07 -0400 (EDT) Received: from vm01.sourceware.org (localhost [127.0.0.1]) by sourceware.org (Postfix) with ESMTP id E70464BC22EC for ; Thu, 23 Apr 2026 15:56:06 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org E70464BC22EC Authentication-Results: sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=suse.de header.i=@suse.de header.a=rsa-sha256 header.s=susede2_rsa header.b=NY31Dc/6; dkim=pass header.d=suse.de header.i=@suse.de header.a=ed25519-sha256 header.s=susede2_ed25519 header.b=zKQralS8; dkim=pass (1024-bit key) header.d=suse.de header.i=@suse.de header.a=rsa-sha256 header.s=susede2_rsa header.b=pPx4DwPi; dkim=neutral header.d=suse.de header.i=@suse.de header.a=ed25519-sha256 header.s=susede2_ed25519 header.b=hhF7XLpc Received: from smtp-out2.suse.de (smtp-out2.suse.de [IPv6:2a07:de40:b251:101:10:150:64:2]) by sourceware.org (Postfix) with ESMTPS id 7B5374BC0C1A for ; Thu, 23 Apr 2026 15:55:40 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 7B5374BC0C1A Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.de ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 7B5374BC0C1A Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2a07:de40:b251:101:10:150:64:2 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1776959740; cv=none; b=rSz9eQB/dl7IZMAvapjRvHc6omMjkM+Ku6npPVRaaBRNuzRiAFnQZ6/fs281gWTCoNzYggsXM7UeNHH9CneQ1SRNvifM/oRk6/DFVFwxUL0MZsyKddTFW+PRrA9oKrY5aX0M/XsminMuBEpz3BIJM8BpxnkIiHaJ4ebDu8xPNS8= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1776959740; c=relaxed/simple; bh=2fukZO6h3/j9clKqJCdhWBtiw5mcxPLfQgsylxegXgI=; h=DKIM-Signature:DKIM-Signature:DKIM-Signature:DKIM-Signature: Message-ID:Date:MIME-Version:Subject:To:From; b=gfjvUQkEg9C66ZC2azeZYYZ8GbCNE47f/IoAJvR6TH9cpjOy4oYaD9oOLQioAhHqOV+txqKF6zGGOKAyLK0GynXvegluALeJzVarMA340NfELcXbPet50jDepGwymt92ewPRhq/S5IAqcAN/qTu52qHExBs5C0aOVx84DoL1cak= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 7B5374BC0C1A Received: from imap1.dmz-prg2.suse.org (imap1.dmz-prg2.suse.org [IPv6:2a07:de40:b281:104:10:150:64:97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id 6CE445BDD1; Thu, 23 Apr 2026 15:55:38 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1776959739; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=BUNNMYVjX0OwY+n/cy1Mw2NivyHowBe6jDFddC7A/zw=; b=NY31Dc/6edt2PZBi+cN34YDzNu7qITPi0ymmfawNZgoezgu5O0iohjzSGuVv4Mbnc2Qtl+ XV13e44kx2NLJicMs9zMUgmAcTYxOVQGdjuAfpPOTdoG8lnGLPg/dHBefXuza3tP+FRGp6 qBKM2LdEOhx+zadQPVXUE3jZYBlqAA4= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1776959739; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=BUNNMYVjX0OwY+n/cy1Mw2NivyHowBe6jDFddC7A/zw=; b=zKQralS8CFB9gRDc9LauMjubvqHH5nY1/0WQAiv/YH7syzgzkq/3HK5D2R7udr2dUv5Szf TMzw0mCPdN0Eb5AQ== Authentication-Results: smtp-out2.suse.de; dkim=pass header.d=suse.de header.s=susede2_rsa header.b=pPx4DwPi; dkim=pass header.d=suse.de header.s=susede2_ed25519 header.b=hhF7XLpc DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1776959738; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=BUNNMYVjX0OwY+n/cy1Mw2NivyHowBe6jDFddC7A/zw=; b=pPx4DwPiyZv0up9KC2RTPxOn1KVcabOXm5a/h4seuv0zU1eryGvpkH0QA+zjYURA2M60JY NtpQEeDo+pWSZYUV5BRFfb3sIKH7uorBSurrIg4ciF8IgDTJpz8hQ6Lmp2SV4zy94iThWl BjMfk2VOG+lvVXUs/qAruKFTGSnHREE= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1776959738; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=BUNNMYVjX0OwY+n/cy1Mw2NivyHowBe6jDFddC7A/zw=; b=hhF7XLpcyVxOexsXa+K4dHknvSCHrvhb3a/84P5t0XdbD0XlvArgnCjc+yz5he3OPgF6Vf KbOsu3PzpL4pQCDA== Received: from imap1.dmz-prg2.suse.org (localhost [127.0.0.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by imap1.dmz-prg2.suse.org (Postfix) with ESMTPS id 5696D593A3; Thu, 23 Apr 2026 15:55:38 +0000 (UTC) Received: from dovecot-director2.suse.de ([2a07:de40:b281:106:10:150:64:167]) by imap1.dmz-prg2.suse.org with ESMTPSA id 9LzhE/pA6mk9QwAAD6G6ig (envelope-from ); Thu, 23 Apr 2026 15:55:38 +0000 Message-ID: <575eb7cb-8809-41b4-942b-9de646be1d27@suse.de> Date: Thu, 23 Apr 2026 17:55:38 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2] [gdb] Fix segfault in sig_write To: Andrew Burgess , gdb-patches@sourceware.org References: <20260402084914.946223-1-tdevries@suse.de> <87340o2swv.fsf@redhat.com> Content-Language: en-US From: Tom de Vries In-Reply-To: <87340o2swv.fsf@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Rspamd-Action: no action X-Rspamd-Server: rspamd2.dmz-prg2.suse.org X-Spamd-Result: default: False [-4.51 / 50.00]; BAYES_HAM(-3.00)[100.00%]; NEURAL_HAM_LONG(-1.00)[-1.000]; R_DKIM_ALLOW(-0.20)[suse.de:s=susede2_rsa,suse.de:s=susede2_ed25519]; NEURAL_HAM_SHORT(-0.20)[-1.000]; MIME_GOOD(-0.10)[text/plain]; MX_GOOD(-0.01)[]; FUZZY_RATELIMITED(0.00)[rspamd.com]; ARC_NA(0.00)[]; MIME_TRACE(0.00)[0:+]; RCVD_VIA_SMTP_AUTH(0.00)[]; TO_DN_SOME(0.00)[]; MID_RHS_MATCH_FROM(0.00)[]; RCVD_TLS_ALL(0.00)[]; SPAMHAUS_XBL(0.00)[2a07:de40:b281:104:10:150:64:97:from]; FROM_EQ_ENVFROM(0.00)[]; FROM_HAS_DN(0.00)[]; RCPT_COUNT_TWO(0.00)[2]; RCVD_COUNT_TWO(0.00)[2]; TO_MATCH_ENVRCPT_ALL(0.00)[]; DBL_BLOCKED_OPENRESOLVER(0.00)[imap1.dmz-prg2.suse.org:helo,imap1.dmz-prg2.suse.org:rdns]; DKIM_SIGNED(0.00)[suse.de:s=susede2_rsa,suse.de:s=susede2_ed25519]; DKIM_TRACE(0.00)[suse.de:+] X-Rspamd-Queue-Id: 6CE445BDD1 X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~public-inbox=simark.ca@sourceware.org On 4/21/26 6:13 PM, Andrew Burgess wrote: > Tom de Vries writes: > >> I ran into a segfault in sig_write: >> ... >> if (gdb_stderr == nullptr || gdb_stderr->fd () == -1) >> ... >> >> [ A regression since commit 817003ed469 ("Rewrite output redirection and >> logging"). ] >> >> This happens as follows. >> >> First, we get gdb_stderr by calling current_gdb_stderr, which returns >> current_ui.current_interpreter.m_stderr.get (), which is not nullptr. >> >> Then we do "gdb_stderr->fd ()", which gets us to >> wrapped_file >::fd: >> ... >> int fd () const override >> { return m_stream->fd (); } >> ... >> >> The "m_stream->" part brings us to: >> ... >> /* A "smart pointer" that references a particular member of the >> current UI. */ >> template >> struct ui_file_ptr >> { >> ui_file *operator-> () const >> { >> return current_ui->*F; >> } >> }; >> ... >> which does "current_ui.m_ui_stderr" which indeed is nullptr. > Hi Andrew, thanks for the review. > It would be nice if the commit message explained *why* there's a nullptr > there. Ack, I've submitted a v3 ( https://sourceware.org/pipermail/gdb-patches/2026-April/226736.html ) explaining that briefly, by referring to the commit that fixed that problem. > I followed the path back through the many iterations of this work to > what I think is the analysis of why this happens: > > https://inbox.sourceware.org/gdb-patches/716d68d7-6888-4da6-a0e1-d8bf6c75f663@suse.de > > Which makes me wonder, is this still a problem after commit: > > commit b171f68e9450e3bba074b0fe918b78c2ebb03af8 > Author: Tom de Vries > Date: Thu Apr 2 23:09:09 2026 +0200 > > [gdb/tui] Make tui_setup_io more robust > > That commit indeed fixes the problem that current_ui.m_ui_stderr is nullptr. What this patch fixes is that sigwrite is not robust enough to handle the situation that current_ui.m_ui_stderr is nullptr, which shouldn't but could happen. We want sigwrite to be as robust as possible because it's something that gets called when things go wrong. >> >> Fix this in wrapped_file::fd by checking for a nullptr m_stream. >> > >> diff --git a/gdb/ui-file.h b/gdb/ui-file.h >> index 6a1d3964335..b7d3aebda9a 100644 >> --- a/gdb/ui-file.h >> +++ b/gdb/ui-file.h >> @@ -420,7 +420,7 @@ class wrapped_file : public ui_file >> { m_stream->emit_style_escape (style); } >> >> int fd () const override >> - { return m_stream->fd (); } >> + { return m_stream == nullptr ? -1 : m_stream->fd (); } > > Maybe the analysis I linked above is out of date, or I found the wrong > thing, but it seemed to indicate we installed a NULL object into > m_stream at some point because we "restored" something that was never > actually "stored" in the first place. > > Tom had a suggestion here: > > https://inbox.sourceware.org/gdb-patches/id:871pt6zwb7.fsf@tromey.com > > which I'm not sure was ever followed up on (but I might have missed it, > there are a lot of threads relating to this work). The idea there is to > avoid restoring something that was never stored. > Right, I have not followed up on that. I've filed a PR about it now ( https://sourceware.org/bugzilla/show_bug.cgi?id=34100 ). It's a valid suggestion, unfortunately it didn't have any relevance for the actual problem at hand, which was a segfault while trying to report a segfault. > Another possibility, if we absolutely cannot avoid "restoring" something > that was never stored would be to have the initial state of the thing > that gets restored be some kind of dummy stream whose fd() method return > -1. > > My problem with the above is it feels (to me) like throwing a random: > "if (ptr != NULL)" check in because we cannot fix the code that > incorrectly sets 'ptr' to NULL. This is not about not being able to fix code, this is about robustly handling the case that something that shouldn't happen does happen. Agreed, the situation that current_ui.m_ui_stderr is nullptr is interesting, and normally we'd like to report it, but the exceptional situation is that we're trying to report something else, but can't. Of course there is an easy solution: ... void sig_write (const char *msg) { - if (gdb_stderr == nullptr || gdb_stderr->fd () == -1) std::ignore = ::write (2, msg, strlen (msg)); - else - gdb_stderr->write_async_safe (msg, strlen (msg)); } ... but that is probably breaking some ui invariant. I hope this clarifies my intention with this patch. Thanks, - Tom > But that's just my instinct when > reading the change, maybe the NULL really is unavoidable, it's just the > commit message doesn't convince me of that, it just explains *where* the > NULL is, not *why* the NULL has to be. > > Thanks, > Andrew > > >> >> void puts_unfiltered (const char *str) override >> { m_stream->puts_unfiltered (str); } >> diff --git a/gdb/ui.h b/gdb/ui.h >> index 891660896ef..ef977470302 100644 >> --- a/gdb/ui.h >> +++ b/gdb/ui.h >> @@ -163,6 +163,10 @@ struct ui >> { >> return current_ui->*F; >> } >> + bool operator== (nullptr_t p) const >> + { >> + return current_ui->*F == nullptr; >> + } >> }; >> >> /* A ui_file that simply forwards. */ >> diff --git a/gdb/unittests/ui-file-selftests.c b/gdb/unittests/ui-file-selftests.c >> index 69e48735001..b9c1aca3774 100644 >> --- a/gdb/unittests/ui-file-selftests.c >> +++ b/gdb/unittests/ui-file-selftests.c >> @@ -48,6 +48,20 @@ run_tests () >> scoped_restore save_7 = make_scoped_restore (&sevenbit_strings, true); >> check_one ("more weird stuff: \xa5", '\\', >> "more weird stuff: \\245"); >> + >> + { >> + /* There's a bug that has the effect "*redirectable_stderr () == nullptr". >> + In that case, gdb_stderr is not nullptr, but the underlying pointer is >> + a nullptr. Check that "gdb_stderr->fd ()" doesn't dereference the >> + underlying nullptr. >> + This allows us to check for a usable gdb_stderr using >> + "gdb_stderr != nullptr && gdb_stderr->fd () == -1", as we do in >> + sig_write. */ >> + scoped_restore restore_stderr >> + = make_scoped_restore (redirectable_stderr (), nullptr); >> + SELF_CHECK (gdb_stderr != nullptr); >> + SELF_CHECK (gdb_stderr->fd () == -1); >> + } >> } >> >> } /* namespace file*/ >> >> base-commit: 8ee110dee1942a20fd206c7107a027e9b04ba56e >> -- >> 2.51.0 >