Re: RFC: Adding non-PIC executable support to MIPS
Daniel Jacobowitz [EMAIL PROTECTED] writes:
All comments welcome - Richard, especially from you. How would you
like to proceed? I think the first step should be to get your other
binutils/gcc patches merged, including MIPS16 PIC; I used those as a
base. But see a few of the notes for potential problems with those
patches.
Yeah, Nick's approved most of the remaining binutils changes (thanks).
I haven't applied them yet because of the doubt over whether st_size
should be even or odd for ISA-encoded MIPS16 symbols. I don't really
have an opinion, so I'll accept a maintainerly decision...
Anyway, the gcc patch looks good to me, thanks. The only niggle
I could see was that you didn't update the comment for:
+/* True if the output file is marked as .abicalls; .option pic0
+ (-call_mixed). This is a GNU extension. */
+#define TARGET_ABICALLS_PIC0 \
+ (TARGET_ABSOLUTE_ABICALLS TARGET_PLT)
(That kind of thing was inevitable given the amount of code you had
to wade through. I'm impressed if there's really only one instance!)
I think the gcc side is good to go, modulo the _mcount thing.
As far as binutils goes, I saw a couple of potential problems:
(1) The patch adds the following code to
_bfd_mips_elf_create_dynamic_sections:
+ if (htab-use_plts_and_copy_relocs htab-root.hplt == NULL)
+ {
+ h = _bfd_elf_define_linkage_sym (abfd, info, s,
+ _PROCEDURE_LINKAGE_TABLE_);
+ htab-root.hplt = h;
+ if (h == NULL)
+ return FALSE;
+ h-type = STT_FUNC;
+ }
But use_plts_and_copy_relocs is only set after all input bfds have
been read in.
(2) The patch sets pointer_equality_needed as follows:
@@ -7432,9 +7484,18 @@ _bfd_mips_elf_check_relocs (bfd *abfd, s
elf_hash_table (info)-dynobj = dynobj = abfd;
break;
}
- /* Fall through */
+ /* Fall through. */
default:
+ /* Most static relocations require pointer equality, except
+for branches. */
+ if (h)
+ h-pointer_equality_needed = TRUE;
+ /* Fall through. */
+
+ case R_MIPS_26:
+ case R_MIPS_PC16:
+ case R_MIPS16_26:
if (h)
((struct mips_elf_link_hash_entry *) h)-has_static_relocs = TRUE;
break;
But pointer equality is needed for non-call GOT relocations too.
I couldn't see anything that explicitly handled that case.
I think it would be more robust to set pointer_equality_needed in a
separate block, rather than combining it with the existing switch
statements. It might then be clearer to set has_nonpic_branches
in the new block too, so that you don't need two copies of:
if (h !PIC_OBJECT_P (abfd))
((struct mips_elf_link_hash_entry *) h)-has_nonpic_branches = TRUE;
Some minor nits too:
+ 0x0399, /* l[wd] $25, %lo(GOTPLT[0])($28) */
+ 0x01d9, /* l[wd] $25, %lo(GOTPLT[0])($14) */
+ 0x01d9, /* l[wd] $25, %lo(GOTPLT[0])($14) */
These are all fixed as either lw or ld.
@@ -1649,13 +1695,16 @@ mips_elf_check_symbols (struct mips_elf_
/* H is a function that might need $25 to be valid on entry.
If we're creating a non-PIC relocatable object, mark H as
being PIC. If we're creating a non-relocatable object with
-non-PIC references to H, make sure that H has an la25 stub. */
+branches to H, make sure that H has an la25 stub. Only
+use the stub for branches from non-PIC objects; GCC's
+-mno-shared uses branches from PIC objects to functions
+which do not require $25. */
if (hti-info-relocatable)
{
if (!PIC_OBJECT_P (hti-output_bfd))
h-root.other = ELF_ST_SET_MIPS_PIC (h-root.other);
}
- else if (h-non_pic_ref !mips_elf_add_la25_stub (hti-info, h))
+ else if (h-has_nonpic_branches !mips_elf_add_la25_stub (hti-info,
h))
{
hti-error = TRUE;
return FALSE;
How about something like the following:
-non-PIC references to H, make sure that H has an la25 stub. */
+non-PIC branches and jumps to H, make sure that H has an la25 stub.
+We specifically ignore branches and jumps from EF_PIC objects,
+where the onus is on the compiler or programmer to perform any
+necessary initialization of $25. Sometimes such initialization
+is unnecessary; for example, -mno-shared functions do not use
+the incoming value of $25, and may therefore be called directly. */
(Wordsmith as necessary.) The original wording made it sound like we'd
created a stub if there were any branches at all, but that the stub
would only be used for branches from non-PIC objects.
@@ -2928,6 +2977,7 @@ mips_elf_gotplt_index (struct bfd_link_i
struct mips_elf_link_hash_table *htab;
htab
Re: RFC: Adding non-PIC executable support to MIPS
Richard Sandiford wrote: Daniel Jacobowitz [EMAIL PROTECTED] writes: All comments welcome - Richard, especially from you. How would you like to proceed? I think the first step should be to get your other binutils/gcc patches merged, including MIPS16 PIC; I used those as a base. But see a few of the notes for potential problems with those patches. Yeah, Nick's approved most of the remaining binutils changes (thanks). I haven't applied them yet because of the doubt over whether st_size should be even or odd for ISA-encoded MIPS16 symbols. I don't really have an opinion, so I'll accept a maintainerly decision... [I'm not sure if this is a helpful suggestion or not, so feel free to ignore it if it's not.] I would suggest that st_size be the actual size of the function, as it lives in memory. A test of it's start/end location is could I stick a random data byte there and have it affect the function. For example, for a Thumb function whose ISA address is 0x0001, I would consider for size purposes that it starts at 0x, since altering that byte at run-time would change the meaning of the function. -- Mark Mitchell CodeSourcery [EMAIL PROTECTED] (650) 331-3385 x713
Re: RFC: Adding non-PIC executable support to MIPS
On Thu, Jul 24, 2008 at 12:16:20PM -0400, Daniel Jacobowitz wrote: I have attached Let's all pretend I attached this glibc patch, instead of the one in my previous message, please. -- Daniel Jacobowitz CodeSourcery 2008-07-24 Mark Shinwell [EMAIL PROTECTED] Daniel Jacobowitz [EMAIL PROTECTED] Richard Sandiford [EMAIL PROTECTED] * sysdeps/mips/dl-lookup.c: New. * sysdeps/mips/do-lookup.h: New. * sysdeps/mips/dl-machine.h (ELF_MACHINE_NO_PLT): Remove definition. (ELF_MACHINE_JMP_SLOT): Alter definition and update comment. (elf_machine_type_class): Likewise. (ELF_MACHINE_PLT_REL): Define. (elf_machine_fixup_plt): New. (elf_machine_plt_value): New. (elf_machine_reloc): Handle jump slot and copy relocations. (elf_machine_lazy_rel): Point relocation place at PLT if required. (RESOLVE_GOTSYM): Take a relocation type argument. (elf_machine_got_rel): Bind lazy stubs directly to their target if !lazy. Skip lazy binding for PLT symbols. (elf_machine_runtime_setup): Fill in .got.plt header. * sysdeps/mips/dl-trampoline.c (IFNEWABI): New macro. (_dl_runtime_pltresolve): New. * sysdeps/mips/bits/linkmap.h: New file. * sysdeps/mips/tls-macros.h: Load $gp as required. Merge 32-bit and 64-bit versions. * sysdeps/unix/sysv/linux/mips/mips32/sysdep.h (SYSCALL_ERROR_LABEL): Delete definition. * sysdeps/unix/sysv/linux/mips/nptl/sysdep-cancel.h (PSEUDO_CPLOAD, PSEUDO_ERRJMP, PSEUDO_SAVEGP, PSEUDO_LOADGP): Define. (PSEUDO): Use them. Move outside __PIC__. (PSEUDO_JMP): New. (CENABLE, CDISABLE): Use it. Index: sysdeps/unix/sysv/linux/mips/mips32/sysdep.h === --- sysdeps/unix/sysv/linux/mips/mips32/sysdep.h (revision 213009) +++ sysdeps/unix/sysv/linux/mips/mips32/sysdep.h (working copy) @@ -35,15 +35,7 @@ # define SYS_ify(syscall_name) __NR_/**/syscall_name #endif -#ifdef __ASSEMBLER__ - -/* We don't want the label for the error handler to be visible in the symbol - table when we define it here. */ -#ifdef __PIC__ -# define SYSCALL_ERROR_LABEL 99b -#endif - -#else /* ! __ASSEMBLER__ */ +#ifndef __ASSEMBLER__ /* Define a macro which expands into the inline wrapper code for a system call. */ Index: sysdeps/unix/sysv/linux/mips/nptl/sysdep-cancel.h === --- sysdeps/unix/sysv/linux/mips/nptl/sysdep-cancel.h (revision 213009) +++ sysdeps/unix/sysv/linux/mips/nptl/sysdep-cancel.h (working copy) @@ -25,28 +25,38 @@ #if !defined NOT_IN_libc || defined IS_IN_libpthread || defined IS_IN_librt -#ifdef __PIC__ +# ifdef __PIC__ +# define PSEUDO_CPLOAD .cpload t9; +# define PSEUDO_ERRJMP la t9, __syscall_error; jr t9; +# define PSEUDO_SAVEGP sw gp, 32(sp); cfi_rel_offset (gp, 32); +# define PSEUDO_LOADGP lw gp, 32(sp); +# else +# define PSEUDO_CPLOAD +# define PSEUDO_ERRJMP j __syscall_error; +# define PSEUDO_SAVEGP +# define PSEUDO_LOADGP +# endif + # undef PSEUDO # define PSEUDO(name, syscall_name, args) \ .align 2; \ L(pseudo_start): \ cfi_startproc; \ - 99: la t9,__syscall_error; \ - jr t9; \ + 99: PSEUDO_ERRJMP \ .type __##syscall_name##_nocancel, @function; \ .globl __##syscall_name##_nocancel; \ __##syscall_name##_nocancel: \ .set noreorder; \ -.cpload t9; \ +PSEUDO_CPLOAD \ li v0, SYS_ify(syscall_name); \ syscall; \ .set reorder; \ -bne a3, zero, SYSCALL_ERROR_LABEL; \ +bne a3, zero, 99b; \ ret; \ .size __##syscall_name##_nocancel,.-__##syscall_name##_nocancel; \ ENTRY (name) \ .set noreorder; \ -.cpload t9; \ +PSEUDO_CPLOAD \ .set reorder; \ SINGLE_THREAD_P(v1); \ bne zero, v1, L(pseudo_cancel); \ @@ -54,17 +64,16 @@ li v0, SYS_ify(syscall_name); \ syscall; \ .set reorder; \ -bne a3, zero, SYSCALL_ERROR_LABEL; \ +bne a3, zero, 99b; \ ret; \ L(pseudo_cancel): \ SAVESTK_##args; \ sw ra, 28(sp); \ cfi_rel_offset (ra, 28); \ -sw gp, 32(sp); \ -cfi_rel_offset (gp, 32); \ +PSEUDO_SAVEGP \ PUSHARGS_##args; /* save syscall args */ \ CENABLE; \ -lw gp, 32(sp); \ +PSEUDO_LOADGP \ sw v0, 44(sp); /* save mask */ \ POPARGS_##args; /* restore syscall args */ \ .set noreorder; \ @@ -75,12 +84,12 @@ sw a3, 40(sp); /* save syscall error flag */ \ lw a0, 44(sp);
Re: RFC: Adding non-PIC executable support to MIPS
Daniel Jacobowitz [EMAIL PROTECTED] writes: - Richard's ld -r support is an addition to the ABI, but does not conflict with anything else, so I included it. I discovered two potential problems: - If a symbol with STO_MIPS_PIC is localized using objcopy, binutils will ignore the flag. I don't think this is presently worth implementing but it might be wise to add an error message. I haven't done that yet. - Superfluous la25 stubs are suppressed when a PIC2 file uses jal. This is an optimization performed by gcc -mno-shared. It will not work after ld -r into a non-PIC file; the jump will appear to come from a non-PIC object and be redirected to a new stub. This is only a minor performance pessimization and I do not plan to fix it. Yeah, for the record, this second one was actually a deliberate choice. There's no real object-level information to indicate -mno-sharedness (unlike PICness), so any attempt to recognise it would simply be a heuristic. The ld -r support was really there for the same reason as the la $25 stubs: to allow real PIC and new non-PIC to be linked together. It seems unlikely that you'd have much -mno-shared code to link in if you're using a new-PIC-compatible sysroot. - It would be nice to generate, in some cases, both a .MIPS.stubs lazy binding stub and a PLT entry. However, I determined that considerable additional work would be required to do this; most likely we'd need two entries for the same symbol in the dynamic symbol table so that the GOT entry could be associated with the lazy loading stub. As things stand it is possible to link non-PIC and PIC code together, and if both call the same function and the non-PIC code takes its address, calls to the function from PIC code will be penalized. I do not expect this case to matter. Most applications will be predominantly PIC or non-PIC. Yeah, FWIW, I think we discussed this in the original three-way thread last year. (I think the original proposal was to use PLTs all the time for new-PIC executables. I remember arguing in favour of keeping .MIPS.stubs as an option because they're more efficient when handling the cases they can. I certainly agree that's it not worth trying to use both .MIPS.stubs and PLTs for the same function.) - I've dropped support for a non-fixed $gp. This is a handy optimization, but it was getting in the way and it was the part of the GCC patch Richard had the most comments on. I can resubmit it after everything else is merged. That's a shame. It was also the bit I liked best ;) What went wrong? (My comments were only minor.) - Richard's implementation had __PIC__ mean abicalls. Our patch changed __PIC__ to mean pic2 abicalls only. I've included that in this patch. My reasoning is that most non-pic non-abicalls code works properly even with pic0 abicalls; the only problem is indirect calls through a register other than $25. This lets glibc automatically use some more efficient sequences in static applications. Hmm, OK. It's the less conservative choice, but I agree it's also the best performance-wise. - I added pointer_equality_needed support to binutils to suppress setting st_value to the PLT entry in most cases. - The GCC new-static-chain.patch causes nested-func-4.exe to fail. _mcount is called through a PLT entry, which clobbers $15. I believe we need to add this to MIPS_SAVE_REG_FOR_PROFILING_P. I didn't fix this yet. - no-fn-name-already-declared.patch removed the call to ASM_OUTPUT_TYPE_DIRECTIVE for Linux. .ent has similar effect, but is suppressed by flag_inhibit_size_directive. This caused glibc's _init to be STT_OBJECT, and not get a PIC call stub. I've changed GCC to emit .type for all platforms; Richard, if this should be restricted to the status quo (i.e. Linux) let me know. No, that sounds right to me FWIW. Also the STT_FUNC check in the linker was unnecessary now that we only use la25 stubs for jump relocations. Agreed. Sorry for the bugs, and thanks for fixing them. I'll try to have a look at the patches over the weekend. Richard
Re: RFC: Adding non-PIC executable support to MIPS
On Thu, Jul 24, 2008 at 09:24:48PM +0100, Richard Sandiford wrote: - I've dropped support for a non-fixed $gp. This is a handy optimization, but it was getting in the way and it was the part of the GCC patch Richard had the most comments on. I can resubmit it after everything else is merged. That's a shame. It was also the bit I liked best ;) What went wrong? (My comments were only minor.) Nothing big: but it made up the bulk of our GCC patch, and I was rebasing on top of yours. We had a lot of trouble getting that bit to work, so I took the conservative path and dropped it. Don't worry - I definitely will return to this as soon as the remaining patches are merged; I liked it too. Originally, we were also going to make small data work with non-PIC abicalls. I'd like to discuss that with you at some later date too. We ran into a lot of trouble combining small data with a non-fixed $gp; I believe it was because reload may introduce small data loads very late. I'd have to try it again before I had anything more sensible to say, though. Sorry for the bugs, and thanks for fixing them. I'll try to have a look at the patches over the weekend. No problem, and thank you for looking at them - and for your patches; I'm really pretty happy with the combined work. -- Daniel Jacobowitz CodeSourcery
Re: RFC: Adding non-PIC executable support to MIPS
On Tue, Jul 01, 2008 at 09:43:30PM +0100, Richard Sandiford wrote: I suppose I still support the trade-off between the 5-insn MIPS I stubs (with extra-long variation for large PLT indices) and the absolute .got.plt address I used. And I still think it's shame we're treating STO_MIPS_PLT and STO_MIPS16 as separate; we then only have 1 bit of st_other unclaimed. I'm undecided about the MIPS I issue, but I completely agree about STO_MIPS16/STO_MIPS_PLT. I wish we'd thought of that too. At least our implementation didn't have STO_MIPS_PIC; so there's one bit left, and assuming we add support for ld -r (likely) we can do it your way. For the final merged versions of these patches, even if they implement our version, I hope to draw heavily on your work. It's always high quality and the GOT cleanups in particular look very useful. TBH, the close relationship between CodeSourcery and MTI make it difficult for a non-Sourcerer and non-MTI employee to continue to be a MIPS maintainer. I won't be in-the-know about this sort of thing. I've been thinking about that a lot recently, since I heard about your implementation. I kind-of guessed it had been agreed with MTI beforehand (although I hadn't realised MTI themselves had written the specification). Having thought it over, I think it would be best if I stand down as a MIPS maintainer and if someone with the appropriate commercial connections is appointed instead. I'd recommend any combination of yourself, Adam Nemet and David Daney (subject to said people being willing, of course). I'm sorry you feel this way. I believe strongly that corporate affiliation is not a useful indicator for maintainership; the system we have set up here relies more on individual knowledge and experience than affiliation. We could have done more to keep everyone informed of our progress; I could write an essay on why we didn't, but I'd rather not. We're talking internally about how to avoid this unfortunate coincidence in the future. Anyway, there's plenty of blame to go around. I think you're doing an excellent job as a GCC maintainer, and so does everyone I spoke to about this at CS. If you no longer have the time or incentive to do it, I won't argue with you about stepping down, but please don't because of this incident. [In any case, I'd decline; I'm trying to shed some of my existing maintenance responsibilities so that I can spend more time on the ones I care most about. Anyone else want to be binutils release manager?] -- Daniel Jacobowitz CodeSourcery
Re: RFC: Adding non-PIC executable support to MIPS
Thanks to everyone for their kind messages. I won't drag this out for non-MIPS folk by replying publicly to each one. Daniel Jacobowitz [EMAIL PROTECTED] writes: the GOT cleanups in particular look very useful. Thanks. To be clear: the withdrawal was simply for the patches in this message. Although the original motivation for the GOT cleanups was to reduce the amount of wasted space in mostly-non-PIC executables, they're really a separate change in their own right. My hope was that, even without the non-PIC stuff, the new code might be more maintainable than what we have now. We could have done more to keep everyone informed of our progress; I could write an essay on why we didn't, but I'd rather not. We're talking internally about how to avoid this unfortunate coincidence in the future. Anyway, there's plenty of blame to go around. FWIW, I don't blame MTI or CS at all for this. Duplicated effort is part of the risk one runs with the model that both you (CS) and I were following. (And for the record, I say I because the fault was mine rather than Specifix's.) When I was doing the work, I was expecting to use the patches as the basis for a discussion on this list. And I honestly expected to have to change some of the details as a result. E.g. I wasn't sure what the reaction would be to requiring MIPS II or above. So it's no surprise that my version as posted is not going to be used. And when I learnt about your alternative implementation, I was expecting some of that implementation to be chosen instead. The difficulty was simply that, as you rightly said, MTI are the authority here. That made any discussion on this list moot. That was just an attempt to clarify things rather than force you into writing an essay ;) Anyway, enough of that. Back to technical stuff. Would it work if we had stubs like this: lui t7,%hi(.got.plt entry) lw t9,%lo(.got.plt entry)(t7) addiu t8,t7,%lo(.got.plt entry) jr t9 ... lui t7,%hi(.got.plt entry + 4) [next entry] and a header like this: lui gp,%hi(.got.plt) lw t9,%lo(.got.plt)(gp) addiu gp,gp,%lo(.got.plt) subut8,t8,gp movet7,ra srl t8,t8,2 jalrt9 subut8,t8,2 (Key for my benefit, 'cos I can only think in terms of numerical registers: t7 = $15 t8 = $24 t9 = $25) The size of the header and first 0x1 stubs would be the same. I think it would also preserve the resolver interface while removing the need for the extra-large .plts. The only incompatibility I can see would be that objdump on older executables would not get the [EMAIL PROTECTED] symbols right for large indices. OTOH, perhaps you could argue that the extra complication of the two PLT entries doesn't count for much given that the code is already written. It's just an idea. Richard
Re: RFC: Adding non-PIC executable support to MIPS
On Wed, Jul 02, 2008 at 08:55:54PM +0100, Richard Sandiford wrote: The size of the header and first 0x1 stubs would be the same. I think it would also preserve the resolver interface while removing the need for the extra-large .plts. The only incompatibility I can see would be that objdump on older executables would not get the [EMAIL PROTECTED] symbols right for large indices. OTOH, perhaps you could argue that the extra complication of the two PLT entries doesn't count for much given that the code is already written. It's just an idea. Your version looks fine to me, it's ABI-preserving, the PLT entries still work for MIPS I and still have the same runtime cost when not resolving. I like it - thanks! I'm not worried about making people upgrade objdump, either. -- Daniel Jacobowitz CodeSourcery
Re: RFC: Adding non-PIC executable support to MIPS
Daniel Jacobowitz [EMAIL PROTECTED] writes: We've shipped our version. Richard's version has presumably also shipped. Right. We did negotiate the ABI changes with MTI; this is not quite as good as doing it in full view, but it was the best we could manage and MTI is as close to a central authority for the MIPS psABI as exists today. Richard, what are your thoughts on reconciling the differences? You can surely guess that I want to avoid changing our ABI now, even for relatively significant technical reasons - I'm all ears if there's a major reason, but in the comparisons I do not see one. I suppose I still support the trade-off between the 5-insn MIPS I stubs (with extra-long variation for large PLT indices) and the absolute .got.plt address I used. And I still think it's shame we're treating STO_MIPS_PLT and STO_MIPS16 as separate; we then only have 1 bit of st_other unclaimed. However, IMO, your argument about MTI being the central authority is a killer one. The purpose of the GNU tools should be to follow appropriate standards where applicable (and extend them where it seems wise). So from that point of view, I agree that the GNU tools should follow the ABI that Nigel and MTI set down. Consider my patch withdrawn. TBH, the close relationship between CodeSourcery and MTI make it difficult for a non-Sourcerer and non-MTI employee to continue to be a MIPS maintainer. I won't be in-the-know about this sort of thing. I've been thinking about that a lot recently, since I heard about your implementation. I kind-of guessed it had been agreed with MTI beforehand (although I hadn't realised MTI themselves had written the specification). Having thought it over, I think it would be best if I stand down as a MIPS maintainer and if someone with the appropriate commercial connections is appointed instead. I'd recommend any combination of yourself, Adam Nemet and David Daney (subject to said people being willing, of course). Richard
Re: RFC: Adding non-PIC executable support to MIPS
Richard Sandiford [EMAIL PROTECTED] writes: I've been thinking about that a lot recently, since I heard about your implementation. I kind-of guessed it had been agreed with MTI beforehand (although I hadn't realised MTI themselves had written the specification). Having thought it over, I think it would be best if I stand down as a MIPS maintainer and if someone with the appropriate commercial connections is appointed instead. I'd recommend any combination of yourself, Adam Nemet and David Daney (subject to said people being willing, of course). I realised afterwards that this might be offensive by who it left out. For the record, it wasn't supposed to be an exclusive list. Other people have strong claims too. ;) Richard
Re: RFC: Adding non-PIC executable support to MIPS
On Mon, Jun 30, 2008 at 01:59:19PM -0700, David VomLehn wrote: This sounds like really good stuff and, on first reading, it all seems to make sense to me. My only real concern is documentation of these changes. FWIW, I'll be posting our version of this project shortly, and it includes an ABI supplement. Supplemental to a somewhat hypothetical document, but there you go... -- Daniel Jacobowitz CodeSourcery
