Adding options for redefining symbols

[Ivan Morén <skogshjort at gmail dot com>]

Hi! This is regarding a patch (attached) to objcopy that might be of interest.

I found myself in need of redefining only the symbols that correspond
to defined functions, not the ones that are undefined declarations.
The use case is redefining symbols to fill the gap of an undefined
function call, and then being able to redefine the symbol back without
affecting the function call symbol.

I wrote a patch for this, thereby extending objcopy with two new options:
    "--redefine-defined-sym <old>=<new>"
    "--redefine-undefined-sym <old>=<new>"
These correspond to only redefining symbols <old> that are defined or
undefined respectively, instead of both.

I would very much like some broad feedback on the following before I
go from hacking a few lines on a fork for a private project to
possibly embarking on a journey of learning how to contribute to FOSS:

    - Is this kind of feature a feasable candidate?
    - Is this feature okay in scope or should it be part of some other redesign?
    - Setting redefine_mode uses switch fallthroughs, is this a
readability no-no?
    - Do I need to find similar alternatives for "--redefine-syms=<filename>"?

Example usage, first redefining for using implementation_a, then
changing the same object file to instead use implementation_b:

    $ objcopy --redefine-sym implementation_a=do_work
    $ objcopy --redefine-defined-sym do_work=implementation_a
--redefine-sym implementation_b=do_work

Hugs,
Ivan Morén

diff --git a/binutils/objcopy.c b/binutils/objcopy.c
index 28b9d3bf92..639b7c8749 100644
--- a/binutils/objcopy.c
+++ b/binutils/objcopy.c
@@ -59,6 +59,7 @@ struct redefine_node
 {
   char *source;
   char *target;
+  char mode;
 };
 
 struct addsym_node
@@ -339,6 +340,8 @@ enum command_line_switch
   OPTION_PURE,
   OPTION_READONLY_TEXT,
   OPTION_REDEFINE_SYM,
+  OPTION_REDEFINE_DEFINED_SYM,
+  OPTION_REDEFINE_UNDEFINED_SYM,
   OPTION_REDEFINE_SYMS,
   OPTION_REMOVE_LEADING_CHAR,
   OPTION_REMOVE_RELOCS,
@@ -470,6 +473,8 @@ static struct option copy_options[] =
   {"pure", no_argument, 0, OPTION_PURE},
   {"readonly-text", no_argument, 0, OPTION_READONLY_TEXT},
   {"redefine-sym", required_argument, 0, OPTION_REDEFINE_SYM},
+  {"redefine-defined-sym", required_argument, 0, OPTION_REDEFINE_DEFINED_SYM},
+  {"redefine-undefined-sym", required_argument, 0, OPTION_REDEFINE_UNDEFINED_SYM},
   {"redefine-syms", required_argument, 0, OPTION_REDEFINE_SYMS},
   {"remove-leading-char", no_argument, 0, OPTION_REMOVE_LEADING_CHAR},
   {"remove-section", required_argument, 0, 'R'},
@@ -535,7 +540,7 @@ static void get_sections (bfd *, asection *, void *);
 static int compare_section_lma (const void *, const void *);
 static void mark_symbols_used_in_relocations (bfd *, asection *, void *);
 static bfd_boolean write_debugging_info (bfd *, void *, long *, asymbol ***);
-static const char *lookup_sym_redefinition (const char *);
+static const char *lookup_sym_redefinition (const char *, const char);
 static const char *find_section_rename (const char *, flagword *);
 
 ATTRIBUTE_NORETURN static void
@@ -1510,7 +1515,7 @@ filter_symbols (bfd *abfd, bfd *obfd, asymbol **osyms,
 	{
 	  char *new_name;
 
-	  new_name = (char *) lookup_sym_redefinition (name);
+	  new_name = (char *) lookup_sym_redefinition (name, undefined ? 2 : 1);
 	  if (new_name == name
 	      && (flags & BSF_SECTION_SYM) != 0)
 	    new_name = (char *) find_section_rename (name, NULL);
@@ -1690,26 +1695,28 @@ filter_symbols (bfd *abfd, bfd *obfd, asymbol **osyms,
 /* Find the redefined name of symbol SOURCE.  */
 
 static const char *
-lookup_sym_redefinition (const char *source)
+lookup_sym_redefinition (const char *source, const char undefined_status)
 {
-  struct redefine_node key_node = {(char *) source, NULL};
+  struct redefine_node key_node = {(char *) source, NULL, 0};
   struct redefine_node *redef_node
     = (struct redefine_node *) htab_find (redefine_specific_htab, &key_node);
 
-  return redef_node == NULL ? source : redef_node->target;
+  return ((redef_node != NULL && redef_node->mode ^ undefined_status)
+          ? redef_node->target : source);
 }
 
 /* Insert a node into symbol redefine hash tabel.  */
 
 static void
 add_redefine_and_check (const char *cause, const char *source,
-			const char *target)
+			const char *target, const char mode)
 {
   struct redefine_node *new_node
     = (struct redefine_node *) xmalloc (sizeof (struct redefine_node));
 
   new_node->source = strdup (source);
   new_node->target = strdup (target);
+  new_node->mode   = mode;
 
   if (htab_find (redefine_specific_htab, new_node) != HTAB_EMPTY_ENTRY)
     fatal (_("%s: Multiple redefinition of symbol \"%s\""),
@@ -1810,7 +1817,7 @@ add_redefine_syms_file (const char *filename)
 	end_of_line:
 	  /* Append the redefinition to the list.  */
 	  if (buf[0] != '\0')
-	    add_redefine_and_check (filename, &buf[0], &buf[outsym_off]);
+	    add_redefine_and_check (filename, &buf[0], &buf[outsym_off], 0);
 
 	  lineno++;
 	  len = 0;
@@ -4805,10 +4812,14 @@ copy_main (int argc, char *argv[])
   int c;
   struct stat statbuf;
   const bfd_arch_info_type *input_arch = NULL;
+  char redefine_mode = 0;
 
   while ((c = getopt_long (argc, argv, "b:B:i:I:j:K:MN:s:O:d:F:L:G:R:SpgxXHhVvW:wDU",
 			   copy_options, (int *) 0)) != EOF)
     {
+      /* Zero means redefine both defined and undefined symbols.
+         One means only redefine undefined symbols.
+         Two means only redefine defined symbols.  */
       switch (c)
 	{
 	case 'b':
@@ -5208,6 +5219,12 @@ copy_main (int argc, char *argv[])
 	  remove_leading_char = TRUE;
 	  break;
 
+	case OPTION_REDEFINE_DEFINED_SYM:
+	  redefine_mode ^= 2 | 1;
+	  __attribute__((fallthrough));
+	case OPTION_REDEFINE_UNDEFINED_SYM:
+	  redefine_mode ^= 1;
+	  __attribute__((fallthrough));
 	case OPTION_REDEFINE_SYM:
 	  {
 	    /* Insert this redefinition onto redefine_specific_htab.  */
@@ -5231,8 +5248,9 @@ copy_main (int argc, char *argv[])
 	    target = (char *) xmalloc (len + 1);
 	    strcpy (target, nextarg);
 
-	    add_redefine_and_check ("--redefine-sym", source, target);
+	    add_redefine_and_check ("--redefine-sym", source, target, redefine_mode);
 
+        redefine_mode = 0;
 	    free (source);
 	    free (target);
 	  }