Better overflow checking for powerpc64 relocations

R_PPC64_ADDR16 is used in three contexts:
- .short data relocation
- 16-bit signed insn fields, eg. addi
- 16-bit unsigned insn fields, eg. ori
In the first case we want to allow both signed and unsigned 16-bit
values, the latter two ought to error if the field exceeds the range
of values allowed for 16-bit signed and unsigned integers
respectively.  These conflicting requirements meant that ld had to
choose the least restrictive overflow checks, and thus it is possible
to construct testcases where an addi field overflows but is not
reported by ld.  Many relocations dealing with 16-bit insn fields have
this problem.  What's more, some relocations that are only ever used
for signed fields of instructions woodenly copied the lax overflow
checking of R_PPC64_ADDR16.

bfd/
	* elf64-ppc.c (ppc64_elf_howto_raw): Use complain_overflow_signed
	for R_PPC64_ADDR14, R_PPC64_ADDR14_BRTAKEN, R_PPC64_ADDR14_BRNTAKEN,
	R_PPC64_SECTOFF, R_PPC64_ADDR16_DS, R_PPC64_SECTOFF_DS,
	R_PPC64_REL16 entries.  Use complain_overflow_dont for R_PPC64_TOC.
	(ppc64_elf_relocate_section): Modify overflow test for 16-bit
	fields in instructions to signed/unsigned according to whether
	the field takes a signed or unsigned value.
gold/
	* powerpc.cc (Powerpc_relocate_functions::Overflow_check): Add
	CHECK_UNSIGNED, CHECK_LOW_INSN, CHECK_HIGH_INSN.
	(Powerpc_relocate_functions::has_overflow_unsigned): New function.
	(Powerpc_relocate_functions::has_overflow_bitfield,
	overflowed): Use the above.
	(Target_powerpc::Relocate::relocate): Correct overflow checking
	for a number of relocations.  Modify overflow test for 16-bit
	fields in instructions to signed/unsigned according to whether
	the field takes a signed or unsigned value.

diff --git a/bfd/ChangeLog b/bfd/ChangeLog
index 31e7ec5b3f870ac943d878003c14dc142b1139ae..d3a50e4e6be3fced6c2171eda12a02ce578f10e3 100644 (file)
--- a/bfd/ChangeLog
+++ b/bfd/ChangeLog
@@ -1,6 +1,15 @@
 2014-06-12  Alan Modra  <amodra@gmail.com>
 
        Backport mainline patches
+       2014-03-08  Alan Modra  <amodra@gmail.com>
+       * elf64-ppc.c (ppc64_elf_howto_raw): Use complain_overflow_signed
+       for R_PPC64_ADDR14, R_PPC64_ADDR14_BRTAKEN, R_PPC64_ADDR14_BRNTAKEN,
+       R_PPC64_SECTOFF, R_PPC64_ADDR16_DS, R_PPC64_SECTOFF_DS,
+       R_PPC64_REL16 entries.  Use complain_overflow_dont for R_PPC64_TOC.
+       (ppc64_elf_relocate_section): Modify overflow test for 16-bit
+       fields in instructions to signed/unsigned according to whether
+       the field takes a signed or unsigned value.
+
        2014-02-19  Alan Modra  <amodra@gmail.com>
        * elf64-ppc.h (struct ppc64_elf_params): Add save_restore_funcs.
        * elf64-ppc.c (ppc64_elf_func_desc_adjust): Use it to control

