This is the mail archive of the mailing list for the binutils project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

RFA: provide a way for a BFD program to notice assertions. Use it in gld.

If you build current HEAD binutils for arm-linux-gnueabi and use
it to build the gcc-4.7 branch, you'll likely *miss* the
following BFD assertions while linking, unless
you look in the build-log, because the linker *continues and
exits without error, creating output*:

/tmp/somewhere/o/ld/ld-new: BFD (GNU Binutils) assertion fail /tmp/somewhere/src/bfd/elf32-arm.c:12259
/tmp/somewhere/o/ld/ld-new: BFD (GNU Binutils) assertion fail /tmp/somewhere/src/bfd/elf32-arm.c:12259
/tmp/somewhere/o/ld/ld-new: BFD (GNU Binutils) assertion fail /tmp/somewhere/src/bfd/elf32-arm.c:12259
/tmp/somewhere/o/ld/ld-new: BFD (GNU Binutils) assertion fail /tmp/somewhere/src/bfd/elf32-arm.c:12259

I hope you're worried, thinking: what are those?  Benevolent or
causing the DSO to fail in "interesting" ways when run?  (Film
at 11.)  The point is (for now) not the cause of the above
assertion, but the way BFD_ASSERT is handled in BFD: nothing
happens, besides the call to _bfd_error_handler which isn't
about handling the errors but rather the error messages.

To cause the calling program to abortively exit with an error
from within the BFD code, you'll have to call abort, except it's
redefined to _bfd_abort and doesn't really abort but call _exit
(EXIT_FAILURE) (see libbfd-in.h).  Brr.  Yes, I know a library
shouldn't abort on the caller and you shouldn't do that, but
apparently it seemed a a good idea at the time to add that piece
of...infrastructure.  The patch and the discussion are lost in
time in the list archives (libbfd-in.h:1.5, c:a 1999-09-11).  I
make no attempt to adjust any abort or _bfd_abort calls: my beef
is rather with the assertions that *don't* cause a non-error
exit for the calling program; here the linker.

Previous hackish attempts to just have BFD abort() on such
assertions have been rightly refused as per above, so here's an
attempt along the lines of _bfd_error_handler.  The BFD patches
change nothing in default behavior, they just add infrastructure
to set an assert handler.  See the ld patch for the effective
change.  I hope, with this patch I won't be the first person to
notice bfd assertions for ARM and high-profile source the next
time. ;)

The assertion handling could be folded into _bfd_error_handler
and making gld have a separate handler, but that would leave no
way for the calling program to see the assert difference other
than parsing the message output.  Besides, and the executive
reason, it'd mean forward-calling the variadic default function
from the variadic handler, which can't be done without other
infrastructure-changes to change bfd_error_handler_type
parameters to instead be passed a va_list or to add intermediate
bfd functions to that effect; much more invasive.  Better IMHO
to add and use a non-variadic handler type.

No test-case, since there's no way to generate a bfd assertion
for use in the linker test-suite.  Anyway, it worked for the bug
at hand: besides the above assertion messages, the linker now
exits with an error, creating no output.

Ok to commit?

	Make bfd asserts cause linker errors.
	* ldmain.c (default_bfd_assert_handler): New variable.
	(ld_bfd_assert_handler): New function.
	(main): Call bfd_set_assert_handler.

	Provide a way for programs to recognize BFD_ASSERT calls.
	* bfd.c (bfd_assert_handler_type): New API type.
	(bfd_set_assert_handler, bfd_get_assert_handler): New API functions.
	(_bfd_assert_handler): New variable.
	(_bfd_default_assert_handler): New function.
	(bfd_assert): Call _bfd_assert_handler, not _bfd_error_handler.
	* libbfd-in.h (_bfd_assert_handler): Declare.
	* libbfd.h, bfd-in2.h: Regenerate.

diff --git a/ld/ldmain.c b/ld/ldmain.c
index b2810a7..3bdaf4d 100644
--- a/ld/ldmain.c
+++ b/ld/ldmain.c
@@ -160,6 +160,8 @@ static struct bfd_link_callbacks link_callbacks =
+static bfd_assert_handler_type default_bfd_assert_handler;
 struct bfd_link_info link_info;
 static void
@@ -173,6 +175,17 @@ ld_cleanup (void)
     unlink_if_ordinary (output_filename);
