[Patch] Gas support for MIPS Compact EH

[Moore, Catherine]

> -----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