Re: Fwd: [PATCH] arm: ensure symbol is a thumb symbol in new binutils

[Nick Clifton <nickc at redhat dot com>]

Hi Christophe,

> I have made the test on buildroot 2018.05-rc3 with bintutils 2.29.1 and 
> 2.30,
> for the two test my boot failed. The proposition of correction doesn't work.

Ho hum.  OK, please could you try out this attached patch instead.  It is part
of a larger patch to allow the behaviour of the assembler to be controlled by
a command line option, whose default is set at configure time.  I just want to
be sure that the "ADR-does-NOT-set-interworking" default option actually does
work before I complete the rest of the patch.

Cheers
  Nick


diff --git a/gas/config/tc-arm.c b/gas/config/tc-arm.c
index dbaf1627bb..a72084fa3c 100644
--- a/gas/config/tc-arm.c
+++ b/gas/config/tc-arm.c
@@ -145,6 +145,32 @@ static int fix_v4bx	     = FALSE;
 /* Warn on using deprecated features.  */
 static int warn_on_deprecated = TRUE;
 
+/* Customises the behaviour of the ADR and ADRL pseudo-ops when given a thumb
+   function pointer as an argument.  If this option is TRUE then the bottom
+   bit of the address stored into the destination register will be set.
+   Otherwise it will be left alone (and presumably will be clear).
+
+   This option and customised default behaviour are necessary because there
+   is no clear specification for the behaviour of ADR and ADRL on thumb
+   symbols in the ARM Reference Manual.  Versions of the binutils prior to
+   2.29 did not set the bit at all.  Versions 2.29 and 2.30 unconditionally
+   set the bit.  Versions 2.31 onwards have this customisation option.
+
+   Setting the interworking bit breaks the ARM Linux kernel.  But not setting
+   the bit breaks code like this:
+   
+     ADR R0,__testFnPtr
+     BLX R0
+
+   if __testFnPtr is a thumb function pointer, but works if it is an ARM
+   function pointer.  */
+#ifdef DEFAULT_THUMB_ADR_SETS_INTERWORKING
+static int thumb_adr_sets_interworking = TRUE;
+#else
+static int thumb_adr_sets_interworking = FALSE;
+#endif
+
+
 /* Understand CodeComposer Studio assembly syntax.  */
 bfd_boolean codecomposer_syntax = FALSE;
 
@@ -8419,11 +8445,12 @@ do_adr (void)
   inst.reloc.pc_rel = 1;
   inst.reloc.exp.X_add_number -= 8;
 
-  if (inst.reloc.exp.X_op == O_symbol
+  if (thumb_adr_sets_interworking
+      && inst.reloc.exp.X_op == O_symbol
       && inst.reloc.exp.X_add_symbol != NULL
       && S_IS_DEFINED (inst.reloc.exp.X_add_symbol)
       && THUMB_IS_FUNC (inst.reloc.exp.X_add_symbol))
-    inst.reloc.exp.X_add_number += 1;
+    inst.reloc.exp.X_add_number |= 1;
 }
 
 /* This is a pseudo-op of the form "adrl rd, label" to be converted
@@ -8443,11 +8470,12 @@ do_adrl (void)
   inst.size		       = INSN_SIZE * 2;
   inst.reloc.exp.X_add_number -= 8;
 
-  if (inst.reloc.exp.X_op == O_symbol
+  if (thumb_adr_sets_interworking
+      && inst.reloc.exp.X_op == O_symbol
       && inst.reloc.exp.X_add_symbol != NULL
       && S_IS_DEFINED (inst.reloc.exp.X_add_symbol)
       && THUMB_IS_FUNC (inst.reloc.exp.X_add_symbol))
-    inst.reloc.exp.X_add_number += 1;
+    inst.reloc.exp.X_add_number |= 1;
 }
 
 static void
@@ -25725,6 +25753,17 @@ struct arm_option_table arm_opts[] =
    &warn_on_deprecated, 0, NULL},
   {"mwarn-syms", N_("warn about symbols that match instruction names [default]"), (int *) (& flag_warn_syms), TRUE, NULL},
   {"mno-warn-syms", N_("disable warnings about symobls that match instructions"), (int *) (& flag_warn_syms), FALSE, NULL},
+#ifdef DEFAULT_THUMB_ADR_SETS_INTERWORKING
+  {"mthumb-adr-sets-interworking", N_("ADR sets interworking bit [default]"),
+   (int *) (& thumb_adr_sets_interworking), TRUE, NULL},
+  {"mno-thumb-adr-sets-interworking", N_("ADR does not set interworking bit"),
+   (int *) (& thumb_adr_sets_interworking), FALSE, NULL},
+#else
+  {"mthumb-adr-sets-interworking", N_("adr sets interworking bit"),
+   (int *) (& thumb_adr_sets_interworking), TRUE, NULL},
+  {"mno-thumb-adr-sets-interworking", N_("adr does not set interworking bit [default]"),
+   (int *) (& thumb_adr_sets_interworking), FALSE, NULL},
+#endif
   {NULL, NULL, NULL, 0, NULL}
 };