+/* If there's a BFD assertion, we'll notice and exit with an error
+   unless otherwise instructed.  */
+static void
+ld_bfd_assert_handler (const char *fmt, const char *bfdver,
+		       const char *file, int line)
+  (*default_bfd_assert_handler) (fmt, bfdver, file, line);
+  config.make_executable = FALSE;
 main (int argc, char **argv)
@@ -199,6 +212,11 @@ main (int argc, char **argv)
   bfd_set_error_program_name (program_name);
+  /* We want to notice and fail on those nasty BFD assertions which are
+     likely to signal incorrect output being generated but otherwise may
+     leave no trace.  */
+  default_bfd_assert_handler = bfd_set_assert_handler (ld_bfd_assert_handler);
   xatexit (ld_cleanup);
   /* Set up the sysroot directory.  */

diff --git a/bfd/bfd.c b/bfd/bfd.c
index 7c14c7a..a1664b5 100644
--- a/bfd/bfd.c
+++ b/bfd/bfd.c
@@ -797,6 +797,88 @@ bfd_get_error_handler (void)
   return _bfd_error_handler;
+	BFD assert handler
+	If BFD finds an internal inconsistency, the bfd assert
+	handler is called with information on the BFD version, BFD
+	source file and line.  If this happens, most programs linked
+	against BFD are expected to want to exit with an error, or mark
+	the current BFD operation as failed, so it is recommended to
+	override the default handler, which just calls
+	_bfd_error_handler and continues.
+.typedef void (*bfd_assert_handler_type) (const char *bfd_formatmsg,
+.                                         const char *bfd_version,
+.                                         const char *bfd_file,
+.                                         int bfd_line);
+/* Note the use of bfd_ prefix on the parameter names above: we want to
+   show which one is the message and which is the version by naming the
+   parameters, but avoid polluting the program-using-bfd namespace as
+   the typedef is visible in the exported headers that the program
+   includes.  Below, it's just for consistency.  */
+static void
+_bfd_default_assert_handler (const char *bfd_formatmsg,
+			     const char *bfd_version,
+			     const char *bfd_file,
+			     int bfd_line)
+  (*_bfd_error_handler) (bfd_formatmsg, bfd_version, bfd_file, bfd_line);
+/* Similar to _bfd_error_handler, a program can decide to exit on an
+   internal BFD error.  We use a non-variadic type to simplify passing
+   on parameters to other functions, e.g. _bfd_error_handler.  */
+bfd_assert_handler_type _bfd_assert_handler = _bfd_default_assert_handler;
+	bfd_set_assert_handler
+	bfd_assert_handler_type bfd_set_assert_handler (bfd_assert_handler_type);
+	Set the BFD assert handler function.  Returns the previous
+	function.
+bfd_set_assert_handler (bfd_assert_handler_type pnew)
+  bfd_assert_handler_type pold;
+  pold = _bfd_assert_handler;
+  _bfd_assert_handler = pnew;
+  return pold;
+	bfd_get_assert_handler
+	bfd_assert_handler_type bfd_get_assert_handler (void);
+	Return the BFD assert handler function.
+bfd_get_assert_handler (void)
+  return _bfd_assert_handler;
@@ -942,8 +1024,8 @@ bfd_set_file_flags (bfd *abfd, flagword flags)
 bfd_assert (const char *file, int line)
-  (*_bfd_error_handler) (_("BFD %s assertion fail %s:%d"),
-			 BFD_VERSION_STRING, file, line);
+  (*_bfd_assert_handler) (_("BFD %s assertion fail %s:%d"),
+			  BFD_VERSION_STRING, file, line);
 /* A more or less friendly abort message.  In libbfd.h abort is
diff --git a/bfd/libbfd-in.h b/bfd/libbfd-in.h
index 640768e..45f0b0c 100644
--- a/bfd/libbfd-in.h
+++ b/bfd/libbfd-in.h
@@ -117,6 +117,7 @@ extern void *bfd_zmalloc2
 extern void _bfd_default_error_handler (const char *s, ...);
 extern bfd_error_handler_type _bfd_error_handler;
+extern bfd_assert_handler_type _bfd_assert_handler;
 /* These routines allocate and free things on the BFD's objalloc.  */
brgds, H-P

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]