This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [GOLD] Edit PowerPC64 ELFv2 function entry code
- From: Alan Modra <amodra at gmail dot com>
- To: Cary Coutant <ccoutant at gmail dot com>
- Cc: Binutils <binutils at sourceware dot org>
- Date: Wed, 9 Dec 2015 10:02:10 +1030
- Subject: Re: [GOLD] Edit PowerPC64 ELFv2 function entry code
- Authentication-results: sourceware.org; auth=none
- References: <20151125014026 dot GV8120 at bubble dot grove dot modra dot org> <CAJimCsHrqYDDPXpdqex6NGwntVi=U7Scmof+=t-n8Lza0nbC8g at mail dot gmail dot com>
On Tue, Dec 08, 2015 at 02:36:12PM -0800, Cary Coutant wrote:
> > + Insn* iview = reinterpret_cast<Insn*>(view - 2 * big_endian);
> > + Insn insn1 = elfcpp::Swap<32, big_endian>::readval(iview - 1);
> > + Insn insn2 = elfcpp::Swap<32, big_endian>::readval(iview);
>
> I'm afraid I don't understand this logic. As I read it, you're looking
> at view for little-endian, and at (view - 2) for big-endian. If that's
> the case, I think you should be using Swap_unaligned here, but I don't
> see why you offset the view by -2 for big-endian.
The reloc in question operates on a two-byte field of the four-byte
instruction. When the insn is little-endian the field and the insn
start at the same address. Not so for big-endian.
> As a matter of style, since big_endian is a bool, I'd prefer to see
> this written in conditional form, e.g., (view - (big_endian ? 2 : 0)).
>
> > + r_type = elfcpp::R_POWERPC_ADDR16_HA;
> > + addend -= 2 * big_endian;
>
> Likewise, "if (big_endian) addend -= 2;". Easier (for me, at least), to read.
I use this style all over powerpc.cc::Relocate::relocate. The main
reason being that it is a little more concise and
Insn* iview = reinterpret_cast<Insn*>(view - 2 * big_endian);
is getting close to the 80 char line limit. I'll fix it after this
patch goes in to use
// Offset from start of insn to d-field reloc.
const int d_offset = big_endian ? 2 : 0;
--
Alan Modra
Australia Development Lab, IBM