[binutils-gdb] gdb: Remove check for gdb_stderr == NULL

[Andrew Burgess <aburgess at sourceware dot org>]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=72542b8ee6558677bed21dd880b68dc2e541c9c7

commit 72542b8ee6558677bed21dd880b68dc2e541c9c7
Author: Andrew Burgess <andrew.burgess@embecosm.com>
Date:   Sun Aug 6 21:45:08 2017 +0100

    gdb: Remove check for gdb_stderr == NULL
    
    Recent changes made gdb_stderr a macro:
    
      #define gdb_stderr (*current_ui_gdb_stderr_ptr ())
    
    and current_ui_gdb_stderr_ptr return this:
    
       &current_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.

Diff:
---
 gdb/ChangeLog | 6 ++++++
 gdb/utils.c   | 4 ++--
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index e8c69f5..1781ddd 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2017-09-10  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* utils.c (abort_with_message): Don't compare gdb_stderr to NULL,
+	check current_ui instead.
+	(internal_vproblem): Likewise.
+
 2017-09-09  Simon Marchi  <simon.marchi@ericsson.com>
 
 	* thread.c (print_thread_info_1): Remove unnecessary calls to
diff --git a/gdb/utils.c b/gdb/utils.c
index a7a97e2..b709af9 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -316,7 +316,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);
@@ -478,7 +478,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");