PPC gold doesn't check for overflow properly

[Cary Coutant <ccoutant at google dot com>]

Alan,

I ran across a large PPC shared library with a bad branch, and found
that a stub table had grown larger than 4 MB. We had a branch to
__cxa_atexit near the end of the stub group, and a PLT call stub had
already been created near the beginning of the stub table for that
group. As a result, the displacement between the call and the stub was
larger than 32 MB, and the linker relocated the branch to the wrong
place.

The call was at address 0xa08 d8a0, and the stub was at 0x807 e198,
giving a displacement of -0x200 f708 (0xffff ffff fdff 08f8,
sign-extended). This got applied as a positive displacement of 0x1ff
08f8 without any complaint.

I see that addr24() calls rela<32>(), which is only going to check for
a 32-bit overflow, not a 24-bit overflow:

  // R_POWERPC_ADDR24: (Symbol + Addend) & 0x3fffffc
  static inline Status
  addr24(unsigned char* view, Address value, Overflow_check overflow)
  {
    Status stat = This::template rela<32>(view, 0, 0x03fffffc, value, overflow);
    if (overflow != CHECK_NONE && (value & 3) != 0)
      stat = STATUS_OVERFLOW;
    return stat;
  }

  template<int valsize>
  static inline Status
  rela(unsigned char* view,
       unsigned int right_shift,
       typename elfcpp::Valtype_base<valsize>::Valtype dst_mask,
       Address value,
       Overflow_check overflow)
  {
    typedef typename elfcpp::Swap<valsize, big_endian>::Valtype Valtype;
    Valtype* wv = reinterpret_cast<Valtype*>(view);
    Valtype val = elfcpp::Swap<valsize, big_endian>::readval(wv);
    Valtype reloc = value >> right_shift;
    val &= ~dst_mask;
    reloc &= dst_mask;
    elfcpp::Swap<valsize, big_endian>::writeval(wv, val | reloc);
    return overflowed<valsize>(value >> right_shift, overflow);
  }

What would you suggest? It seems like rela<32>() could check that the
result fits within the mask given it, but it defers all of the
signed-vs-unsigned logic to overflowed<32>(). We could add another
template parameter: rela<valsize, immsize>(), and have it call
overflowed<immsize>(). Or, just pass the dst_mask to overflowed(), and
have has_overflow_[un]signed() check against the mask.

When we do detect a relocation overflow, would it also make sense to
print a suggestion to use --stub-group-size with a value smaller than
28 MB? (As a workaround, I've suggested using 24 MB, and that seems to
work.)

Aside from overflow detection, it seems like add_plt_call_entry(),
when it finds an existing stub, could check that the stub is within
range of the branch we're looking at, and go ahead and create a new
stub if it's not. As long as we create stubs in the same order as the
branches, it should always be possible for a branch to reach its stub,
regardless of the size of the stub table.

Also, I noted that using the default options, the stub group size is
set much smaller if we find 14-bit relocations in a section, but if
you set --stub-group-size, that's the size we use regardless. Would it
make sense to set stub14_group_size proportionally to stub_group_size,
or to provide a separate option?

-cary