Richard Sandiford writes: > Thiemo Seufer writes: >> Richard Sandiford wrote: >>> David Daney writes: >>> > ! #define MIPS_FUNCTION_STUB_SIZE(INFO) \ >>> > ! (elf_hash_table (INFO)->dynsymcount > 65536 ? 20 : 16) >>> >>> Sorry to be a pain, but as I said earlier, I really do think we should >>> cache the chosen stub size in mips_elf_link_hash_table (and get rid of >>> this macro entirely). That will emphasise that always_size_dynamic_sections >>> is the place that makes the decision, and that it's only safe to use this >>> value once that function has been called. I think that will be more robust >>> and easier to understand in future. >>> >>> Apart from that, and from Thiemo's and Daniel's comments, this looks >>> really good to me. Thanks a lot for doing this! >> >> Does this followup patch look ok? > > Looks good, apart from the fact that you're setting function_stub_size > in _bfd_mips_elf_create_dynamic_sections rather than the suggested > always_size_dynamic_sections. Was there a particular reason for that? > As David says, create_dynamic_sections is too early; AIUI, > create_dynamic_sections is called as soon as we know we need > dynamic sections, so the value of dynsymcount at that point is > going to be pretty arbitrary. > > The reason I suggested always_size_sections is that I think we should > determine the stub entry size and stub section size at the same time. > > Thanks for doing this though! I volunteer to come up with some > testcases tomorrow. OK, here are the testcases. They showed up a couple of problems: (1) We were using "li" for the lower half of 32-bit indexes, thus cancelling out the "lui"! Daniel quoted the definition of STUB_LI16U and asked about whether it should be "ori", but I think there was some confusion. We don't want to rename STUB_LI16U to STUB_ORI -- it's fine as it is -- but we _do_ want a separate STUB_ORI for "ori t8,t8,foo" (as opposed to "ori t8,zero,foo"). (2) dynsymcount doesn't include the local section symbols at the time that the MIPS section-sizing code is run. Although the exact number of such symbols isn't known at that stage, we can get a conservative estimate by reusing a loop in _bfd_mips_elf_final_link. This loop was originally copied from _bfd_elf_link_renumber_dynsyms. We should try to reuse code between the two, as the current MIPS version is already out-of-sync, but I'd rather do that separately, as it touches non-MIPS code. Because the estimate in (2) is conservative, we can't insist that a file with 0x10000 dynsyms use 16-byte stubs. The tests therefore test a file with 0xfff1 dynsyms instead. We often seem to need to update these kinds of tests after changes to target-independent code. I've tried to make that easier by including a base_syms variable. See the comments for details. I've attached two patches. The first is relative to Thiemo's patch from yesterday; the second is a complete patch against mainline. Tested on mipsel-linux-gnu, mips64-linux-gnu and mipsisa64-elf. OK to install? bfd/ * elfxx-mips.c (mips_elf_link_hash_table): Add function_stub_size. (STUB_ORI): New macro. (STUB_LI16U): Fix formatting. (MIPS_FUNCTION_STUB_SIZE): Delete. (MIPS_FUNCTION_STUB_MAX_SIZE): Likewise. (MIPS_FUNCTION_STUB_NORMAL_SIZE): New macro. (MIPS_FUNCTION_STUB_BIG_SIZE): Likewise. (_bfd_mips_elf_adjust_dynamic_symbol): Use htab->function_stub_size instead of MIPS_FUNCTION_STUB_SIZE. (count_section_dynsyms): New function, split out from _bfd_mips_elf_final_link. (_bfd_mips_elf_always_size_sections): Get a worst-case estimate of the number of dynamic symbols needed and use it to set up function_stub_size. Use function_stub_size rather than MIPS_FUNCTION_STUB_SIZE to determine the size of the stub section. Use 16-byte stubs for 0x10000 dynamic symbols. (_bfd_mips_elf_size_dynamic_sections): Use htab->function_stub_size instead of MIPS_FUNCTION_STUB_SIZE. Fix formatting. (_bfd_mips_elf_finish_dynamic_symbol): Likewise. Change the size of the stub buffer from MIPS_FUNCTION_STUB_MAX_SIZE to MIPS_FUNCTION_STUB_BIG_SIZE. Tweak the check for unhandled dynindxes. Use MIPS_FUNCTION_STUB_BIG_SIZE rather than a hard-coded 20. Use STUB_ORI rather than STUB_LI16U for big stubs. (_bfd_mips_elf_link_hash_table_create): Initialize function_stub_size. (_bfd_mips_elf_final_link): Use count_section_dynsyms. ld/testsuite/ * ld-mips-elf/stub-dynsym-1.s, * ld-mips-elf/stub-dynsym-1.ld, * ld-mips-elf/stub-dynsym-1-7fff.d, * ld-mips-elf/stub-dynsym-1-8000.d, * ld-mips-elf/stub-dynsym-1-fff0.d, * ld-mips-elf/stub-dynsym-1-10000.d, * ld-mips-elf/stub-dynsym-1-2fe80.d: New test. * ld-mips-elf/mips-elf.exp: Run it.