[PATCH] arc/bfd: Fix segfault from debug code, and disable debug by default

Andrew Burgess andrew.burgess@embecosm.com
Wed Jul 13 20:41:00 GMT 2016


This patch proposes to revert a recent commit b9316f59852ff821cf621a
(Enable relocation overflow messages by default):

  https://sourceware.org/ml/binutils/2016-07/msg00074.html

Sorry that I did not see this patch previously otherwise I would have
raised my objections earlier.  My objections are:

  1. Should be using the einfo / info callbacks on the bfd_link_info
     object to print messages, not calling fprintf to stderr from
     within the bfd library.

  2. Messages are not wrapped in _( ... ) for internationalisation.

  3. Leaving the macro being called PR_DEBUG is not nice if it is now
     on all the time, if we still want to use a macro at all then it
     should be renamed to something like RELOC_OVERFLOW_MESSAGE, or
     similar.

  4. Some of the message are not really production ready (in my
     opinion), for example: "Relocation overflows !!!!".  I've fixed
     this in the patch below.

  5. Some of this code causes ld to segfault.  Fixed in the patch
     below.

I've fixed 4 and 5, these are the easier ones.  I've not addressed 1,
2 or 3.  Ideally I'd prefer to just silence the debug again by
default, and if it's really, really wanted, then it can be enabled
properly again later.

Feedback welcome,

Thanks,
Andrew


---

The PR_DEBUG macro in arc-elf32.c file prints information about
relocation errors.  There are a few problems with this macro; first, the
printing is done using fprintf to stderr, rather than using the
einfo/info callbacks on the bfd_link_info object.  Second, none of the
messages are wrapped in _("...") to allow for internationalisation,
third, one of the debug messages causes a segfault, and finally (and
this is more opinion) some of the messages should be improved before
being put into production.

This commit fixes the segfault, and improves one of the
messages (removing a "!!!").  I've also turned the debug off for now.
If there's a real desire to have this being produced all of the time
then the internationalisation and use of einfo/info can be fixed later.

bfd/ChangeLog:

	* elf32-arc.c (PR_DEBUG): Only define to fprintf when
	ARC_ENABLE_DEBUG is defined.
	(debug_arc_reloc): Handle symbols that are not from an input file.
	(arc_do_relocation): Remove excessive exclamation points.
---
 bfd/ChangeLog   |  7 +++++++
 bfd/elf32-arc.c | 12 ++++++++----
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/bfd/elf32-arc.c b/bfd/elf32-arc.c
index ab27d4a..c58013d 100644
--- a/bfd/elf32-arc.c
+++ b/bfd/elf32-arc.c
@@ -29,11 +29,10 @@
 #include "opcode/arc.h"
 #include "arc-plt.h"
 
-#define PR_DEBUG(fmt, args...) fprintf (stderr, fmt, ##args)
-
 /* #define ARC_ENABLE_DEBUG 1  */
 #ifndef ARC_ENABLE_DEBUG
 #define ARC_DEBUG(...)
+#define PR_DEBUG(fmt, args...)
 #else
 static char *
 name_for_global_symbol (struct elf_link_hash_entry *h)
@@ -45,6 +44,7 @@ name_for_global_symbol (struct elf_link_hash_entry *h)
     return h->root.root.string;
 }
 #define ARC_DEBUG(args...) fprintf (stderr, ##args)
+#define PR_DEBUG(fmt, args...) fprintf (stderr, fmt, ##args)
 #endif
 
 
@@ -669,7 +669,11 @@ debug_arc_reloc (struct arc_relocation_data reloc_data)
 		   ((unsigned int) reloc_data.sym_section->output_section->vma));
 	}
       PR_DEBUG ( "\n");
-      PR_DEBUG ("  file: %s\n", reloc_data.sym_section->owner->filename);
+      if (reloc_data.sym_section->owner)
+        {
+          PR_DEBUG ("  file: %s\n",
+                    reloc_data.sym_section->owner->filename);
+        }
     }
   else
     {
@@ -917,7 +921,7 @@ arc_do_relocation (bfd_byte * contents,
 #define DEBUG_ARC_RELOC(A) debug_arc_reloc (A)
   if (flag != bfd_reloc_ok)
     {
-      PR_DEBUG ( "Relocation overflows !!!!\n");
+      PR_DEBUG ("Relocation overflows:\n");
 
       DEBUG_ARC_RELOC (reloc_data);
 
-- 
2.5.1



More information about the Binutils mailing list