This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
RE: [Patch] Gas support for MIPS Compact EH
- From: "Moore, Catherine" <Catherine_Moore at mentor dot com>
- To: Richard Sandiford <rdsandiford at googlemail dot com>
- Cc: "Schmidt, Bernd" <Bernd_Schmidt at mentor dot com>, "binutils at sourceware dot org" <binutils at sourceware dot org>
- Date: Thu, 20 Feb 2014 21:24:23 +0000
- Subject: RE: [Patch] Gas support for MIPS Compact EH
- Authentication-results: sourceware.org; auth=none
- References: <FD3DCEAC5B03E9408544A1E416F11242F8FC5972 at NA-MBX-01 dot mgc dot mentorg dot com> <87k3me9jia dot fsf at talisman dot default> <FD3DCEAC5B03E9408544A1E416F11242012EAC2AE0 at NA-MBX-01 dot mgc dot mentorg dot com> <8738jt5zt1 dot fsf at talisman dot default> <52F67C03 dot 2050609 at codesourcery dot com> <87y51k4k2s dot fsf at talisman dot default> <FD3DCEAC5B03E9408544A1E416F11242012EACEB23 at NA-MBX-01 dot mgc dot mentorg dot com> <87zjlmhv7j dot fsf at sandifor-thinkpad dot stglab dot manchester dot uk dot ibm dot com>
> -----Original Message-----
> From: Richard Sandiford [mailto:rdsandiford@googlemail.com]
> Sent: Thursday, February 20, 2014 6:37 AM
> To: Moore, Catherine
> Cc: Schmidt, Bernd; binutils@sourceware.org
> Subject: Re: [Patch] Gas support for MIPS Compact EH
>
> "Moore, Catherine" <Catherine_Moore@mentor.com> writes:
> >> -----Original Message-----
> >> From: Richard Sandiford [mailto:rdsandiford@googlemail.com]
> >> Sent: Sunday, February 09, 2014 6:11 AM
> >> To: Schmidt, Bernd
> >> Cc: Moore, Catherine; binutils@sourceware.org
> >> Subject: Re: [Patch] Gas support for MIPS Compact EH
> >>
> >> Bernd Schmidt <bernds@codesourcery.com> writes:
> >> > On 02/08/2014 05:34 PM, Richard Sandiford wrote:
> >> >>>> The tc_cfi_fix_eh_ref and tc_cfi_emit_expr hooks don't seem very
> >> >>>> consistent; the former relies on the caller to clear the bytes,
> >> >>>> whereas the latter is supposed to do it itself.
> >> >>>
> >> >>> All fixed, now using a hook to return a reloc and eliminated the
> >> >>> use of R_MIPS_EH from the assembler.
> >> >>
> >> >> Hmm, but how does it work under the new scheme? It looks like gas
> >> >> now always emits the .eh_frame_entry sections using R_MIPS_PC32,
> >> >> is
> >> that right?
> >> >> But the linker chooses the .eh_frame_hdr encoding based on
> >> >> --pcrel-eh-reloc, which also controls how R_MIPS_EH is handled.
> >> >> So if
> >> the:
> >> >>
> >> >> DW_EH_PE_sdata4 | DW_EH_PE_datarel | DW_EH_PE_indirect
> >> >>
> >> >> encoding is chosen for the .eh_frame_entry sections at link time,
> >> >> what converts the input sections to use that encoding instead of
> >> >> the original R_MIPS_PC32? I'd assumed R_MIPS_EH was defined the
> >> >> way it was to avoid that kind of thing.
> >> >
> >> > What's changed is that the linker is no longer really involved in
> >> > these decisions - the code you see in the linker-specific parts of
> >> > the patch are there merely to deal with R_MIPS_EH relocs in object
> >> > files generated by previous toolchains. We've always kind of
> >> > already decided at compile time which encoding to use and passed
> >> > the -pcrel-eh-reloc option to the linker to ensure it made the
> >> > choice we wanted. What's new in this version of the patches is
> >> > based on the realization that gcc can produce
> >> > datarel|indirect encoding without linker help (using the new
> >> > datarel|forcegpword
> >> > op).
> >>
> >> Ah, OK, so it's now up to the assembler writer to do the indirection by
> hand?
> >> I hadn't realised that. (To be fair, the patch added the
> >> .forcegpword directive but didn't have any examples or tests to show
> >> how it was used, or any documentation explaining it. I deliberately
> >> didn't complain about the latter though because most ops are
> >> undocumented.)
> >>
> >> Using GP-relative is probably a bad idea for MIPS because the GP base
> >> can vary in the case of multigot. When resolving the relocations in
> >> the individual input sections, the GP used will be for that input
> >> bfd's GOT, which isn't necessarily going to be the primary GOT. So
> >> if we're doing the indirection by hand, why not use pcrel|indirect
> >> instead? I suppose that amounts to using .ehword (under the new
> >> PC-relative definition) rather than .forcegpword. I think it'd be
> >> better to drop .forcegpword if possible.
> >>
> >> FWIW, pcrel|indirect is what the linker tries to use for .eh_frame on
> >> MIPS, if the original input object used absolute indirect.
> >>
> >> > On Linux targets you'll see this generated by the compiler, on
> >> > bare-metal you'll get R_MIPS_PC32. The R_MIPS_EH reloc is longer
> >> > produced. So it's all a lot more straightforward, directly
> >> > producing an encoding appropriate for the target at compile time.
> >> >
> >> >> I thought R_MIPS_EH would be used for the .eh_frame_entry entries
> >> >> only, since in that case the actual encoding of the address isn't
> >> >> known until link time.
> >> >
> >> > I think previous versions of the code were just slightly confused -
> >> > the idea was that datarel|indirect required things to be put into
> >> > the got, which has to be done by the linker. It turns out that this
> >> > isn't necessary, so there is no longer a need to use R_MIPS_EH.
> >> >
> >> > Does this clarify things?
> >>
> >> I don't think it really answers my first question. It's still the
> >> linker that decides what encoding goes in the .eh_frame_hdr.
> >> Then all the following .eh_frame_entry sections must use that
> >> encoding for the text addresses.
> >>
> >> The input objects use relocations to mark those .eh_frame_entry text
> >> addresses. In the original scheme those relocations were R_MIPS_EH,
> >> giving the linker control of both the .eh_frame_hdr encoding and the
> >> .eh_frame_entry fields that that encoding controls. That part seemed
> >> consistent and safe. (The unsafe part was that other parts of the
> >> assembler seemed to assume that R_MIPS_EH always meant
> >> datarel-indirect.)
> >>
> >> In the new scheme the assembler picks an encoding-specific relocation
> >> for those .eh_frame_entry text addresses (always PC-relative in the
> >> posted patch), but the linker is still the one that chooses the
> >> .eh_frame_hdr encoding. And in the posted patch the encoding used in
> >> .eh_frame_hdr is decided by the --pcrel-eh-reloc option. The default
> >> linker behaviour is to use datarel-indirect, which is the opposite of
> >> what the assembler now uses. So if I do:
> >>
> >> as foo.s -o foo.o
> >> ld foo.o -o foo
> >>
> >> it looked like foo.o would use a PC-relative encoding for any
> >> .eh_frame_entry sections, but foo's .eh_frame_hdr would say that they
> >> are datarel-indirect rather than PC-relative.
> >>
> >> In other words, I think using datarel-indirect for the .eh_frame_hdr
> >> is only safe if all input objects are using the old scheme. So that
> >> seems like a bad default. And I'm not sure --pcrel-eh-reloc is safe
> >> for old objects because of the assumption in some cases that
> >> R_MIPS_EH would generate datarel- indirect.
> >>
> >> Maybe for the FSF version we should just drop the R_MIPS_EH stuff
> >> altogether, since at this point it sounds like it would be safer to
> >> reject the old objects than to try to guess what's happening.
> >> The .eh_frame_hdr could then be hard-coded to use a PC-relative
> >> encoding, like the assembler is now. It seems a bit of a shame in a
> >> way though -- and like I say, using PC-relative wouldn't apply well
> >> to targets like VxWorks where the gap between the text and data
> >> segments isn't fixed at link time.
> >>
> >
> > Dropping the R_MIPS_EH stuff is not a desirable option for us.
> > If we were to go back to the original implementation, using R_MIPS_EH
> > for entries in the .eh_frame_entry sections, then your objection to
> > the implementation was the inconsistent treatment in the assembler?
>
> Yes, or more specifically: the assembler was sometimes associating
> R_MIPS_EH with a specific encoding type, even though we don't know at
> assembly time what encoding is associated with R_MIPS_EH.
>
> > You said:
> > RS> Or, to put it another way, why are we making the choice between
> > RS> R_MIPS_PC32 and R_MIPS_EH at assembly time in
> mips_cfi_emit_expr,
> > RS> but not in mips_cfi_fix_eh_ref, even though the patch appears to
> > RS> allow the same two encodings in both casees?
> >
> > I'd like to take this approach in the next patch:
> > 1. Keep the R_MIPS_EH relocation
> > 2. Let the linker choose the appropriate encoding 3. Clean up the
> > assembler inconsistencies
>
> That's OK with me, but just to clarify: (3) IMO means that R_MIPS_EH is
> never associated with a specific encoding in the assembler. I.e.
> R_MIPS_EH is purely for an as-yet unknown encoding that is chosen by the
> linker rather than the assembler or the assembly author. And AIUI the only
> place that happens is in .eh_frame_entry.
>
> Perhaps one way of doing that would to have a generic
> BFD_RELOC_EH_FRAME_HDR_32 that R_MIPS_EH maps to. Then when
> emitting the .eh_frame_entry addresses, the assembler unconditionally uses
> that BFD_RELOC_ rather than a target hook.
>
> What do you plan to do for .ehword? Since the assembler generates the
> .eh_frame_entry itself, and since R_MIPS_EH should only be used there
> (since that's the only place where the linker controls the encoding), I don't
> think there are any valid uses of an R_MIPS_EH-producing .ehword.
>
> > This would also mean dropping the .forcegpword directive as you
> suggested.
>
> My problem with .forcegpword was more that the GP base isn't a DSO-wide
> value, it's only local to the input object. So IMO dropping it (which is fine) is
> separate from whether to keep R_MIPS_EH. I assume if we drop it then the
> only supported LSDA encoding will be pcrel and pcrel|indirect, at least for
> now?
>
That's not right. We need datarel|indirect for the MIPS Linux toolchain.
I don't follow why dropping the .forcegpword directive implies that datarel|indirect can't be generated.
Thanks,
Catherine