Check return value of bfd_init

Message ID 875zxoiulq.fsf@tromey.com
State New, archived
Headers

Commit Message

Tom Tromey Oct. 26, 2018, 3:41 p.m. UTC
  >>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:

Tom> Moving the call to bfd_init below the call to "new ui" makes it work.

Like so.

Tom

commit 2d0b2f3697a1f289e843f047697f252bf653d4db
Author: Tom Tromey <tom@tromey.com>
Date:   Thu Oct 25 09:00:52 2018 -0600

    Check return value of bfd_init
    
    Alan recently added a way for BFD library users to check whether they
    were in fact loading a compatible version of BFD:
    
    https://sourceware.org/ml/binutils/2018-10/msg00198.html
    
    It seemed reasonable to me that gdb should do this check as well, in
    case someone is dynamically linking against BFD.
    
    Simon pointed out that an earlier version of the patch would cause a
    gdb crash if the test failed.  This version works around this by
    lowering the call to bfd_init and adding a comment explaining where
    'error' can safely be called in captured_main_1.
    
    gdb/ChangeLog
    2018-10-25  Tom Tromey  <tom@tromey.com>
    
            * main.c (captured_main_1): Check return value of bfd_init.
  

Comments

Simon Marchi Oct. 26, 2018, 6:31 p.m. UTC | #1
On 2018-10-26 11:41, Tom Tromey wrote:
>>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:
> 
> Tom> Moving the call to bfd_init below the call to "new ui" makes it 
> work.
> 
> Like so.
> 
> Tom
> 
> commit 2d0b2f3697a1f289e843f047697f252bf653d4db
> Author: Tom Tromey <tom@tromey.com>
> Date:   Thu Oct 25 09:00:52 2018 -0600
> 
>     Check return value of bfd_init
> 
>     Alan recently added a way for BFD library users to check whether 
> they
>     were in fact loading a compatible version of BFD:
> 
>     https://sourceware.org/ml/binutils/2018-10/msg00198.html
> 
>     It seemed reasonable to me that gdb should do this check as well, 
> in
>     case someone is dynamically linking against BFD.
> 
>     Simon pointed out that an earlier version of the patch would cause 
> a
>     gdb crash if the test failed.  This version works around this by
>     lowering the call to bfd_init and adding a comment explaining where
>     'error' can safely be called in captured_main_1.
> 
>     gdb/ChangeLog
>     2018-10-25  Tom Tromey  <tom@tromey.com>
> 
>             * main.c (captured_main_1): Check return value of bfd_init.
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 61dc039d4fe..a90c2978185 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,7 @@
> +2018-10-25  Tom Tromey  <tom@tromey.com>
> +
> +	* main.c (captured_main_1): Check return value of bfd_init.
> +
>  2018-10-25  Andrew Burgess  <andrew.burgess@embecosm.com>
> 
>  	* python/py-function.c (convert_values_to_python): Return
> diff --git a/gdb/main.c b/gdb/main.c
> index 8709357e924..bf6a58180a0 100644
> --- a/gdb/main.c
> +++ b/gdb/main.c
> @@ -506,7 +506,6 @@ captured_main_1 (struct captured_main_args 
> *context)
>    textdomain (PACKAGE);
>  #endif
> 
> -  bfd_init ();
>    notice_open_fds ();
> 
>    saved_command_line = (char *) xstrdup ("");
> @@ -517,12 +516,17 @@ captured_main_1 (struct captured_main_args 
> *context)
>    setvbuf (stderr, NULL, _IONBF, BUFSIZ);
>  #endif
> 
> +  /* Note: `error' cannot be called before this point, because the
> +     caller will crash when trying to print the exception.  */
>    main_ui = new ui (stdin, stdout, stderr);
>    current_ui = main_ui;
> 
>    gdb_stdtargerr = gdb_stderr;	/* for moment */
>    gdb_stdtargin = gdb_stdin;	/* for moment */
> 
> +  if (bfd_init () == BFD_INIT_MAGIC)
> +    error (_("fatal error: libbfd ABI mismatch"));

The comparison operator is the wrong one (== instead of !=), you 
probably left it like this after testing.  LGTM with that fixed.

Simon
  
Tom Tromey Oct. 26, 2018, 7:47 p.m. UTC | #2
>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:

Simon> The comparison operator is the wrong one (== instead of !=), you
Simon> probably left it like this after testing.  LGTM with that fixed.

This patch is cursed.

Tom
  

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 61dc039d4fe..a90c2978185 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,7 @@ 
+2018-10-25  Tom Tromey  <tom@tromey.com>
+
+	* main.c (captured_main_1): Check return value of bfd_init.
+
 2018-10-25  Andrew Burgess  <andrew.burgess@embecosm.com>
 
 	* python/py-function.c (convert_values_to_python): Return
diff --git a/gdb/main.c b/gdb/main.c
index 8709357e924..bf6a58180a0 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -506,7 +506,6 @@  captured_main_1 (struct captured_main_args *context)
   textdomain (PACKAGE);
 #endif
 
-  bfd_init ();
   notice_open_fds ();
 
   saved_command_line = (char *) xstrdup ("");
@@ -517,12 +516,17 @@  captured_main_1 (struct captured_main_args *context)
   setvbuf (stderr, NULL, _IONBF, BUFSIZ);
 #endif
 
+  /* Note: `error' cannot be called before this point, because the
+     caller will crash when trying to print the exception.  */
   main_ui = new ui (stdin, stdout, stderr);
   current_ui = main_ui;
 
   gdb_stdtargerr = gdb_stderr;	/* for moment */
   gdb_stdtargin = gdb_stdin;	/* for moment */
 
+  if (bfd_init () == BFD_INIT_MAGIC)
+    error (_("fatal error: libbfd ABI mismatch"));
+
 #ifdef __MINGW32__
   /* On Windows, argv[0] is not necessarily set to absolute form when
      GDB is found along PATH, without which relocation doesn't work.  */