* [PATCH] Check return value of bfd_init @ 2018-10-25 15:03 Tom Tromey 2018-10-25 15:25 ` Simon Marchi 0 siblings, 1 reply; 5+ messages in thread From: Tom Tromey @ 2018-10-25 15:03 UTC (permalink / raw) To: gdb-patches; +Cc: Tom Tromey 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. Tested by rebuilding and then starting the resulting gdb. gdb/ChangeLog 2018-10-25 Tom Tromey <tom@tromey.com> * main.c (captured_main_1): Check return value of bfd_init. --- gdb/ChangeLog | 4 ++++ gdb/main.c | 3 ++- 2 files changed, 6 insertions(+), 1 deletion(-) 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..96def3080eb 100644 --- a/gdb/main.c +++ b/gdb/main.c @@ -506,7 +506,8 @@ captured_main_1 (struct captured_main_args *context) textdomain (PACKAGE); #endif - bfd_init (); + if (bfd_init () != BFD_INIT_MAGIC) + error (_("fatal error: libbfd ABI mismatch")); notice_open_fds (); saved_command_line = (char *) xstrdup (""); -- 2.17.2 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Check return value of bfd_init 2018-10-25 15:03 [PATCH] Check return value of bfd_init Tom Tromey @ 2018-10-25 15:25 ` Simon Marchi [not found] ` <87efcehwlx.fsf@tromey.com> 0 siblings, 1 reply; 5+ messages in thread From: Simon Marchi @ 2018-10-25 15:25 UTC (permalink / raw) To: Tom Tromey, gdb-patches [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2445 bytes --] On 2018-10-25 11:03 a.m., Tom Tromey wrote: > 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. Tested by rebuilding > and then starting the resulting gdb. > > gdb/ChangeLog > 2018-10-25 Tom Tromey <tom@tromey.com> > > * main.c (captured_main_1): Check return value of bfd_init. > --- > gdb/ChangeLog | 4 ++++ > gdb/main.c | 3 ++- > 2 files changed, 6 insertions(+), 1 deletion(-) > > 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..96def3080eb 100644 > --- a/gdb/main.c > +++ b/gdb/main.c > @@ -506,7 +506,8 @@ captured_main_1 (struct captured_main_args *context) > textdomain (PACKAGE); > #endif > > - bfd_init (); > + if (bfd_init () != BFD_INIT_MAGIC) > + error (_("fatal error: libbfd ABI mismatch")); > notice_open_fds (); > > saved_command_line = (char *) xstrdup (""); > It doesn't work to call error that early. Try to reverse the condition and start gdb, you'll get: $ ./gdb ASAN:SIGSEGV ================================================================= ==12947==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000090 (pc 0x0000013a7a2f bp 0x7ffcbd048510 sp 0x7ffcbd048460 T0) #0 0x13a7a2e in gdb_main(captured_main_args*) /home/emaisin/src/binutils-gdb/gdb/main.c:1194 #1 0xb27098 in main /home/emaisin/src/binutils-gdb/gdb/gdb.c:32 #2 0x7ff24431482f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f) #3 0xb26ef8 in _start (/home/emaisin/build/binutils-gdb/gdb/gdb+0xb26ef8) AddressSanitizer can not provide additional info. SUMMARY: AddressSanitizer: SEGV /home/emaisin/src/binutils-gdb/gdb/main.c:1194 gdb_main(captured_main_args*) ==12947==ABORTING So fprintf to stderr and exit instead? Simon \x16º&Öéj×!zÊÞ¶êç×|ë™b²Ö«r\x18\x1dn–r\x17¬ ^ permalink raw reply [flat|nested] 5+ messages in thread
[parent not found: <87efcehwlx.fsf@tromey.com>]
* Re: [PATCH] Check return value of bfd_init [not found] ` <87efcehwlx.fsf@tromey.com> @ 2018-10-26 15:41 ` Tom Tromey 2018-10-26 18:31 ` Simon Marchi 0 siblings, 1 reply; 5+ messages in thread From: Tom Tromey @ 2018-10-26 15:41 UTC (permalink / raw) To: Tom Tromey; +Cc: Simon Marchi, gdb-patches >>>>> "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")); + #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. */ ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Check return value of bfd_init 2018-10-26 15:41 ` Tom Tromey @ 2018-10-26 18:31 ` Simon Marchi 2018-10-26 19:48 ` Tom Tromey 0 siblings, 1 reply; 5+ messages in thread From: Simon Marchi @ 2018-10-26 18:31 UTC (permalink / raw) To: Tom Tromey; +Cc: Simon Marchi, gdb-patches 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Check return value of bfd_init 2018-10-26 18:31 ` Simon Marchi @ 2018-10-26 19:48 ` Tom Tromey 0 siblings, 0 replies; 5+ messages in thread From: Tom Tromey @ 2018-10-26 19:48 UTC (permalink / raw) To: Simon Marchi; +Cc: Tom Tromey, Simon Marchi, gdb-patches >>>>> "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 ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-10-26 19:48 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-25 15:03 [PATCH] Check return value of bfd_init Tom Tromey
2018-10-25 15:25 ` Simon Marchi
[not found] ` <87efcehwlx.fsf@tromey.com>
2018-10-26 15:41 ` Tom Tromey
2018-10-26 18:31 ` Simon Marchi
2018-10-26 19:48 ` Tom Tromey
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox