[PATCH] D14700: [mips][freebsd] Explicitly select PC-relative Personality and LSDA encodings.

[Daniel Sanders via llvm-commits]

Hi Rafael,

Sorry for the slow reply. I'm still catching up on emails after my holiday.

I've replied to your comments below but I feel I ought to mention that we seem to be getting distracted by Linux-specifics that aren't relevant to FreeBSD or this FreeBSD-specific patch. I'm wondering if separate issues have got mixed up somewhere along the lines. 

> -----Original Message-----
> From: Rafael Espíndola [mailto:rafael.espindola at gmail.com]
> Sent: 24 December 2015 00:40
> To: Daniel Sanders
> Cc: reviews+D14700+public+694feb8b7227989b at reviews.llvm.org; llvm-
> commits; Sean Bruno
> Subject: Re: [PATCH] D14700: [mips][freebsd] Explicitly select PC-relative
> Personality and LSDA encodings.
> 
> > Using DW_EH_PE_indirect without DW_EH_PE_pcrel is accepted
> 
> That produces a non-pic relocation to a rw position.

For FreeBSD, yes.

For Linux, it's not that clear cut. It's true that this is what comes out of the assembler but this value of personality encoding is automatically transformed into DW_EH_PE_indirect|DW_EH_PE_pcrel|DW_EH_PE_sdata4 when the object is linked into a shared library.

> > but lead to a link-time problem back when I was making read-only
> .eh_frame work for Android. It produced 'DT_TEXTREL in a shared library'
> warnings but I didn't establish the cause due to time constraints and the
> need to debug the linker to find the cause. I assume a particular conversion
> to pcrel is missing.
> 
> That is because it is non-pic
>
> What about indirect and pcrel? That is what is actually desired:
> 
> * indirect: makes .eh_frame read only
> * pcrel: makes the code PIC.

For FreeBSD, this is what the patch does and it works (with the binutils patch mentioned in the commit message).

For Linux, indirect and pcrel produce:
	tmp.s: Assembler messages:
	tmp.s:17: Error: invalid or unsupported encoding in .cfi_personality
 
> and no, that would not require a R_MIPS_PC64. On x86_64 the relocations
> are
> 
> R_X86_64_64   from the mutable cell to __gxx_personality_v0
> R_X86_64_PC32 from .eh_frame to the cell.
> 
> So for mips the corresponding case would be
> 
> R_MIPS_64
> R_MIPS_PC32

Can you explain what happens to any carry out of bit 31 when these are resolved? Does it promote the 32-bit value to 64-bit and do a 64-bit addition or does it discard the carry-out?
 
> I tried the attached file with gas for 32 bit mips. Everything seems
> to work as expected with CFI_DIFF_EXPR_OK removed.

Thanks, that should help us fix this in new GAS's.

The only snag with changing GAS is that it takes a long time for changes to propagate sufficiently for us to be able to change LLVM to take advantage it. I agree with you that GAS should accept pcrel encodings but even if such a patch were committed today, it could be up to a year before we can count on Linux distributions supporting pcrel. Linux is stuck with the 'wrong-but-works' values in the meantime.

> With 64bit mips gas fails with just
> 
> .cfi_startproc
> .cfi_endproc
> 
> Because it tries to create a 64 bit pcrel.

Is this is with your CFI_DIFF_EXPR_OK change? Mips64 currently works for me on FreeBSD (with this patch) and Linux.

> In any case. Think this is enough evidence to show that there is
> something wrong at the gas and that should be fixed or at least
> understood, not blindly propagated to llvm.

I'm not quite sure what you mean by 'blindly propagated to llvm' here. With this patch committed, FreeBSD will use the values you want. It's the existing Linux implementation that uses wrong-but-working values and is constrained by flaws in the current tools. There was a point where FreeBSD's GAS had its own set of flaws but this was fixed by the patch referenced in the commit message.

> Cheers,
> Rafael