From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id 2QJjJkSi52k2fzAAWB0awg (envelope-from ) for ; Tue, 21 Apr 2026 12:13:56 -0400 Authentication-Results: simark.ca; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=Hp2vB5FE; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 977D81E0C3; Tue, 21 Apr 2026 12:13:56 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-25) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-3.4 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H2,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 CE0D61E093 for ; Tue, 21 Apr 2026 12:13:55 -0400 (EDT) Received: from vm01.sourceware.org (localhost [127.0.0.1]) by sourceware.org (Postfix) with ESMTP id 6B9744BA23CA for ; Tue, 21 Apr 2026 16:13:55 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 6B9744BA23CA Authentication-Results: sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=Hp2vB5FE Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTP id AD4944BA23CA for ; Tue, 21 Apr 2026 16:13:28 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org AD4944BA23CA Authentication-Results: sourceware.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org AD4944BA23CA Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1776788008; cv=none; b=NWGaUa0920OgMAp749WahyPZoOVdnIa5D2xVG07YPj9MRJOTJ4YWx1S3P2IFDlo/U7wB9dIGdF4S8PNy/xh7pSNWICXNkAKb18Z+Lwo8bGyZPZmJoIC5hQJMzKQwW2W8AD2SGwRfnpYFcKzWf/2CE49v6NSf6b6Tdy9Qam8ARZs= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1776788008; c=relaxed/simple; bh=PdyQW4t8j5tc00GJVPCSCE2m0CNrPIi0Ab+0euccJUY=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=RBo8JYwgTb+M3a1XcUnXHetZc59WpXzuKnWt22u4fdXXRuMpoRLrmxJdyRTqj7Islxfm0ZDynwXGBnTf9vyphM36Ksa2nZ/FtklVf2vXtQ67Ch2dMRNUFzoAuQIcqwgE+SOJXCGsIVuXo/A/NVXLGEfYcup4czAQD5lB63Kapk0= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org AD4944BA23CA DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1776788008; h=from:from:reply-to:subject:subject: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=zW79zLSQzhXd0aruzB92ehLDgEPrDEkuBcom9kGF1Lk=; b=Hp2vB5FEbkOhm+X6NBPnGQzq4G/i6SZQaOm/2dtjAQ1fb186QcaRNJiLMcGqG0L7X62zRH UKnAaL+v4vZt0AUfh3cYQrMAVXKp+1J4olYzaTzDbxcWZWm31dyRSPMb50q9WV4ZllVcCm JYadxNHrzgFjxg91ubEXqX+iDuHSV1A= Received: from mail-wm1-f69.google.com (mail-wm1-f69.google.com [209.85.128.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-508-I0sJ6C3zP_OgPfqBuG1aFw-1; Tue, 21 Apr 2026 12:13:25 -0400 X-MC-Unique: I0sJ6C3zP_OgPfqBuG1aFw-1 X-Mimecast-MFC-AGG-ID: I0sJ6C3zP_OgPfqBuG1aFw_1776788005 Received: by mail-wm1-f69.google.com with SMTP id 5b1f17b1804b1-488cc31ea57so35872875e9.3 for ; Tue, 21 Apr 2026 09:13:25 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1776788004; x=1777392804; h=mime-version:message-id:date:references:in-reply-to:subject:to:from :x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=zW79zLSQzhXd0aruzB92ehLDgEPrDEkuBcom9kGF1Lk=; b=lURhFuMFVlpnjPf9jpMYLe9ovRtuUgFPxk98SvnMUd2FHAE74p0fcwTUL82n2f04aQ 31vk3At03gBwZTo8pQVWhpjoQT2Wzzz1c7B0rOs8AeJ3yrdjkgJkRSqlRqTOLylQL0U2 2vdM6Q/tNmiYuLgrrGK6iUBi25yA37KqR5xoot5SQe1NNu3SNye2/s1SNI+TaH+55CQN ESVTd8t+ugXxVqSIprbpI5YstGc06DcvH7y+gLgHQ8f0smneHQcUEi0MJR+NTT6mA4F6 uMEDJWq/v4ZNxJQzQy5RfmEQaDnUIW9R1FuFvBqq7tpeU/qqw2yYOK8tYLAMg/Nibcqr RdEQ== X-Forwarded-Encrypted: i=1; AFNElJ8MIIYu7I5+TB/puEn2U1U1MLUaVXobjXG8FIxaDKVUWxTtv0SUzpldlaJ3Dfg7CZ056idGvHEVV3zlGQ==@sourceware.org X-Gm-Message-State: AOJu0YwWmESmKYzGxUzAXXSbDAu4Lor2b0HvrYkxctZnq0tQgpT5JI7X 2/qn8n0MOap2G/r151AUQ2ce4qHtHGhX9+QKXv4y02ZVUEEdXOQbVwODcZWS8qBFGFqW+hhAPki G903tFMWrfjL0NsHb+X2VmEqxmpXnTcgqV10CLRyuhD8crMYTgsyePV7GgwNKydbxtMTxuBI= X-Gm-Gg: AeBDievKRcgB0Vf6TuB5ld68lx3XZrKVVHZXRdeHoVuy9108XCbBUl4IrpIIyHVZAiZ 5okBFhFFF9wvZsTNKgz99p/9FJxZC7PDfSRfl2pE4iafzng3CYFT2bxbCkqNSY/urS/cOXDnuTv Jmwov1RBMIdaptpP4cvXeIVElY89njGTBAtvH4oSky3v3T2M5X/o5CV2yyG273NlNFs0j7YsiLo cvanwU+On/F1kW+0r2wbDsPV8JcDiPhafNLiup2knoltlCv3BW3sA99piVgq3v6snMKkN3986AY CU7MpmsbA5f8Maw8k01vDkRLyPu2qZcAXAP44RbQoxps9iJkVi6P9YYg9CCtl1eez/VJEhY7f+s Ovkgz1AkznI2OlmLNYtjbBw4pNo0= X-Received: by 2002:a05:600c:3150:b0:488:9fb7:376d with SMTP id 5b1f17b1804b1-488fb792ce4mr276188255e9.28.1776788003059; Tue, 21 Apr 2026 09:13:23 -0700 (PDT) X-Received: by 2002:a05:600c:3150:b0:488:9fb7:376d with SMTP id 5b1f17b1804b1-488fb792ce4mr276187865e9.28.1776788002428; Tue, 21 Apr 2026 09:13:22 -0700 (PDT) Received: from localhost ([31.111.84.232]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-488fb7aa593sm119228515e9.24.2026.04.21.09.13.21 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 21 Apr 2026 09:13:21 -0700 (PDT) From: Andrew Burgess To: Tom de Vries , gdb-patches@sourceware.org Subject: Re: [PATCH v2] [gdb] Fix segfault in sig_write In-Reply-To: <20260402084914.946223-1-tdevries@suse.de> References: <20260402084914.946223-1-tdevries@suse.de> Date: Tue, 21 Apr 2026 17:13:20 +0100 Message-ID: <87340o2swv.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: o5BlfGG9LALUYx6dpMpVslisD02QcaWtGF5AMIDC5_Q_1776788005 X-Mimecast-Originator: redhat.com Content-Type: text/plain 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 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. It would be nice if the commit message explained *why* there's a nullptr there. 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 > > 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. 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. 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