| Summary: | wrong overflow check in R_MIPS_HI16 | ||
|---|---|---|---|
| Product: | binutils | Reporter: | ma.jiang |
| Component: | ld | Assignee: | Not yet assigned to anyone <unassigned> |
| Status: | CLOSED FIXED | ||
| Severity: | normal | CC: | nickc |
| Priority: | P2 | ||
| Version: | unspecified | ||
| Target Milestone: | --- | ||
| Host: | Target: | ||
| Build: | Last reconfirmed: | ||
| Attachments: |
source file
linker script |
||
Created attachment 7479 [details]
linker script
still no reply? I can not build my glibc(with -g3 option) due to this bug... Hi all,
This bug still exist in the trunk code. Using the attached files, I can still get the error "relocation truncated to fit: R_MIPS_HI16 against `_gp_disp'". This is *NOT* right, as the mips abi said that R_MIPS_HI16 need no overflow checks. Moreover, Only the o32 abi support R_MIPS_HI16 for _gp_disp, per the mips abi. R_MIPS_HI16 should never get overflowed, because under o32 abi, the address width is 32.
So, I think we should just get rid of the overflow check for R_MIPS_HI16. Here is the patch for trunk.
--- bfd/elfxx-mips.c.orig 2016-11-28 23:09:23.343671301 +0800
+++ bfd/elfxx-mips.c 2016-11-28 23:23:49.452670956 +0800
@@ -5875,7 +5875,6 @@ mips_elf_calculate_relocation (bfd *abfd
value = mips_elf_high (addend + gp - p - 1);
else
value = mips_elf_high (addend + gp - p);
- overflowed_p = mips_elf_overflow_p (value, 16);
}
break;
The master branch has been updated by Nick Clifton <nickc@sourceware.org>: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=40a0bfddf07620f5321927b3231502debb3b73bc commit 40a0bfddf07620f5321927b3231502debb3b73bc Author: Ma Jiang <ma.jiang@zte.com.cn> Date: Thu Dec 1 12:21:30 2016 +0000 Fix handling of MIPS16 HI16 relocs. PR ld/16720 * elfxx-mips.c (mips_elf_calculate_relocation): Remove overflow test for HI16 relocs. Hi Ma, Sorry for the delay in reviewing this PR. I agree with your analysis, so I have gone ahead and checked in your suggested patch. Thanks very much for creating it! Cheers Nick (In reply to Nick Clifton from comment #5) > Hi Ma, > > Sorry for the delay in reviewing this PR. > > I agree with your analysis, so I have gone ahead and checked in your > suggested patch. Thanks very much for creating it! > > Cheers > Nick Hi Nick， It's happy to see this local patch go upstream. Thanks, I'll close the bug now. The master branch has been updated by Faraz Shahbazker <farazs@sourceware.org>: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=1e129bbefadbf09ace0fc7fcb3cfcda13700e3b8 commit 1e129bbefadbf09ace0fc7fcb3cfcda13700e3b8 Author: Faraz Shahbazker <fshahbazker@wavecomp.com> Date: Thu May 23 18:16:08 2019 -0700 MIPS/LD: Skip overflow check for %pcrel_hi relocations Overflow checks were removed for all hi16 relocations except PC-relative high relocations per PR ld/16720. Remove overflow checks from %pcrel_hi relocations so that we can correctly handle negative offsets from PC. bfd/ * elfxx-mips.c (mips_elf_calculate_relocation) <R_MIPS_PCHI16>: Remove overflow check. ld/ * testsuite/ld-mips-elf/undefweak-overflow.s: Remove test case for pcrel_hi/pcrel_lo. * testsuite/ld-mips-elf/undefweak-overflow.d: Update to match. * testsuite/ld-mips-elf/reloc-pcrel-r6.s: New test source. * testsuite/ld-mips-elf/reloc-pcrel-r6.d: New test linker script. * testsuite/ld-mips-elf/reloc-pcrel-r6.ld: New test. * testsuite/ld-mips-elf/mips-elf.exp: Run the new test. |
Created attachment 7478 [details] source file There is a overflow check in mips ld. ================================================================= if (r_type == R_MIPS16_HI16) value = mips_elf_high (addend + gp - p - 4); else value = mips_elf_high (addend + gp - p); overflowed_p = mips_elf_overflow_p (value, 16); ================================================================= This check might have some problems when "addend + gp - p" is a negative number.In my cases, I got "addend + gp - p=-132666256".This number should be ok for a "R_MIPS16_HI16+R_MIPS16_LO16" as it obviously could be put into a 32bits-signed-int. But, the ld throw a overflow error. First, it get a value=63512 from mips_elf_high, then it check if this value could be put into a 16bits-signed-int in mips_elf_overflow_p. And of course, 63512 can not be put into a 16bits-signed-int.So，a wrong overflow error is generated finally. In my opinion, we only need to check whether "addend + gp - p" could be put into a 32bits-signed-int in R_MIPS16_HI16. Because, a 32bits-signed-int can be expressed correctly by R_MIPS16_HI16+R_MIPS16_LO16. The code could be like: bfd_vma offset; if (r_type == R_MIPS16_HI16) { value = mips_elf_high (addend + gp - p - 4); offset = addend + gp - p - 4; } else { value = mips_elf_high (addend + gp - p); offset = addend + gp - p; } overflowed_p = mips_elf_overflow_p (offset, 32); **************************************************************************** This bug can be reproduced by attached files, using commands like: gcc ldtest.c -o ldtest -Wl,-T bug.lds -static -fPIC