diff --git a/bfd/elf64-ppc.c b/bfd/elf64-ppc.c
index 971426e117d60996d92c891c844faf30fbcab2c9..c6041923bc9df4a2d13f9c95d8db8be9bb5bf90c 100644 (file)
--- a/bfd/elf64-ppc.c
+++ b/bfd/elf64-ppc.c
@@ -359,7 +359,7 @@ static reloc_howto_type ppc64_elf_howto_raw[] = {
         16,                    /* bitsize */
         FALSE,                 /* pc_relative */
         0,                     /* bitpos */
-        complain_overflow_bitfield, /* complain_on_overflow */
+        complain_overflow_signed, /* complain_on_overflow */
         ppc64_elf_branch_reloc, /* special_function */
         "R_PPC64_ADDR14",      /* name */
         FALSE,                 /* partial_inplace */
@@ -376,7 +376,7 @@ static reloc_howto_type ppc64_elf_howto_raw[] = {
         16,                    /* bitsize */
         FALSE,                 /* pc_relative */
         0,                     /* bitpos */
-        complain_overflow_bitfield, /* complain_on_overflow */
+        complain_overflow_signed, /* complain_on_overflow */
         ppc64_elf_brtaken_reloc, /* special_function */
         "R_PPC64_ADDR14_BRTAKEN",/* name */
         FALSE,                 /* partial_inplace */
@@ -393,7 +393,7 @@ static reloc_howto_type ppc64_elf_howto_raw[] = {
         16,                    /* bitsize */
         FALSE,                 /* pc_relative */
         0,                     /* bitpos */
-        complain_overflow_bitfield, /* complain_on_overflow */
+        complain_overflow_signed, /* complain_on_overflow */
         ppc64_elf_brtaken_reloc, /* special_function */
         "R_PPC64_ADDR14_BRNTAKEN",/* name */
         FALSE,                 /* partial_inplace */
@@ -634,7 +634,6 @@ static reloc_howto_type ppc64_elf_howto_raw[] = {
         32,                    /* bitsize */
         TRUE,                  /* pc_relative */
         0,                     /* bitpos */
-        /* FIXME: Verify.  Was complain_overflow_bitfield.  */
         complain_overflow_signed, /* complain_on_overflow */
         bfd_elf_generic_reloc, /* special_function */
         "R_PPC64_REL32",       /* name */
@@ -729,7 +728,7 @@ static reloc_howto_type ppc64_elf_howto_raw[] = {
         16,                    /* bitsize */
         FALSE,                 /* pc_relative */
         0,                     /* bitpos */
-        complain_overflow_bitfield, /* complain_on_overflow */
+        complain_overflow_signed, /* complain_on_overflow */
         ppc64_elf_sectoff_reloc, /* special_function */
         "R_PPC64_SECTOFF",     /* name */
         FALSE,                 /* partial_inplace */
@@ -1017,7 +1016,7 @@ static reloc_howto_type ppc64_elf_howto_raw[] = {
         64,                    /* bitsize */
         FALSE,                 /* pc_relative */
         0,                     /* bitpos */
-        complain_overflow_bitfield, /* complain_on_overflow */
+        complain_overflow_dont, /* complain_on_overflow */
         ppc64_elf_toc64_reloc, /* special_function */
         "R_PPC64_TOC",         /* name */
         FALSE,                 /* partial_inplace */
@@ -1105,7 +1104,7 @@ static reloc_howto_type ppc64_elf_howto_raw[] = {
         16,                    /* bitsize */
         FALSE,                 /* pc_relative */
         0,                     /* bitpos */
-        complain_overflow_bitfield, /* complain_on_overflow */
+        complain_overflow_signed, /* complain_on_overflow */
         bfd_elf_generic_reloc, /* special_function */
         "R_PPC64_ADDR16_DS",   /* name */
         FALSE,                 /* partial_inplace */
@@ -1180,7 +1179,7 @@ static reloc_howto_type ppc64_elf_howto_raw[] = {
         16,                    /* bitsize */
         FALSE,                 /* pc_relative */
         0,                     /* bitpos */
-        complain_overflow_bitfield, /* complain_on_overflow */
+        complain_overflow_signed, /* complain_on_overflow */
         ppc64_elf_sectoff_reloc, /* special_function */
         "R_PPC64_SECTOFF_DS",  /* name */
         FALSE,                 /* partial_inplace */
@@ -1952,7 +1951,7 @@ static reloc_howto_type ppc64_elf_howto_raw[] = {
         16,                    /* bitsize */
         TRUE,                  /* pc_relative */
         0,                     /* bitpos */
-        complain_overflow_bitfield, /* complain_on_overflow */
+        complain_overflow_signed, /* complain_on_overflow */
         bfd_elf_generic_reloc, /* special_function */
         "R_PPC64_REL16",       /* name */
         FALSE,                 /* partial_inplace */
@@ -12895,6 +12894,8 @@ ppc64_elf_relocate_section (bfd *output_bfd,
       bfd_vma max_br_offset;
       bfd_vma from;
       const Elf_Internal_Rela orig_rel = *rel;
+      reloc_howto_type *howto;
+      struct reloc_howto_struct alt_howto;
 
       r_type = ELF64_R_TYPE (rel->r_info);
       r_symndx = ELF64_R_SYM (rel->r_info);
@@ -14451,6 +14452,7 @@ ppc64_elf_relocate_section (bfd *output_bfd,
        }
 
       /* Do any further special processing.  */
+      howto = ppc64_elf_howto_table[(int) r_type];
       switch (r_type)
        {
        default:
@@ -14525,7 +14527,7 @@ ppc64_elf_relocate_section (bfd *output_bfd,
              info->callbacks->einfo
                (_("%P: %H: error: %s not a multiple of %u\n"),
                 input_bfd, input_section, rel->r_offset,
-                ppc64_elf_howto_table[r_type]->name,
+                howto->name,
                 mask + 1);
              bfd_set_error (bfd_error_bad_value);
              ret = FALSE;
@@ -14546,23 +14548,45 @@ ppc64_elf_relocate_section (bfd *output_bfd,
          info->callbacks->einfo
            (_("%P: %H: unresolvable %s against `%T'\n"),
             input_bfd, input_section, rel->r_offset,
-            ppc64_elf_howto_table[(int) r_type]->name,
+            howto->name,
             h->elf.root.root.string);
          ret = FALSE;
        }
 
-      r = _bfd_final_link_relocate (ppc64_elf_howto_table[(int) r_type],
-                                   input_bfd,
-                                   input_section,
-                                   contents,
-                                   rel->r_offset,
-                                   relocation,
-                                   addend);
+      /* 16-bit fields in insns mostly have signed values, but a
+        few insns have 16-bit unsigned values.  Really, we should
+        have different reloc types.  */
+      if (howto->complain_on_overflow != complain_overflow_dont
+         && howto->dst_mask == 0xffff
+         && (input_section->flags & SEC_CODE) != 0)
+       {
+         enum complain_overflow complain = complain_overflow_signed;
+
+         insn = bfd_get_32 (input_bfd, contents + (rel->r_offset & ~3));
+         if (howto->rightshift == 0
+             ? ((insn & (0x3f << 26)) == 28u << 26 /* andi */
+                || (insn & (0x3f << 26)) == 24u << 26 /* ori */
+                || (insn & (0x3f << 26)) == 26u << 26 /* xori */
+                || (insn & (0x3f << 26)) == 10u << 26 /* cmpli */)
+             : ((insn & (0x3f << 26)) == 29u << 26 /* andis */
+                || (insn & (0x3f << 26)) == 25u << 26 /* oris */
+                || (insn & (0x3f << 26)) == 27u << 26 /* xoris */))
+           complain = complain_overflow_unsigned;
+         if (howto->complain_on_overflow != complain)
+           {
+             alt_howto = *howto;
+             alt_howto.complain_on_overflow = complain;
+             howto = &alt_howto;
+           }
+       }
+
+      r = _bfd_final_link_relocate (howto, input_bfd, input_section, contents,
+                                   rel->r_offset, relocation, addend);
 
       if (r != bfd_reloc_ok)
        {
          char *more_info = NULL;
-         const char *reloc_name = ppc64_elf_howto_table[r_type]->name;
+         const char *reloc_name = howto->name;
 
          if (reloc_dest != DEST_NORMAL)
            {
@@ -14582,7 +14606,7 @@ ppc64_elf_relocate_section (bfd *output_bfd,
                continue;
              if (h != NULL
                  && h->elf.root.type == bfd_link_hash_undefweak
-                 && ppc64_elf_howto_table[r_type]->pc_relative)
+                 && howto->pc_relative)
                {
                  /* Assume this is a call protected by other code that
                     detects the symbol is undefined.  If this is the case,

diff --git a/gold/ChangeLog b/gold/ChangeLog
index c267e191ff39be38bfa95be778c646af8e23046b..bbd352777b58505d40079f151b9d83a8bf390644 100644 (file)
--- a/gold/ChangeLog
+++ b/gold/ChangeLog
@@ -1,3 +1,17 @@
+2014-06-12  Alan Modra  <amodra@gmail.com>
+
+       Backport mainline patches
+       2014-03-07  Alan Modra  <amodra@gmail.com>
+       * powerpc.cc (Powerpc_relocate_functions::Overflow_check): Add
+       CHECK_UNSIGNED, CHECK_LOW_INSN, CHECK_HIGH_INSN.
+       (Powerpc_relocate_functions::has_overflow_unsigned): New function.
+       (Powerpc_relocate_functions::has_overflow_bitfield,
+       overflowed): Use the above.
+       (Target_powerpc::Relocate::relocate): Correct overflow checking
+       for a number of relocations.  Modify overflow test for 16-bit
+       fields in instructions to signed/unsigned according to whether
+       the field takes a signed or unsigned value.
+
 2014-05-27  H.J. Lu  <hongjiu.lu@intel.com>
 
        PR gold/16945

diff --git a/gold/powerpc.cc b/gold/powerpc.cc
index 1aa4791841906126bfa9f670c929a12964c6e50a..d10977eb11e0ba47abb5e0f88b476c791108351a 100644 (file)
--- a/gold/powerpc.cc
+++ b/gold/powerpc.cc
@@ -1446,7 +1446,10 @@ public:
   {
     CHECK_NONE,
     CHECK_SIGNED,
-    CHECK_BITFIELD
+    CHECK_UNSIGNED,
+    CHECK_BITFIELD,
+    CHECK_LOW_INSN,
+    CHECK_HIGH_INSN
   };
 
   enum Status
@@ -1472,12 +1475,20 @@ private:
 
   template<int valsize>
   static inline bool
-  has_overflow_bitfield(Address value)
+  has_overflow_unsigned(Address value)
   {
     Address limit = static_cast<Address>(1) << ((valsize - 1) >> 1);
     limit <<= ((valsize - 1) >> 1);
     limit <<= ((valsize - 1) - 2 * ((valsize - 1) >> 1));
-    return value > (limit << 1) - 1 && value + limit > (limit << 1) - 1;
+    return value > (limit << 1) - 1;
+  }
+
+  template<int valsize>
+  static inline bool
+  has_overflow_bitfield(Address value)
+  {
+    return (has_overflow_unsigned<valsize>(value)
+           && has_overflow_signed<valsize>(value));
   }
 
   template<int valsize>
@@ -1489,6 +1500,11 @@ private:
        if (has_overflow_signed<valsize>(value))
          return STATUS_OVERFLOW;
       }
+    else if (overflow == CHECK_UNSIGNED)
+      {
+       if (has_overflow_unsigned<valsize>(value))
+         return STATUS_OVERFLOW;
+      }
     else if (overflow == CHECK_BITFIELD)
       {
        if (has_overflow_bitfield<valsize>(value))
@@ -7250,6 +7266,7 @@ Target_powerpc<size, big_endian>::Relocate::relocate(
     }
 
   typename Reloc::Overflow_check overflow = Reloc::CHECK_NONE;
+  elfcpp::Shdr<size, big_endian> shdr(relinfo->data_shdr);
   switch (r_type)
     {
     case elfcpp::R_POWERPC_ADDR32:
@@ -7263,16 +7280,19 @@ Target_powerpc<size, big_endian>::Relocate::relocate(
        overflow = Reloc::CHECK_SIGNED;
       break;
 
-    case elfcpp::R_POWERPC_ADDR24:
-    case elfcpp::R_POWERPC_ADDR16:
     case elfcpp::R_POWERPC_UADDR16:
-    case elfcpp::R_PPC64_ADDR16_DS:
-    case elfcpp::R_POWERPC_ADDR14:
-    case elfcpp::R_POWERPC_ADDR14_BRTAKEN:
-    case elfcpp::R_POWERPC_ADDR14_BRNTAKEN:
       overflow = Reloc::CHECK_BITFIELD;
       break;
 
+    case elfcpp::R_POWERPC_ADDR16:
+      // We really should have three separate relocations,
+      // one for 16-bit data, one for insns with 16-bit signed fields,
+      // and one for insns with 16-bit unsigned fields.
+      overflow = Reloc::CHECK_BITFIELD;
+      if ((shdr.get_sh_flags() & elfcpp::SHF_EXECINSTR) != 0)
+       overflow = Reloc::CHECK_LOW_INSN;
+      break;
+
     case elfcpp::R_POWERPC_ADDR16_HI:
     case elfcpp::R_POWERPC_ADDR16_HA:
     case elfcpp::R_POWERPC_GOT16_HI:
@@ -7299,17 +7319,31 @@ Target_powerpc<size, big_endian>::Relocate::relocate(
     case elfcpp::R_POWERPC_GOT_DTPREL16_HA:
     case elfcpp::R_POWERPC_REL16_HI:
     case elfcpp::R_POWERPC_REL16_HA:
-      if (size == 32)
-       break;
-    case elfcpp::R_POWERPC_REL24:
-    case elfcpp::R_PPC_PLTREL24:
-    case elfcpp::R_PPC_LOCAL24PC:
+      if (size != 32)
+       overflow = Reloc::CHECK_HIGH_INSN;
+      break;
+
     case elfcpp::R_POWERPC_REL16:
     case elfcpp::R_PPC64_TOC16:
     case elfcpp::R_POWERPC_GOT16:
     case elfcpp::R_POWERPC_SECTOFF:
     case elfcpp::R_POWERPC_TPREL16:
     case elfcpp::R_POWERPC_DTPREL16:
+    case elfcpp::R_POWERPC_GOT_TLSGD16:
+    case elfcpp::R_POWERPC_GOT_TLSLD16:
+    case elfcpp::R_POWERPC_GOT_TPREL16:
+    case elfcpp::R_POWERPC_GOT_DTPREL16:
+      overflow = Reloc::CHECK_LOW_INSN;
+      break;
+
+    case elfcpp::R_POWERPC_ADDR24:
+    case elfcpp::R_POWERPC_ADDR14:
+    case elfcpp::R_POWERPC_ADDR14_BRTAKEN:
+    case elfcpp::R_POWERPC_ADDR14_BRNTAKEN:
+    case elfcpp::R_PPC64_ADDR16_DS:
+    case elfcpp::R_POWERPC_REL24:
+    case elfcpp::R_PPC_PLTREL24:
+    case elfcpp::R_PPC_LOCAL24PC:
     case elfcpp::R_PPC64_TPREL16_DS:
     case elfcpp::R_PPC64_DTPREL16_DS:
     case elfcpp::R_PPC64_TOC16_DS:
@@ -7318,14 +7352,28 @@ Target_powerpc<size, big_endian>::Relocate::relocate(
     case elfcpp::R_POWERPC_REL14:
     case elfcpp::R_POWERPC_REL14_BRTAKEN:
     case elfcpp::R_POWERPC_REL14_BRNTAKEN:
-    case elfcpp::R_POWERPC_GOT_TLSGD16:
-    case elfcpp::R_POWERPC_GOT_TLSLD16:
-    case elfcpp::R_POWERPC_GOT_TPREL16:
-    case elfcpp::R_POWERPC_GOT_DTPREL16:
       overflow = Reloc::CHECK_SIGNED;
       break;
     }
 
+  if (overflow == Reloc::CHECK_LOW_INSN
+      || overflow == Reloc::CHECK_HIGH_INSN)
+    {
+      Insn* iview = reinterpret_cast<Insn*>(view - 2 * big_endian);
+      Insn insn = elfcpp::Swap<32, big_endian>::readval(iview);
+
+      overflow = Reloc::CHECK_SIGNED;
+      if (overflow == Reloc::CHECK_LOW_INSN
+         ? ((insn & (0x3f << 26)) == 28u << 26 /* andi */
+            || (insn & (0x3f << 26)) == 24u << 26 /* ori */
+            || (insn & (0x3f << 26)) == 26u << 26 /* xori */
+            || (insn & (0x3f << 26)) == 10u << 26 /* cmpli */)
+         : ((insn & (0x3f << 26)) == 29u << 26 /* andis */
+            || (insn & (0x3f << 26)) == 25u << 26 /* oris */
+            || (insn & (0x3f << 26)) == 27u << 26 /* xoris */))
+       overflow = Reloc::CHECK_UNSIGNED;
+    }
+
   typename Powerpc_relocate_functions<size, big_endian>::Status status
     = Powerpc_relocate_functions<size, big_endian>::STATUS_OK;
   switch (r_type)