Created attachment 9540 [details] src.cpp Debian bug: https://bugs.debian.org/834147 Moved from GCC: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77300 When compiling the attached testcase with: mipsel-linux-gnu-g++-6 -O2 -fPIC src.cpp I get: /usr/lib/gcc-cross/mipsel-linux-gnu/6/../../../../mipsel-linux-gnu/bin/ld: /tmp/ccqVuMV3.o: Can't find matching LO16 reloc against `_ZN1HIiE5m_fn4ERK1G.isra.1' for R_MIPS_GOT16 at 0xcc in section `.text._ZN1HIiE5m_fn5ERK1G[_ZN1HIiE5m_fn5ERK1G]' Originally I thought this was a bug in how GCC generates GOT16 relocations but it turns out it's probably a bug in GAS instead. See the GCC bug for more information.
Created attachment 9541 [details] src.s Assembly output from compiling the testcase with: mipsel-linux-gnu-g++-6 -O2 -fPIC -S src.cpp
Created attachment 9544 [details] Intended bug fix Thank you for your bug report. I have looked into this problem and it looks like a GAS regression to me, albeit a very old one. It was introduced with commit 8614eeee67f9, which unfortunately was bundled with an unrelated change, made with no discussion or justification posted, had no test case included, and only a terse notification was sent afterwards ("Traditional MIPS patches"), <https://sourceware.org/ml/binutils/2000-07/msg00018.html>. It made symbols in linkonce or what is these days known as comdat sections to be treated as external for the purpose of PIC relocation generation even if their binding remains STB_LOCAL. This in turn disabled GOT16/LO16 relocation pairing as no complementing LO16 relocation is expected for external GOT16 references in the o32 ABI. Please try the patch attached which seems to fix the issue for me and causes no regressions in `mips-linux-gnu' binutils testing. I'll be committing it as soon as I have suitable test cases made and it has passed full regression testing across the relevant targets. NB GCC has done a pretty decent job here as in actual code execution the GOT16 reference just below $L15 is actually paired with the complementing LO16 reference at $L17 as it's in a delay slot of a branch going to the latter location. So the GOT16 reference is not orphan even though it is duplicate from the ELF file's point of view. gas/ * config/tc-mips.c (pic_need_relax): Don't check for linkonce symbols, remove the `segtype' parameter. (mips_frob_file, md_estimate_size_before_relax): Adjust accordingly. (s_is_linkonce): Add an explanatory comment.
Thanks for the patch, it seems to work. I ran a full build of blender where the bug originally came from and it builds fine now. James
Maciej, is it OK for me to ask for your patch to be applied to the Debian binutils package?
Sure, the GNU GPL applies. Due to other commitments it'll take me a few days yet to get the test cases made, but I don't plan to change the code update itself any further, so any future merge from upstream Debian people will make should result in an easy to resolve "can be reverse-applied" result. NB I'm still waiting to have my Bugzilla permissions restored (there's been some confusion around it), which is why I wasn't able to update the status of this bug.
(In reply to Maciej W. Rozycki from comment #5) > Sure, the GNU GPL applies. Due to other commitments it'll take me a > few days yet to get the test cases made, but I don't plan to change the > code update itself any further, so any future merge from upstream > Debian people will make should result in an easy to resolve "can be > reverse-applied" result. > > NB I'm still waiting to have my Bugzilla permissions restored (there's > been some confusion around it), which is why I wasn't able to update > the status of this bug. Ping. Do you think this can be merged now? Thanks.
I yet need to integrate the test case with the test suite, but please be assured this fix will make it to 2.28.
(In reply to Maciej W. Rozycki from comment #7) > I yet need to integrate the test case with the test suite, > but please be assured this fix will make it to 2.28. Thanks!
The master branch has been updated by Maciej W. Rozycki <macro@sourceware.org>: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=9e009953a54bfbf79d83f37797f846c923aeea43 commit 9e009953a54bfbf79d83f37797f846c923aeea43 Author: Maciej W. Rozycki <macro@imgtec.com> Date: Wed Jan 18 18:18:21 2017 +0000 PR gas/20649: MIPS: Fix GOT16/LO16 reloc pairing with comdat sections Correct a regression from commit 8614eeee67f9 ("Traditional MIPS patches"), <https://sourceware.org/ml/binutils/2000-07/msg00018.html>, which caused symbols in linkonce or what is these days known as comdat sections to be treated as external for the purpose of PIC relocation generation even if their binding remains STB_LOCAL. This in turn disabled GOT16/LO16 relocation pairing with references to such symbols, as no complementing LO16 relocation is expected for external GOT16 references in the o32 ABI, which ultimately leads to link errors, e.g.: ld: comdat-reloc.o: Can't find matching LO16 reloc against `foo' for R_MIPS_GOT16 at 0x24 in section `.text.bar[bar]' as with the LD test case included with this change. Revert the special case for symbols in comdat sections then, making code actually match `adjust_reloc_syms' as indicated in its explanatory comment, and adjust calling code accordingly. Also bring back the corresponding description of what now is `s_is_linkonce', lost with commit 5f0fe04bc550 ("Improved MIPS16/MIPS32 code intermixing for gas."), <https://www.sourceware.org/ml/binutils/2006-07/msg00039.html>. gas/ PR gas/20649 * config/tc-mips.c (pic_need_relax): Don't check for linkonce symbols, remove the `segtype' parameter. (mips_frob_file, md_estimate_size_before_relax): Adjust accordingly. (s_is_linkonce): Add an explanatory comment. * testsuite/gas/mips/comdat-reloc.d: New test. * testsuite/gas/mips/comdat-reloc.s: New test source. * testsuite/gas/mips/mips.exp: Run the new test. ld/ PR gas/20649 * testsuite/ld-mips-elf/mips-elf.exp: Add PIC comdat GOT16/LO16 relocation pairing link test.
The binutils-2_28-branch branch has been updated by Maciej W. Rozycki <macro@sourceware.org>: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=65060a78866f374e25f4668d12efc783235d19d1 commit 65060a78866f374e25f4668d12efc783235d19d1 Author: Maciej W. Rozycki <macro@imgtec.com> Date: Wed Jan 18 18:18:21 2017 +0000 PR gas/20649: MIPS: Fix GOT16/LO16 reloc pairing with comdat sections Correct a regression from commit 8614eeee67f9 ("Traditional MIPS patches"), <https://sourceware.org/ml/binutils/2000-07/msg00018.html>, which caused symbols in linkonce or what is these days known as comdat sections to be treated as external for the purpose of PIC relocation generation even if their binding remains STB_LOCAL. This in turn disabled GOT16/LO16 relocation pairing with references to such symbols, as no complementing LO16 relocation is expected for external GOT16 references in the o32 ABI, which ultimately leads to link errors, e.g.: ld: comdat-reloc.o: Can't find matching LO16 reloc against `foo' for R_MIPS_GOT16 at 0x24 in section `.text.bar[bar]' as with the LD test case included with this change. Revert the special case for symbols in comdat sections then, making code actually match `adjust_reloc_syms' as indicated in its explanatory comment, and adjust calling code accordingly. Also bring back the corresponding description of what now is `s_is_linkonce', lost with commit 5f0fe04bc550 ("Improved MIPS16/MIPS32 code intermixing for gas."), <https://www.sourceware.org/ml/binutils/2006-07/msg00039.html>. gas/ PR gas/20649 * config/tc-mips.c (pic_need_relax): Don't check for linkonce symbols, remove the `segtype' parameter. (mips_frob_file, md_estimate_size_before_relax): Adjust accordingly. (s_is_linkonce): Add an explanatory comment. * testsuite/gas/mips/comdat-reloc.d: New test. * testsuite/gas/mips/comdat-reloc.s: New test source. * testsuite/gas/mips/mips.exp: Run the new test. ld/ PR gas/20649 * testsuite/ld-mips-elf/mips-elf.exp: Add PIC comdat GOT16/LO16 relocation pairing link test. (cherry picked from commit 9e009953a54bfbf79d83f37797f846c923aeea43)
Fixed as per the automatic commit messages.