This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: PPC gold doesn't check for overflow properly
- From: Alan Modra <amodra at gmail dot com>
- To: Cary Coutant <ccoutant at google dot com>
- Cc: Binutils <binutils at sourceware dot org>
- Date: Fri, 21 Nov 2014 08:21:12 +1030
- Subject: Re: PPC gold doesn't check for overflow properly
- Authentication-results: sourceware.org; auth=none
- References: <CAHACq4rr8Z6bKL74tmunhwZYUJx1s6md=+0vg5+oWVDjNr84bA at mail dot gmail dot com> <20141120111444 dot GB4477 at bubble dot grove dot modra dot org> <CAHACq4piLzr-ci8w9c-tC_+qzf13Ar-shNAgw_028kvmtO7jKQ at mail dot gmail dot com>
On Thu, Nov 20, 2014 at 10:12:22AM -0800, Cary Coutant wrote:
> In rela(), you pass (value >> right_shift) to overflowed():
>
> return overflowed<valsize>(value >> right_shift, overflow);
>
> But in addr24(), you pass a valsize of 26:
>
> + Status stat = This::template rela<32,26>(view, 0, 0x03fffffc,
> + value, overflow);
>
> Given the right shift, shouldn't that be 24?
No. The right shift here is 0. The field has 24 bits (hence the
name), but the bottom 2 bits of the value are masked. So the range is
that for a signed 26-bit multiple of four.
> (addr14() uses a right shift of 0, so valsize == 16 looks right for that case.)
>
> - if (status != Powerpc_relocate_functions<size, big_endian>::STATUS_OK)
> - gold_error_at_location(relinfo, relnum, rela.get_r_offset(),
> - _("relocation overflow"));
> + if (status != Powerpc_relocate_functions<size, big_endian>::STATUS_OK
> + && !has_stub_value
> + && !(gsym != NULL
> + && gsym->is_weak_undefined()
> + && is_branch_reloc(r_type)))
> + {
> + gold_error_at_location(relinfo, relnum, rela.get_r_offset(),
> + _("relocation overflow"));
> + if (has_stub_value)
> + gold_info(_("try relinking with a smaller --stub-group-size"));
> + }
>
> With !has_stub_value in the outer condition, I don't think that info
> message will ever get printed.
That was silly of me.
> Why add all the new conditions?
We don't want to report an error on a branch to an undefined weak.
These are presumably from code like
if (foo)
foo();
where the call (to zero when foo is undefined weak) will never be
executed.
* powerpc.cc (Target_powerpc::Relocate::relocate): Correct test
for undefined weaks.
diff --git a/gold/powerpc.cc b/gold/powerpc.cc
index 0442e56..4c90e38 100644
--- a/gold/powerpc.cc
+++ b/gold/powerpc.cc
@@ -7672,10 +7672,10 @@ Target_powerpc<size, big_endian>::Relocate::relocate(
break;
}
if (status != Powerpc_relocate_functions<size, big_endian>::STATUS_OK
- && !has_stub_value
- && !(gsym != NULL
- && gsym->is_weak_undefined()
- && is_branch_reloc(r_type)))
+ && (has_stub_value
+ || !(gsym != NULL
+ && gsym->is_weak_undefined()
+ && is_branch_reloc(r_type))))
{
gold_error_at_location(relinfo, relnum, rela.get_r_offset(),
_("relocation overflow"));
--
Alan Modra
Australia Development Lab, IBM