Re: s390 gas bug

["Martin Schwidefsky" <schwidefsky at de dot ibm dot com>]

Hi,
the TC_FORCE_RELOCATION problem is still haunting me. I now have a
patch that fixes the problem. But it is really, really ugly:

diff -urN src/gas/config/tc-s390.c src-s390/gas/config/tc-s390.c
--- binutils-2.11.90.0.27/gas/config/tc-s390.c     Tue Oct 30 16:32:00 2001
+++ binutils-2.11.90.0.27-s390/gas/config/tc-s390.c     Tue Oct 30 16:32:03
2001
@@ -1641,6 +1641,36 @@
   return 1;
 }

+/* Return true if we must always emit a reloc for a type and false if
+   there is some hope of resolving it a assembly time.  */
+int
+tc_s390_force_relocation (fixp)
+     struct fix *fixp;
+{
+  /* Ensure we emit a relocation for every reference to the global
+     offset table or to the procedure link table.  */
+  switch (fixp->fx_r_type)
+    {
+    case BFD_RELOC_390_GOT12:
+    case BFD_RELOC_32_GOT_PCREL:
+    case BFD_RELOC_32_GOTOFF:
+    case BFD_RELOC_390_GOTPC:
+    case BFD_RELOC_390_GOT16:
+    case BFD_RELOC_390_GOTPCDBL:
+    case BFD_RELOC_390_GOT64:
+    case BFD_RELOC_390_GOTENT:
+    case BFD_RELOC_390_PLT32:
+    case BFD_RELOC_390_PLT16DBL:
+    case BFD_RELOC_390_PLT32DBL:
+    case BFD_RELOC_390_PLT64:
+    case BFD_RELOC_VTABLE_INHERIT:
+    case BFD_RELOC_VTABLE_ENTRY:
+      return 1;
+    default:
+      return 0;
+    }
+}
+
 /* Apply a fixup to the object code.  This is called for all the
    fixups we generated by the call to fix_new_exp, above.  In the call
    above we used a reloc code which was the largest legal reloc code
@@ -1654,7 +1684,7 @@
 md_apply_fix3 (fixp, valuep, seg)
      fixS *fixp;
      valueT *valuep;
-     segT seg ATTRIBUTE_UNUSED;
+     segT seg;
 {
   char *where;
   valueT value;
@@ -1662,22 +1692,34 @@
   value = *valuep;
   where = fixp->fx_frag->fr_literal + fixp->fx_where;

-  if (fixp->fx_subsy != NULL)
+  if (fixp->fx_subsy != NULL)
     {
+      if ((fixp->fx_addsy != NULL
+       && S_GET_SEGMENT (fixp->fx_addsy) == S_GET_SEGMENT (fixp->fx_subsy)
+       && SEG_NORMAL (S_GET_SEGMENT (fixp->fx_addsy)))
+      || (S_GET_SEGMENT (fixp->fx_subsy) == absolute_section))
+    value += S_GET_VALUE (fixp->fx_subsy);
       if (!S_IS_DEFINED (fixp->fx_subsy))
     as_bad_where (fixp->fx_file, fixp->fx_line,
                _("unresolved fx_subsy symbol that must be resolved"));
       value -= S_GET_VALUE(fixp->fx_subsy);
-    }

-  if (fixp->fx_addsy != NULL)
+      if (S_GET_SEGMENT (fixp->fx_subsy) == seg && ! fixp->fx_pcrel)
+    value += MD_PCREL_FROM_SECTION (fixp, seg);
+    }
+
+  if (fixp->fx_addsy != NULL)
     {
-      /* `*valuep' may contain the value of the symbol on which the reloc
-     will be based; we have to remove it.  */
-      if (fixp->fx_addsy->sy_used_in_reloc
-      && S_GET_SEGMENT (fixp->fx_addsy) != absolute_section
-      && S_GET_SEGMENT (fixp->fx_addsy) != undefined_section
-      && ! bfd_is_com_section (S_GET_SEGMENT (fixp->fx_addsy)))
+      if ((fixp->fx_subsy != NULL
+       && S_GET_SEGMENT (fixp->fx_addsy) == S_GET_SEGMENT (fixp->fx_subsy)
+       && SEG_NORMAL (S_GET_SEGMENT(fixp->fx_addsy)))
+      || (S_GET_SEGMENT (fixp->fx_addsy) == seg
+          && fixp->fx_pcrel && TC_RELOC_RTSYM_LOC_FIXUP (fixp))
+      || (!fixp->fx_pcrel
+          && S_GET_SEGMENT (fixp->fx_addsy) == absolute_section)
+      || (S_GET_SEGMENT (fixp->fx_addsy) != undefined_section
+          && !bfd_is_com_section (S_GET_SEGMENT (fixp->fx_addsy))
+          && TC_FIX_ADJUSTABLE(fixp)))
     value -= S_GET_VALUE (fixp->fx_addsy);

       if (fixp->fx_pcrel)
diff -urN src/gas/config/tc-s390.h src-s390/gas/config/tc-s390.h
--- src/gas/config/tc-s390.h  Fri Jul 27 01:02:55 2001
+++ src-s390/gas/config/tc-s390.h  Tue Oct 30 16:31:00 2001
@@ -44,12 +44,14 @@
            && S_IS_DEFINED ((FIX)->fx_addsy)      \
            && ! S_IS_COMMON ((FIX)->fx_addsy))))

-#define TC_FORCE_RELOCATION(FIXP)       \
-  ((FIXP)->fx_r_type == BFD_RELOC_VTABLE_INHERIT  \
-   || (FIXP)->fx_r_type == BFD_RELOC_VTABLE_ENTRY)
+#define TC_FORCE_RELOCATION(FIXP) tc_s390_force_relocation(FIXP)
+extern int tc_s390_force_relocation PARAMS ((struct fix *));

 #define tc_fix_adjustable(X)  tc_s390_fix_adjustable(X)
 extern int tc_s390_fix_adjustable PARAMS ((struct fix *));
+
+#define TC_FIX_ADJUSTABLE(fixP) \
+  (! symbol_used_in_reloc_p ((fixP)->fx_addsy) && tc_fix_adjustable (fixP))

 /* The target BFD architecture.  */
 #define TARGET_ARCH bfd_arch_s390

The problem is that fixup_segment does too much for a certain type of
relocation that is quite common for 31 bit s/390:

.LT0:
.LC1:  .long function@PLT - .LT0

A relocation of this kind is forced but fixup_segment insists to do
   add_number += (S_GET_VALUE (add_symbolP) - S_GET_VALUE (sub_symbolP))
in the case add_symbolP is in the same segment as sub_symbolP.
I think the TC_FORCE_RELOCATION check is done too late in fixup_segment
(in other situations as well), but I do not dare to change fixup_segment.
It will certainly break some or all other architectures.
I tried to catch all condition where the value of an add_symbol or of a
sub_symbol has been errornously added to the value passed to md_apply_fix3.
The two terms are quite complicated and will definitly break as soon
as a small change is done to fixup_segment. Any chance that fixup_segment
will get reworked in the near future ?

There is another patch in the making. The elf backends for s390 & s390x
are quite old compared to where i386 is. I created more or less new version
of elf32-s390.c and elf64-s390.c that closely matches elf32-i386.c in style
and functions. The question regarding this patch is, should I check these
two files in, or has anyone the desire to proof-read them first?

blue skies,
   Martin

Linux/390 Design & Development, IBM Deutschland Entwicklung GmbH
Schönaicherstr. 220, D-71032 Böblingen, Telefon: 49 - (0)7031 - 16-2247
E-Mail: schwidefsky@de.ibm.com