From patchwork Fri Sep 8 10:54:29 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 22760 Received: (qmail 48271 invoked by alias); 8 Sep 2017 10:54:57 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 47850 invoked by uid 89); 8 Sep 2017 10:54:56 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.4 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, RCVD_IN_SORBS_SPAM, SPF_PASS autolearn=ham version=3.3.2 spammy=H*MI:andrew, sk:fputs_u, Seeing X-HELO: mail-wr0-f179.google.com Received: from mail-wr0-f179.google.com (HELO mail-wr0-f179.google.com) (209.85.128.179) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 08 Sep 2017 10:54:51 +0000 Received: by mail-wr0-f179.google.com with SMTP id v109so4024018wrc.1 for ; Fri, 08 Sep 2017 03:54:51 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=ORc3QhOd4BS/R0Nogq6noTAucLGtiDRAG6kfgPl1aW4=; b=BT84rYRp6zG5QLAXaKJXYjwmI5MB4SixWbGIMfff3GuPjf6Hyv07CuMLOprfTHoOCn C2sYEuZCKCB0Rp9qmlyaxfDpFjjOk9/dJIO9nL65wTPrxLCW2s6bAc7SI3Te/pbqcZ/t vqd6G7pdob1S45u/2GeXMcmuYsTFYtkbKr+ieZV0i3F08SXvspzue7vcTwvtdjygtyKV uLoIvBDaE+MsTb7tYPWZzzHVTUEdyo7mN6oYHfyDpVFAYIgtN4S2Xa9kv3PNEbcXU7RE 0HBxA7+PT0lDmbhkWmOlBM6R/jJ1ZbDoCdnnbwymM+uDfzufVY94GoleS7WJ+69b5RW+ lErw== X-Gm-Message-State: AHPjjUgsMF5/hOK5OoGxg8nu5NEpYjrr7ScpIYwostLHVbT3ykCEPKAy rH9lgr/zUmUuutIqEg0= X-Google-Smtp-Source: ADKCNb5l29NCe/TBbgZatAOzGbqe0xKpCLs8eEiRDfoU29RXpfa8QySTDolKQvJGc3N03m9uxCjm0Q== X-Received: by 10.223.134.248 with SMTP id 53mr1654186wry.57.1504868088846; Fri, 08 Sep 2017 03:54:48 -0700 (PDT) Received: from localhost (vpn-konference.ms.mff.cuni.cz. [195.113.20.101]) by smtp.gmail.com with ESMTPSA id m9sm1187236wrf.51.2017.09.08.03.54.47 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 08 Sep 2017 03:54:48 -0700 (PDT) From: Andrew Burgess To: gdb-patches@sourceware.org Cc: Andrew Burgess Subject: [PATCH] gdb: Remove check for gdb_stderr == NULL Date: Fri, 8 Sep 2017 11:54:29 +0100 Message-Id: <20170908105429.26563-1-andrew.burgess@embecosm.com> X-IsSubscribed: yes Recent changes made gdb_stderr a macro: #define gdb_stderr (*current_ui_gdb_stderr_ptr ()) and current_ui_gdb_stderr_ptr return this: ¤t_ui->m_gdb_stderr The problem is that this is undefined if current_ui is NULL, which can happen early on during gdb start up. If we run into an error during early gdb start up then we write the error message to gdb_stderr. However, if we are too early during the start up then current_ui is NULL, and using the gdb_stderr macro triggers undefined behaviour. We try to avoid this using a check 'gdb_stderr == NULL' which was fine before the recent changes, but now, still triggers undefined behaviour. A better check is instead 'current_ui == NULL' which is what I use in this patch. Triggering this failure is pretty hard, most of the really early errors are only triggered if pretty basic things are not as expected, for example, if the default signal handlers are not as expected. Seeing one of these errors trigger usually means that someone working on gdb has made an incorrect change. Still, the errors are present in gdb, and should we ever trigger one it would be nice if gdb didn't crash. For testing this change I've been applying this patch which adds an unconditional error into a function called early during gdb start up. Later in the same function is a real error call which, in some circumstances could be triggered: ## START ## diff --git a/gdb/common/signals-state-save-restore.c b/gdb/common/signals-state-save-restore.c index d11a9ae006c..d75ba70f894 100644 --- a/gdb/common/signals-state-save-restore.c +++ b/gdb/common/signals-state-save-restore.c @@ -37,6 +37,9 @@ static sigset_t original_signal_mask; void save_original_signals_state (void) { + + internal_error (__FILE__, __LINE__, "example error"); + #ifdef HAVE_SIGACTION int i; int res; ## END ## gdb/ChangeLog: * utils.c (abort_with_message): Don't compare gdb_stderr to NULL, check current_ui instead. (internal_vproblem): Likewise. --- gdb/ChangeLog | 6 ++++++ gdb/utils.c | 4 ++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/gdb/utils.c b/gdb/utils.c index f2da2df60f5..1c2bb5b8db9 100644 --- a/gdb/utils.c +++ b/gdb/utils.c @@ -335,7 +335,7 @@ error_stream (const string_file &stream) static void ATTRIBUTE_NORETURN abort_with_message (const char *msg) { - if (gdb_stderr == NULL) + if (current_ui == NULL) fputs (msg, stderr); else fputs_unfiltered (msg, gdb_stderr); @@ -497,7 +497,7 @@ internal_vproblem (struct internal_problem *problem, } /* Fall back to abort_with_message if gdb_stderr is not set up. */ - if (gdb_stderr == NULL) + if (current_ui == NULL) { fputs (reason, stderr); abort_with_message ("\n");