* [PATCH 1/2] LoongArch: Fix instruction immediate bug caused by sign-extend
@ 2023-07-17 8:22 mengqinggang
2023-07-17 8:22 ` [PATCH 2/2] LoongArch: Fix immediate overflow check bug mengqinggang
0 siblings, 1 reply; 7+ messages in thread
From: mengqinggang @ 2023-07-17 8:22 UTC (permalink / raw)
To: binutils
Cc: xuchenghua, chenglulu, liuzhensong, xry111, i.swmail, maskray,
hejinyang, mengqinggang
For extreme code mode, the instruction sequences is
pcalau12i $t0, hi20
addi.d $t1, $zero, lo12
lu32i.d $t1, lo20
lu52i.d $t1, hi12
add.d $t1, $t0, $t1
If lo12 > 0x7ff, hi20 need to add 0x1, lo20 need to sub 0x1.
If hi20 > 0x7ffff, lo20 need to add 0x1.
bfd/ChangeLog:
* elfnn-loongarch.c (RELOCATE_CALC_PC32_HI20): Redefined.
(RELOCATE_CALC_PC64_HI32): Redefined.
---
bfd/elfnn-loongarch.c | 59 +++++++++++++++++++++++++++++++++++--------
1 file changed, 49 insertions(+), 10 deletions(-)
diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
index d3d8419d80b..e9c408b2dff 100644
--- a/bfd/elfnn-loongarch.c
+++ b/bfd/elfnn-loongarch.c
@@ -2284,26 +2284,65 @@ loongarch_reloc_is_fatal (struct bfd_link_info *info,
return fatal;
}
+/* If lo12 immediate > 0x7ff, because sign-extend caused by addi.d/ld.d,
+ hi20 immediate need to add 0x1.
+ For example: pc 0x120000000, symbol 0x120000812
+ lo12 immediate is 0x812, 0x120000812 & 0xfff = 0x812
+ hi20 immediate is 1, because lo12 imm > 0x7ff, symbol need to add 0x1000
+ (((0x120000812 + 0x1000) & ~0xfff) - (0x120000000 & ~0xfff)) >> 12 = 0x1
+
+ At run:
+ pcalau12i $t0, hi20 (0x1)
+ $t0 = 0x120000000 + (0x1 << 12) = 0x120001000
+ addi.d $t0, $t0, lo12 (0x812)
+ $t0 = 0x120001000 + 0xfffffffffffff812 (-(0x1000 - 0x812) = -0x7ee)
+ = 0x120001000 - 0x7ee (0x1000 - 0x7ee = 0x812)
+ = 0x120000812
+ Without hi20 add 0x1000, the result 0x120000000 - 0x7ee = 0x11ffff812 is
+ error.
+ 0x1000 + sign-extend-to64(0x8xx) = 0x8xx. */
#define RELOCATE_CALC_PC32_HI20(relocation, pc) \
({ \
bfd_vma __lo = (relocation) & ((bfd_vma)0xfff); \
- pc = pc & (~(bfd_vma)0xfff); \
+ relocation = (relocation & ~(bfd_vma)0xfff) \
+ - (pc & ~(bfd_vma)0xfff); \
if (__lo > 0x7ff) \
- { \
relocation += 0x1000; \
- } \
- relocation &= ~(bfd_vma)0xfff; \
- relocation -= pc; \
})
+/* For example: pc is 0x11000010000100, symbol is 0x1812348ffff812
+ offset = (0x1812348ffff812 & ~0xfff) - (0x11000010000100 & ~0xfff)
+ = 0x712347ffff000
+ lo12: 0x1812348ffff812 & 0xfff = 0x812
+ hi20: 0x7ffff + 0x1(lo12 > 0x7ff) = 0x80000
+ lo20: 0x71234 - 0x1(lo12 > 0x7ff) + 0x1(hi20 > 0x7ffff)
+ hi12: 0x0
+
+ pcalau12i $t1, hi20 (0x80000)
+ $t1 = 0x11000010000100 + sign-extend(0x80000 << 12)
+ = 0x11000010000100 + 0xffffffff80000000
+ = 0x10ffff90000000
+ addi.d $t0, $zero, lo12 (0x812)
+ $t0 = 0xfffffffffffff812 (if lo12 > 0x7ff, because sign-extend,
+ lo20 need to sub 0x1)
+ lu32i.d $t0, lo12 (0x71234)
+ $t0 = {0x71234, 0xfffff812}
+ = 0x71234fffff812
+ lu52i.d $t0, hi12 (0x0)
+ $t0 = {0x0, 0x71234fffff812}
+ = 0x71234fffff812
+ add.d $t1, $t1, $t0
+ $t1 = 0x10ffff90000000 + 0x71234fffff812
+ = 0x1812348ffff812. */
#define RELOCATE_CALC_PC64_HI32(relocation, pc) \
({ \
- bfd_vma __lo = (relocation) & ((bfd_vma)0xfff); \
+ bfd_vma __lo = (relocation & (bfd_vma)0xfff); \
+ relocation = (relocation & ~(bfd_vma)0xfff) \
+ - (pc & ~(bfd_vma)0xfff); \
if (__lo > 0x7ff) \
- { \
- relocation -= 0x100000000; \
- } \
- relocation -= (pc & ~(bfd_vma)0xffffffff); \
+ relocation += (0x1000 - 0x100000000); \
+ if (relocation & 0x80000000) \
+ relocation += 0x100000000; \
})
static int
--
2.36.0
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 2/2] LoongArch: Fix immediate overflow check bug 2023-07-17 8:22 [PATCH 1/2] LoongArch: Fix instruction immediate bug caused by sign-extend mengqinggang @ 2023-07-17 8:22 ` mengqinggang 2023-07-21 3:56 ` mengqinggang 0 siblings, 1 reply; 7+ messages in thread From: mengqinggang @ 2023-07-17 8:22 UTC (permalink / raw) To: binutils Cc: xuchenghua, chenglulu, liuzhensong, xry111, i.swmail, maskray, hejinyang, mengqinggang For B16/B21/B26/PCREL20_S2 relocations, if immediate overflow check after rightshift, and the mask need to include sign bit. Now, the immediate overflow check before rightshift for easier understand. bfd/ChangeLog: * elfxx-loongarch.c (reloc_bits_pcrel20_s2): Delete. (reloc_bits_b16): Delete. (reloc_bits_b21): Delete. (reloc_bits_b26): Delete. (reloc_sign_bits): New. --- bfd/elfxx-loongarch.c | 160 +++++++++--------------------------------- 1 file changed, 32 insertions(+), 128 deletions(-) diff --git a/bfd/elfxx-loongarch.c b/bfd/elfxx-loongarch.c index da440d55c3c..2d299050c12 100644 --- a/bfd/elfxx-loongarch.c +++ b/bfd/elfxx-loongarch.c @@ -54,13 +54,7 @@ typedef struct loongarch_reloc_howto_type_struct static bool reloc_bits (bfd *abfd, reloc_howto_type *howto, bfd_vma *val); static bool -reloc_bits_pcrel20_s2 (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val); -static bool -reloc_bits_b16 (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val); -static bool -reloc_bits_b21 (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val); -static bool -reloc_bits_b26 (bfd *abfd, reloc_howto_type *howto, bfd_vma *val); +reloc_sign_bits (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val); static bfd_reloc_status_type loongarch_elf_add_sub_reloc (bfd *, arelent *, asymbol *, void *, @@ -457,7 +451,7 @@ static loongarch_reloc_howto_type loongarch_howto_table[] = 0x3fffc00, /* dst_mask */ false, /* pcrel_offset */ BFD_RELOC_LARCH_SOP_POP_32_S_10_16_S2, /* bfd_reloc_code_real_type */ - reloc_bits_b16, /* adjust_reloc_bits */ + reloc_sign_bits, /* adjust_reloc_bits */ NULL), /* larch_reloc_type_name */ LOONGARCH_HOWTO (R_LARCH_SOP_POP_32_S_5_20, /* type (43). */ @@ -493,7 +487,7 @@ static loongarch_reloc_howto_type loongarch_howto_table[] = false, /* pcrel_offset */ BFD_RELOC_LARCH_SOP_POP_32_S_0_5_10_16_S2, /* bfd_reloc_code_real_type */ - reloc_bits_b21, /* adjust_reloc_bits */ + reloc_sign_bits, /* adjust_reloc_bits */ NULL), /* larch_reloc_type_name */ LOONGARCH_HOWTO (R_LARCH_SOP_POP_32_S_0_10_10_16_S2, /* type (45). */ @@ -511,7 +505,7 @@ static loongarch_reloc_howto_type loongarch_howto_table[] = false, /* pcrel_offset */ BFD_RELOC_LARCH_SOP_POP_32_S_0_10_10_16_S2, /* bfd_reloc_code_real_type */ - reloc_bits_b26, /* adjust_reloc_bits */ + reloc_sign_bits, /* adjust_reloc_bits */ NULL), /* larch_reloc_type_name */ LOONGARCH_HOWTO (R_LARCH_SOP_POP_32_U, /* type (46). */ @@ -766,7 +760,7 @@ static loongarch_reloc_howto_type loongarch_howto_table[] = 0x3fffc00, /* dst_mask. */ false, /* pcrel_offset. */ BFD_RELOC_LARCH_B16, /* bfd_reloc_code_real_type. */ - reloc_bits_b16, /* adjust_reloc_bits. */ + reloc_sign_bits, /* adjust_reloc_bits. */ "b16"), /* larch_reloc_type_name. */ LOONGARCH_HOWTO (R_LARCH_B21, /* type (65). */ @@ -783,7 +777,7 @@ static loongarch_reloc_howto_type loongarch_howto_table[] = 0x3fffc1f, /* dst_mask. */ false, /* pcrel_offset. */ BFD_RELOC_LARCH_B21, /* bfd_reloc_code_real_type. */ - reloc_bits_b21, /* adjust_reloc_bits. */ + reloc_sign_bits, /* adjust_reloc_bits. */ "b21"), /* larch_reloc_type_name. */ LOONGARCH_HOWTO (R_LARCH_B26, /* type (66). */ @@ -800,7 +794,7 @@ static loongarch_reloc_howto_type loongarch_howto_table[] = 0x03ffffff, /* dst_mask. */ false, /* pcrel_offset. */ BFD_RELOC_LARCH_B26, /* bfd_reloc_code_real_type. */ - reloc_bits_b26, /* adjust_reloc_bits. */ + reloc_sign_bits, /* adjust_reloc_bits. */ "b26"), /* larch_reloc_type_name. */ LOONGARCH_HOWTO (R_LARCH_ABS_HI20, /* type (67). */ @@ -1436,7 +1430,7 @@ static loongarch_reloc_howto_type loongarch_howto_table[] = 0x1ffffe0, /* dst_mask. */ false, /* pcrel_offset. */ BFD_RELOC_LARCH_PCREL20_S2, /* bfd_reloc_code_real_type. */ - reloc_bits_pcrel20_s2, /* adjust_reloc_bits. */ + reloc_sign_bits, /* adjust_reloc_bits. */ NULL), /* larch_reloc_type_name. */ /* Canonical Frame Address. */ @@ -1677,12 +1671,14 @@ reloc_bits (bfd *abfd ATTRIBUTE_UNUSED, } static bool -reloc_bits_pcrel20_s2 (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val) +reloc_sign_bits (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val) { + if (howto->complain_on_overflow != complain_overflow_signed) + return false; + bfd_signed_vma val = (bfd_signed_vma)(*fix_val); - bfd_signed_vma mask = ((bfd_signed_vma)0x1 << howto->bitsize) - 1; - /* Check rightshift. */ + /* Check alignment. FIXME: if rightshift is not alingment. */ if (howto->rightshift && (val & ((((bfd_signed_vma) 1) << howto->rightshift) - 1))) { @@ -1692,8 +1688,12 @@ reloc_bits_pcrel20_s2 (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val) return false; } - val = val >> howto->rightshift; + bfd_signed_vma mask = ((bfd_signed_vma)0x1 << (howto->bitsize + + howto->rightshift - 1)) - 1; + /* Positive number: high part is all 0; + Negative number: if high part is not all 0, high part must be all 1. + high part: from sign bit to highest bit. */ if ((val & ~mask) && ((val & ~mask) != ~mask)) { (*_bfd_error_handler) (_("%pB: relocation %s overflow 0x%lx"), @@ -1702,123 +1702,27 @@ reloc_bits_pcrel20_s2 (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val) return false; } - /* Perform insn bits field. */ - val = val & mask; - val <<= howto->bitpos; - - *fix_val = (bfd_vma)val; - - return true; -} - - -/* Adjust val to perform insn - R_LARCH_SOP_POP_32_S_10_16_S2 - R_LARCH_B16. */ -static bool -reloc_bits_b16 (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val) -{ - bfd_signed_vma val = *fix_val; - bfd_signed_vma mask = ((bfd_signed_vma)0x1 << howto->bitsize) - 1; - - if (howto->complain_on_overflow != complain_overflow_signed) - return false; - - /* Judge whether 4 bytes align. */ - if (val & ((0x1UL << howto->rightshift) - 1)) - return false; - - val = val >> howto->rightshift; - - if ((val & ~mask) && ((val & ~mask) != ~mask)) - { - (*_bfd_error_handler) (_("%pB: relocation %s overflow 0x%lx"), - abfd, howto->name, (long) val); - bfd_set_error (bfd_error_bad_value); - return false; - } - - /* Perform insn bits field. */ - val = val & mask; - val <<= howto->bitpos; - - *fix_val = val; - - return true; -} - -/* Reloc type : - R_LARCH_SOP_POP_32_S_0_5_10_16_S2 - R_LARCH_B21. */ -static bool -reloc_bits_b21 (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val) -{ - bfd_signed_vma val = *fix_val; - bfd_signed_vma mask = ((bfd_signed_vma)0x1 << howto->bitsize) - 1; - - if (howto->complain_on_overflow != complain_overflow_signed) - return false; - - /* Judge whether 4 bytes align. */ - if (val & ((0x1UL << howto->rightshift) - 1)) - return false; - val = val >> howto->rightshift; - - if ((val & ~mask) && ((val & ~mask) != ~mask)) - { - (*_bfd_error_handler) (_("%pB: relocation %s overflow 0x%lx"), - abfd, howto->name, (long) val); - bfd_set_error (bfd_error_bad_value); - return false; - } - - /* Perform insn bits field. */ + /* can delete? */ + mask = ((bfd_signed_vma)0x1 << howto->bitsize) - 1; val = val & mask; - /* Perform insn bits field. 15:0<<10, 20:16>>16. */ - val = ((val & 0xffff) << 10) | ((val >> 16) & 0x1f); - - *fix_val = val; - - return true; -} - -/* Reloc type: - R_LARCH_SOP_POP_32_S_0_10_10_16_S2 - R_LARCH_B26. */ -static bool -reloc_bits_b26 (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val) -{ - bfd_signed_vma val = *fix_val; - bfd_signed_vma mask = ((bfd_signed_vma)0x1 << howto->bitsize) - 1; - - if (howto->complain_on_overflow != complain_overflow_signed) - return false; - - /* Judge whether 4 bytes align. */ - if (val & ((0x1UL << howto->rightshift) - 1)) - return false; - - val = val >> howto->rightshift; - - if ((val & ~mask) && ((val & ~mask) != ~mask)) + switch (howto->type) { - (*_bfd_error_handler) (_("%pB: relocation %s overflow 0x%lx"), - abfd, howto->name, (long) val); - bfd_set_error (bfd_error_bad_value); - return false; + case R_LARCH_SOP_POP_32_S_0_10_10_16_S2: + case R_LARCH_B26: + /* Perform insn bits field. 25:16>>16, 15:0<<10. */ + val = ((val & 0xffff) << 10) | ((val >> 16) & 0x3ff); + break; + case R_LARCH_B21: + val = ((val & 0xffff) << 10) | ((val >> 16) & 0x1f); + break; + default: + val <<= howto->bitpos; + break; } - - /* Perform insn bits field. */ - val = val & mask; - - /* Perform insn bits field. 25:16>>16, 15:0<<10. */ - val = ((val & 0xffff) << 10) | ((val >> 16) & 0x3ff); - *fix_val = val; - return true; } -- 2.36.0 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] LoongArch: Fix immediate overflow check bug 2023-07-17 8:22 ` [PATCH 2/2] LoongArch: Fix immediate overflow check bug mengqinggang @ 2023-07-21 3:56 ` mengqinggang 2023-07-22 7:47 ` Xi Ruoyao 2023-07-22 7:52 ` Xi Ruoyao 0 siblings, 2 replies; 7+ messages in thread From: mengqinggang @ 2023-07-21 3:56 UTC (permalink / raw) To: binutils Cc: xuchenghua, chenglulu, liuzhensong, xry111, i.swmail, maskray, hejinyang Hi These two patches also need to be merged into the 2.41 branch. Thanks. 在 2023/7/17 下午4:22, mengqinggang 写道: > For B16/B21/B26/PCREL20_S2 relocations, if immediate overflow check after > rightshift, and the mask need to include sign bit. > > Now, the immediate overflow check before rightshift for easier understand. > > bfd/ChangeLog: > > * elfxx-loongarch.c (reloc_bits_pcrel20_s2): Delete. > (reloc_bits_b16): Delete. > (reloc_bits_b21): Delete. > (reloc_bits_b26): Delete. > (reloc_sign_bits): New. > --- > bfd/elfxx-loongarch.c | 160 +++++++++--------------------------------- > 1 file changed, 32 insertions(+), 128 deletions(-) > > diff --git a/bfd/elfxx-loongarch.c b/bfd/elfxx-loongarch.c > index da440d55c3c..2d299050c12 100644 > --- a/bfd/elfxx-loongarch.c > +++ b/bfd/elfxx-loongarch.c > @@ -54,13 +54,7 @@ typedef struct loongarch_reloc_howto_type_struct > static bool > reloc_bits (bfd *abfd, reloc_howto_type *howto, bfd_vma *val); > static bool > -reloc_bits_pcrel20_s2 (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val); > -static bool > -reloc_bits_b16 (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val); > -static bool > -reloc_bits_b21 (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val); > -static bool > -reloc_bits_b26 (bfd *abfd, reloc_howto_type *howto, bfd_vma *val); > +reloc_sign_bits (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val); > > static bfd_reloc_status_type > loongarch_elf_add_sub_reloc (bfd *, arelent *, asymbol *, void *, > @@ -457,7 +451,7 @@ static loongarch_reloc_howto_type loongarch_howto_table[] = > 0x3fffc00, /* dst_mask */ > false, /* pcrel_offset */ > BFD_RELOC_LARCH_SOP_POP_32_S_10_16_S2, /* bfd_reloc_code_real_type */ > - reloc_bits_b16, /* adjust_reloc_bits */ > + reloc_sign_bits, /* adjust_reloc_bits */ > NULL), /* larch_reloc_type_name */ > > LOONGARCH_HOWTO (R_LARCH_SOP_POP_32_S_5_20, /* type (43). */ > @@ -493,7 +487,7 @@ static loongarch_reloc_howto_type loongarch_howto_table[] = > false, /* pcrel_offset */ > BFD_RELOC_LARCH_SOP_POP_32_S_0_5_10_16_S2, > /* bfd_reloc_code_real_type */ > - reloc_bits_b21, /* adjust_reloc_bits */ > + reloc_sign_bits, /* adjust_reloc_bits */ > NULL), /* larch_reloc_type_name */ > > LOONGARCH_HOWTO (R_LARCH_SOP_POP_32_S_0_10_10_16_S2, /* type (45). */ > @@ -511,7 +505,7 @@ static loongarch_reloc_howto_type loongarch_howto_table[] = > false, /* pcrel_offset */ > BFD_RELOC_LARCH_SOP_POP_32_S_0_10_10_16_S2, > /* bfd_reloc_code_real_type */ > - reloc_bits_b26, /* adjust_reloc_bits */ > + reloc_sign_bits, /* adjust_reloc_bits */ > NULL), /* larch_reloc_type_name */ > > LOONGARCH_HOWTO (R_LARCH_SOP_POP_32_U, /* type (46). */ > @@ -766,7 +760,7 @@ static loongarch_reloc_howto_type loongarch_howto_table[] = > 0x3fffc00, /* dst_mask. */ > false, /* pcrel_offset. */ > BFD_RELOC_LARCH_B16, /* bfd_reloc_code_real_type. */ > - reloc_bits_b16, /* adjust_reloc_bits. */ > + reloc_sign_bits, /* adjust_reloc_bits. */ > "b16"), /* larch_reloc_type_name. */ > > LOONGARCH_HOWTO (R_LARCH_B21, /* type (65). */ > @@ -783,7 +777,7 @@ static loongarch_reloc_howto_type loongarch_howto_table[] = > 0x3fffc1f, /* dst_mask. */ > false, /* pcrel_offset. */ > BFD_RELOC_LARCH_B21, /* bfd_reloc_code_real_type. */ > - reloc_bits_b21, /* adjust_reloc_bits. */ > + reloc_sign_bits, /* adjust_reloc_bits. */ > "b21"), /* larch_reloc_type_name. */ > > LOONGARCH_HOWTO (R_LARCH_B26, /* type (66). */ > @@ -800,7 +794,7 @@ static loongarch_reloc_howto_type loongarch_howto_table[] = > 0x03ffffff, /* dst_mask. */ > false, /* pcrel_offset. */ > BFD_RELOC_LARCH_B26, /* bfd_reloc_code_real_type. */ > - reloc_bits_b26, /* adjust_reloc_bits. */ > + reloc_sign_bits, /* adjust_reloc_bits. */ > "b26"), /* larch_reloc_type_name. */ > > LOONGARCH_HOWTO (R_LARCH_ABS_HI20, /* type (67). */ > @@ -1436,7 +1430,7 @@ static loongarch_reloc_howto_type loongarch_howto_table[] = > 0x1ffffe0, /* dst_mask. */ > false, /* pcrel_offset. */ > BFD_RELOC_LARCH_PCREL20_S2, /* bfd_reloc_code_real_type. */ > - reloc_bits_pcrel20_s2, /* adjust_reloc_bits. */ > + reloc_sign_bits, /* adjust_reloc_bits. */ > NULL), /* larch_reloc_type_name. */ > > /* Canonical Frame Address. */ > @@ -1677,12 +1671,14 @@ reloc_bits (bfd *abfd ATTRIBUTE_UNUSED, > } > > static bool > -reloc_bits_pcrel20_s2 (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val) > +reloc_sign_bits (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val) > { > + if (howto->complain_on_overflow != complain_overflow_signed) > + return false; > + > bfd_signed_vma val = (bfd_signed_vma)(*fix_val); > - bfd_signed_vma mask = ((bfd_signed_vma)0x1 << howto->bitsize) - 1; > > - /* Check rightshift. */ > + /* Check alignment. FIXME: if rightshift is not alingment. */ > if (howto->rightshift > && (val & ((((bfd_signed_vma) 1) << howto->rightshift) - 1))) > { > @@ -1692,8 +1688,12 @@ reloc_bits_pcrel20_s2 (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val) > return false; > } > > - val = val >> howto->rightshift; > + bfd_signed_vma mask = ((bfd_signed_vma)0x1 << (howto->bitsize > + + howto->rightshift - 1)) - 1; > > + /* Positive number: high part is all 0; > + Negative number: if high part is not all 0, high part must be all 1. > + high part: from sign bit to highest bit. */ > if ((val & ~mask) && ((val & ~mask) != ~mask)) > { > (*_bfd_error_handler) (_("%pB: relocation %s overflow 0x%lx"), > @@ -1702,123 +1702,27 @@ reloc_bits_pcrel20_s2 (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val) > return false; > } > > - /* Perform insn bits field. */ > - val = val & mask; > - val <<= howto->bitpos; > - > - *fix_val = (bfd_vma)val; > - > - return true; > -} > - > - > -/* Adjust val to perform insn > - R_LARCH_SOP_POP_32_S_10_16_S2 > - R_LARCH_B16. */ > -static bool > -reloc_bits_b16 (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val) > -{ > - bfd_signed_vma val = *fix_val; > - bfd_signed_vma mask = ((bfd_signed_vma)0x1 << howto->bitsize) - 1; > - > - if (howto->complain_on_overflow != complain_overflow_signed) > - return false; > - > - /* Judge whether 4 bytes align. */ > - if (val & ((0x1UL << howto->rightshift) - 1)) > - return false; > - > - val = val >> howto->rightshift; > - > - if ((val & ~mask) && ((val & ~mask) != ~mask)) > - { > - (*_bfd_error_handler) (_("%pB: relocation %s overflow 0x%lx"), > - abfd, howto->name, (long) val); > - bfd_set_error (bfd_error_bad_value); > - return false; > - } > - > - /* Perform insn bits field. */ > - val = val & mask; > - val <<= howto->bitpos; > - > - *fix_val = val; > - > - return true; > -} > - > -/* Reloc type : > - R_LARCH_SOP_POP_32_S_0_5_10_16_S2 > - R_LARCH_B21. */ > -static bool > -reloc_bits_b21 (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val) > -{ > - bfd_signed_vma val = *fix_val; > - bfd_signed_vma mask = ((bfd_signed_vma)0x1 << howto->bitsize) - 1; > - > - if (howto->complain_on_overflow != complain_overflow_signed) > - return false; > - > - /* Judge whether 4 bytes align. */ > - if (val & ((0x1UL << howto->rightshift) - 1)) > - return false; > - > val = val >> howto->rightshift; > - > - if ((val & ~mask) && ((val & ~mask) != ~mask)) > - { > - (*_bfd_error_handler) (_("%pB: relocation %s overflow 0x%lx"), > - abfd, howto->name, (long) val); > - bfd_set_error (bfd_error_bad_value); > - return false; > - } > - > - /* Perform insn bits field. */ > + /* can delete? */ > + mask = ((bfd_signed_vma)0x1 << howto->bitsize) - 1; > val = val & mask; > > - /* Perform insn bits field. 15:0<<10, 20:16>>16. */ > - val = ((val & 0xffff) << 10) | ((val >> 16) & 0x1f); > - > - *fix_val = val; > - > - return true; > -} > - > -/* Reloc type: > - R_LARCH_SOP_POP_32_S_0_10_10_16_S2 > - R_LARCH_B26. */ > -static bool > -reloc_bits_b26 (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val) > -{ > - bfd_signed_vma val = *fix_val; > - bfd_signed_vma mask = ((bfd_signed_vma)0x1 << howto->bitsize) - 1; > - > - if (howto->complain_on_overflow != complain_overflow_signed) > - return false; > - > - /* Judge whether 4 bytes align. */ > - if (val & ((0x1UL << howto->rightshift) - 1)) > - return false; > - > - val = val >> howto->rightshift; > - > - if ((val & ~mask) && ((val & ~mask) != ~mask)) > + switch (howto->type) > { > - (*_bfd_error_handler) (_("%pB: relocation %s overflow 0x%lx"), > - abfd, howto->name, (long) val); > - bfd_set_error (bfd_error_bad_value); > - return false; > + case R_LARCH_SOP_POP_32_S_0_10_10_16_S2: > + case R_LARCH_B26: > + /* Perform insn bits field. 25:16>>16, 15:0<<10. */ > + val = ((val & 0xffff) << 10) | ((val >> 16) & 0x3ff); > + break; > + case R_LARCH_B21: > + val = ((val & 0xffff) << 10) | ((val >> 16) & 0x1f); > + break; > + default: > + val <<= howto->bitpos; > + break; > } > > - > - /* Perform insn bits field. */ > - val = val & mask; > - > - /* Perform insn bits field. 25:16>>16, 15:0<<10. */ > - val = ((val & 0xffff) << 10) | ((val >> 16) & 0x3ff); > - > *fix_val = val; > - > return true; > } > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] LoongArch: Fix immediate overflow check bug 2023-07-21 3:56 ` mengqinggang @ 2023-07-22 7:47 ` Xi Ruoyao 2023-07-22 7:52 ` Xi Ruoyao 1 sibling, 0 replies; 7+ messages in thread From: Xi Ruoyao @ 2023-07-22 7:47 UTC (permalink / raw) To: mengqinggang, binutils, Nick Clifton Cc: xuchenghua, chenglulu, liuzhensong, i.swmail, maskray, hejinyang On Fri, 2023-07-21 at 11:56 +0800, mengqinggang wrote: > Hi > > These two patches also need to be merged into the 2.41 branch. Thanks. +Nick too. For 1/2 it should be OK to backport. For 2/2 the subject and the commit message do not add up: the subject says "Fix immediate overflow check bug", but the message says "for easier understand". So does 2/2 really fix a bug? If true we should backport it, otherwise we should not. > 在 2023/7/17 下午4:22, mengqinggang 写道: > > For B16/B21/B26/PCREL20_S2 relocations, if immediate overflow check after > > rightshift, and the mask need to include sign bit. > > > > Now, the immediate overflow check before rightshift for easier understand. > > > > bfd/ChangeLog: > > > > * elfxx-loongarch.c (reloc_bits_pcrel20_s2): Delete. > > (reloc_bits_b16): Delete. > > (reloc_bits_b21): Delete. > > (reloc_bits_b26): Delete. > > (reloc_sign_bits): New. > > --- > > bfd/elfxx-loongarch.c | 160 +++++++++--------------------------------- > > 1 file changed, 32 insertions(+), 128 deletions(-) > > > > diff --git a/bfd/elfxx-loongarch.c b/bfd/elfxx-loongarch.c > > index da440d55c3c..2d299050c12 100644 > > --- a/bfd/elfxx-loongarch.c > > +++ b/bfd/elfxx-loongarch.c > > @@ -54,13 +54,7 @@ typedef struct loongarch_reloc_howto_type_struct > > static bool > > reloc_bits (bfd *abfd, reloc_howto_type *howto, bfd_vma *val); > > static bool > > -reloc_bits_pcrel20_s2 (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val); > > -static bool > > -reloc_bits_b16 (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val); > > -static bool > > -reloc_bits_b21 (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val); > > -static bool > > -reloc_bits_b26 (bfd *abfd, reloc_howto_type *howto, bfd_vma *val); > > +reloc_sign_bits (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val); > > > > static bfd_reloc_status_type > > loongarch_elf_add_sub_reloc (bfd *, arelent *, asymbol *, void *, > > @@ -457,7 +451,7 @@ static loongarch_reloc_howto_type loongarch_howto_table[] = > > 0x3fffc00, /* dst_mask */ > > false, /* pcrel_offset */ > > BFD_RELOC_LARCH_SOP_POP_32_S_10_16_S2, /* bfd_reloc_code_real_type */ > > - reloc_bits_b16, /* adjust_reloc_bits */ > > + reloc_sign_bits, /* adjust_reloc_bits */ > > NULL), /* larch_reloc_type_name */ > > > > LOONGARCH_HOWTO (R_LARCH_SOP_POP_32_S_5_20, /* type (43). */ > > @@ -493,7 +487,7 @@ static loongarch_reloc_howto_type loongarch_howto_table[] = > > false, /* pcrel_offset */ > > BFD_RELOC_LARCH_SOP_POP_32_S_0_5_10_16_S2, > > /* bfd_reloc_code_real_type */ > > - reloc_bits_b21, /* adjust_reloc_bits */ > > + reloc_sign_bits, /* adjust_reloc_bits */ > > NULL), /* larch_reloc_type_name */ > > > > LOONGARCH_HOWTO (R_LARCH_SOP_POP_32_S_0_10_10_16_S2, /* type (45). */ > > @@ -511,7 +505,7 @@ static loongarch_reloc_howto_type loongarch_howto_table[] = > > false, /* pcrel_offset */ > > BFD_RELOC_LARCH_SOP_POP_32_S_0_10_10_16_S2, > > /* bfd_reloc_code_real_type */ > > - reloc_bits_b26, /* adjust_reloc_bits */ > > + reloc_sign_bits, /* adjust_reloc_bits */ > > NULL), /* larch_reloc_type_name */ > > > > LOONGARCH_HOWTO (R_LARCH_SOP_POP_32_U, /* type (46). */ > > @@ -766,7 +760,7 @@ static loongarch_reloc_howto_type loongarch_howto_table[] = > > 0x3fffc00, /* dst_mask. */ > > false, /* pcrel_offset. */ > > BFD_RELOC_LARCH_B16, /* bfd_reloc_code_real_type. */ > > - reloc_bits_b16, /* adjust_reloc_bits. */ > > + reloc_sign_bits, /* adjust_reloc_bits. */ > > "b16"), /* larch_reloc_type_name. */ > > > > LOONGARCH_HOWTO (R_LARCH_B21, /* type (65). */ > > @@ -783,7 +777,7 @@ static loongarch_reloc_howto_type loongarch_howto_table[] = > > 0x3fffc1f, /* dst_mask. */ > > false, /* pcrel_offset. */ > > BFD_RELOC_LARCH_B21, /* bfd_reloc_code_real_type. */ > > - reloc_bits_b21, /* adjust_reloc_bits. */ > > + reloc_sign_bits, /* adjust_reloc_bits. */ > > "b21"), /* larch_reloc_type_name. */ > > > > LOONGARCH_HOWTO (R_LARCH_B26, /* type (66). */ > > @@ -800,7 +794,7 @@ static loongarch_reloc_howto_type loongarch_howto_table[] = > > 0x03ffffff, /* dst_mask. */ > > false, /* pcrel_offset. */ > > BFD_RELOC_LARCH_B26, /* bfd_reloc_code_real_type. */ > > - reloc_bits_b26, /* adjust_reloc_bits. */ > > + reloc_sign_bits, /* adjust_reloc_bits. */ > > "b26"), /* larch_reloc_type_name. */ > > > > LOONGARCH_HOWTO (R_LARCH_ABS_HI20, /* type (67). */ > > @@ -1436,7 +1430,7 @@ static loongarch_reloc_howto_type loongarch_howto_table[] = > > 0x1ffffe0, /* dst_mask. */ > > false, /* pcrel_offset. */ > > BFD_RELOC_LARCH_PCREL20_S2, /* bfd_reloc_code_real_type. */ > > - reloc_bits_pcrel20_s2, /* adjust_reloc_bits. */ > > + reloc_sign_bits, /* adjust_reloc_bits. */ > > NULL), /* larch_reloc_type_name. */ > > > > /* Canonical Frame Address. */ > > @@ -1677,12 +1671,14 @@ reloc_bits (bfd *abfd ATTRIBUTE_UNUSED, > > } > > > > static bool > > -reloc_bits_pcrel20_s2 (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val) > > +reloc_sign_bits (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val) > > { > > + if (howto->complain_on_overflow != complain_overflow_signed) > > + return false; > > + > > bfd_signed_vma val = (bfd_signed_vma)(*fix_val); > > - bfd_signed_vma mask = ((bfd_signed_vma)0x1 << howto->bitsize) - 1; > > > > - /* Check rightshift. */ > > + /* Check alignment. FIXME: if rightshift is not alingment. */ > > if (howto->rightshift > > && (val & ((((bfd_signed_vma) 1) << howto->rightshift) - 1))) > > { > > @@ -1692,8 +1688,12 @@ reloc_bits_pcrel20_s2 (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val) > > return false; > > } > > > > - val = val >> howto->rightshift; > > + bfd_signed_vma mask = ((bfd_signed_vma)0x1 << (howto->bitsize > > + + howto->rightshift - 1)) - 1; > > > > + /* Positive number: high part is all 0; > > + Negative number: if high part is not all 0, high part must be all 1. > > + high part: from sign bit to highest bit. */ > > if ((val & ~mask) && ((val & ~mask) != ~mask)) > > { > > (*_bfd_error_handler) (_("%pB: relocation %s overflow 0x%lx"), > > @@ -1702,123 +1702,27 @@ reloc_bits_pcrel20_s2 (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val) > > return false; > > } > > > > - /* Perform insn bits field. */ > > - val = val & mask; > > - val <<= howto->bitpos; > > - > > - *fix_val = (bfd_vma)val; > > - > > - return true; > > -} > > - > > - > > -/* Adjust val to perform insn > > - R_LARCH_SOP_POP_32_S_10_16_S2 > > - R_LARCH_B16. */ > > -static bool > > -reloc_bits_b16 (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val) > > -{ > > - bfd_signed_vma val = *fix_val; > > - bfd_signed_vma mask = ((bfd_signed_vma)0x1 << howto->bitsize) - 1; > > - > > - if (howto->complain_on_overflow != complain_overflow_signed) > > - return false; > > - > > - /* Judge whether 4 bytes align. */ > > - if (val & ((0x1UL << howto->rightshift) - 1)) > > - return false; > > - > > - val = val >> howto->rightshift; > > - > > - if ((val & ~mask) && ((val & ~mask) != ~mask)) > > - { > > - (*_bfd_error_handler) (_("%pB: relocation %s overflow 0x%lx"), > > - abfd, howto->name, (long) val); > > - bfd_set_error (bfd_error_bad_value); > > - return false; > > - } > > - > > - /* Perform insn bits field. */ > > - val = val & mask; > > - val <<= howto->bitpos; > > - > > - *fix_val = val; > > - > > - return true; > > -} > > - > > -/* Reloc type : > > - R_LARCH_SOP_POP_32_S_0_5_10_16_S2 > > - R_LARCH_B21. */ > > -static bool > > -reloc_bits_b21 (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val) > > -{ > > - bfd_signed_vma val = *fix_val; > > - bfd_signed_vma mask = ((bfd_signed_vma)0x1 << howto->bitsize) - 1; > > - > > - if (howto->complain_on_overflow != complain_overflow_signed) > > - return false; > > - > > - /* Judge whether 4 bytes align. */ > > - if (val & ((0x1UL << howto->rightshift) - 1)) > > - return false; > > - > > val = val >> howto->rightshift; > > - > > - if ((val & ~mask) && ((val & ~mask) != ~mask)) > > - { > > - (*_bfd_error_handler) (_("%pB: relocation %s overflow 0x%lx"), > > - abfd, howto->name, (long) val); > > - bfd_set_error (bfd_error_bad_value); > > - return false; > > - } > > - > > - /* Perform insn bits field. */ > > + /* can delete? */ > > + mask = ((bfd_signed_vma)0x1 << howto->bitsize) - 1; > > val = val & mask; > > > > - /* Perform insn bits field. 15:0<<10, 20:16>>16. */ > > - val = ((val & 0xffff) << 10) | ((val >> 16) & 0x1f); > > - > > - *fix_val = val; > > - > > - return true; > > -} > > - > > -/* Reloc type: > > - R_LARCH_SOP_POP_32_S_0_10_10_16_S2 > > - R_LARCH_B26. */ > > -static bool > > -reloc_bits_b26 (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val) > > -{ > > - bfd_signed_vma val = *fix_val; > > - bfd_signed_vma mask = ((bfd_signed_vma)0x1 << howto->bitsize) - 1; > > - > > - if (howto->complain_on_overflow != complain_overflow_signed) > > - return false; > > - > > - /* Judge whether 4 bytes align. */ > > - if (val & ((0x1UL << howto->rightshift) - 1)) > > - return false; > > - > > - val = val >> howto->rightshift; > > - > > - if ((val & ~mask) && ((val & ~mask) != ~mask)) > > + switch (howto->type) > > { > > - (*_bfd_error_handler) (_("%pB: relocation %s overflow 0x%lx"), > > - abfd, howto->name, (long) val); > > - bfd_set_error (bfd_error_bad_value); > > - return false; > > + case R_LARCH_SOP_POP_32_S_0_10_10_16_S2: > > + case R_LARCH_B26: > > + /* Perform insn bits field. 25:16>>16, 15:0<<10. */ > > + val = ((val & 0xffff) << 10) | ((val >> 16) & 0x3ff); > > + break; > > + case R_LARCH_B21: > > + val = ((val & 0xffff) << 10) | ((val >> 16) & 0x1f); > > + break; > > + default: > > + val <<= howto->bitpos; > > + break; > > } > > > > - > > - /* Perform insn bits field. */ > > - val = val & mask; > > - > > - /* Perform insn bits field. 25:16>>16, 15:0<<10. */ > > - val = ((val & 0xffff) << 10) | ((val >> 16) & 0x3ff); > > - > > *fix_val = val; > > - > > return true; > > } > > > -- Xi Ruoyao <xry111@xry111.site> School of Aerospace Science and Technology, Xidian University ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] LoongArch: Fix immediate overflow check bug 2023-07-21 3:56 ` mengqinggang 2023-07-22 7:47 ` Xi Ruoyao @ 2023-07-22 7:52 ` Xi Ruoyao 2023-07-22 8:23 ` Jinyang He 1 sibling, 1 reply; 7+ messages in thread From: Xi Ruoyao @ 2023-07-22 7:52 UTC (permalink / raw) To: mengqinggang, binutils, Nick Clifton Cc: xuchenghua, chenglulu, liuzhensong, i.swmail, maskray, hejinyang On Fri, 2023-07-21 at 11:56 +0800, mengqinggang wrote: > Hi > > These two patches also need to be merged into the 2.41 branch. Thanks. +Nick too. For 1/2 it should be OK to backport. For 2/2 the subject and the commit message do not add up: the subject says "Fix immediate overflow check bug", but the message says "for easier understand". So does 2/2 really fix a bug? If true we should backport it, otherwise we should not. > 在 2023/7/17 下午4:22, mengqinggang 写道: > > For B16/B21/B26/PCREL20_S2 relocations, if immediate overflow check after > > rightshift, and the mask need to include sign bit. > > > > Now, the immediate overflow check before rightshift for easier understand. > > > > bfd/ChangeLog: > > > > * elfxx-loongarch.c (reloc_bits_pcrel20_s2): Delete. > > (reloc_bits_b16): Delete. > > (reloc_bits_b21): Delete. > > (reloc_bits_b26): Delete. > > (reloc_sign_bits): New. > > --- > > bfd/elfxx-loongarch.c | 160 +++++++++--------------------------------- > > 1 file changed, 32 insertions(+), 128 deletions(-) > > > > diff --git a/bfd/elfxx-loongarch.c b/bfd/elfxx-loongarch.c > > index da440d55c3c..2d299050c12 100644 > > --- a/bfd/elfxx-loongarch.c > > +++ b/bfd/elfxx-loongarch.c > > @@ -54,13 +54,7 @@ typedef struct loongarch_reloc_howto_type_struct > > static bool > > reloc_bits (bfd *abfd, reloc_howto_type *howto, bfd_vma *val); > > static bool > > -reloc_bits_pcrel20_s2 (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val); > > -static bool > > -reloc_bits_b16 (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val); > > -static bool > > -reloc_bits_b21 (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val); > > -static bool > > -reloc_bits_b26 (bfd *abfd, reloc_howto_type *howto, bfd_vma *val); > > +reloc_sign_bits (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val); > > > > static bfd_reloc_status_type > > loongarch_elf_add_sub_reloc (bfd *, arelent *, asymbol *, void *, > > @@ -457,7 +451,7 @@ static loongarch_reloc_howto_type loongarch_howto_table[] = > > 0x3fffc00, /* dst_mask */ > > false, /* pcrel_offset */ > > BFD_RELOC_LARCH_SOP_POP_32_S_10_16_S2, /* bfd_reloc_code_real_type */ > > - reloc_bits_b16, /* adjust_reloc_bits */ > > + reloc_sign_bits, /* adjust_reloc_bits */ > > NULL), /* larch_reloc_type_name */ > > > > LOONGARCH_HOWTO (R_LARCH_SOP_POP_32_S_5_20, /* type (43). */ > > @@ -493,7 +487,7 @@ static loongarch_reloc_howto_type loongarch_howto_table[] = > > false, /* pcrel_offset */ > > BFD_RELOC_LARCH_SOP_POP_32_S_0_5_10_16_S2, > > /* bfd_reloc_code_real_type */ > > - reloc_bits_b21, /* adjust_reloc_bits */ > > + reloc_sign_bits, /* adjust_reloc_bits */ > > NULL), /* larch_reloc_type_name */ > > > > LOONGARCH_HOWTO (R_LARCH_SOP_POP_32_S_0_10_10_16_S2, /* type (45). */ > > @@ -511,7 +505,7 @@ static loongarch_reloc_howto_type loongarch_howto_table[] = > > false, /* pcrel_offset */ > > BFD_RELOC_LARCH_SOP_POP_32_S_0_10_10_16_S2, > > /* bfd_reloc_code_real_type */ > > - reloc_bits_b26, /* adjust_reloc_bits */ > > + reloc_sign_bits, /* adjust_reloc_bits */ > > NULL), /* larch_reloc_type_name */ > > > > LOONGARCH_HOWTO (R_LARCH_SOP_POP_32_U, /* type (46). */ > > @@ -766,7 +760,7 @@ static loongarch_reloc_howto_type loongarch_howto_table[] = > > 0x3fffc00, /* dst_mask. */ > > false, /* pcrel_offset. */ > > BFD_RELOC_LARCH_B16, /* bfd_reloc_code_real_type. */ > > - reloc_bits_b16, /* adjust_reloc_bits. */ > > + reloc_sign_bits, /* adjust_reloc_bits. */ > > "b16"), /* larch_reloc_type_name. */ > > > > LOONGARCH_HOWTO (R_LARCH_B21, /* type (65). */ > > @@ -783,7 +777,7 @@ static loongarch_reloc_howto_type loongarch_howto_table[] = > > 0x3fffc1f, /* dst_mask. */ > > false, /* pcrel_offset. */ > > BFD_RELOC_LARCH_B21, /* bfd_reloc_code_real_type. */ > > - reloc_bits_b21, /* adjust_reloc_bits. */ > > + reloc_sign_bits, /* adjust_reloc_bits. */ > > "b21"), /* larch_reloc_type_name. */ > > > > LOONGARCH_HOWTO (R_LARCH_B26, /* type (66). */ > > @@ -800,7 +794,7 @@ static loongarch_reloc_howto_type loongarch_howto_table[] = > > 0x03ffffff, /* dst_mask. */ > > false, /* pcrel_offset. */ > > BFD_RELOC_LARCH_B26, /* bfd_reloc_code_real_type. */ > > - reloc_bits_b26, /* adjust_reloc_bits. */ > > + reloc_sign_bits, /* adjust_reloc_bits. */ > > "b26"), /* larch_reloc_type_name. */ > > > > LOONGARCH_HOWTO (R_LARCH_ABS_HI20, /* type (67). */ > > @@ -1436,7 +1430,7 @@ static loongarch_reloc_howto_type loongarch_howto_table[] = > > 0x1ffffe0, /* dst_mask. */ > > false, /* pcrel_offset. */ > > BFD_RELOC_LARCH_PCREL20_S2, /* bfd_reloc_code_real_type. */ > > - reloc_bits_pcrel20_s2, /* adjust_reloc_bits. */ > > + reloc_sign_bits, /* adjust_reloc_bits. */ > > NULL), /* larch_reloc_type_name. */ > > > > /* Canonical Frame Address. */ > > @@ -1677,12 +1671,14 @@ reloc_bits (bfd *abfd ATTRIBUTE_UNUSED, > > } > > > > static bool > > -reloc_bits_pcrel20_s2 (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val) > > +reloc_sign_bits (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val) > > { > > + if (howto->complain_on_overflow != complain_overflow_signed) > > + return false; > > + > > bfd_signed_vma val = (bfd_signed_vma)(*fix_val); > > - bfd_signed_vma mask = ((bfd_signed_vma)0x1 << howto->bitsize) - 1; > > > > - /* Check rightshift. */ > > + /* Check alignment. FIXME: if rightshift is not alingment. */ > > if (howto->rightshift > > && (val & ((((bfd_signed_vma) 1) << howto->rightshift) - 1))) > > { > > @@ -1692,8 +1688,12 @@ reloc_bits_pcrel20_s2 (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val) > > return false; > > } > > > > - val = val >> howto->rightshift; > > + bfd_signed_vma mask = ((bfd_signed_vma)0x1 << (howto->bitsize > > + + howto->rightshift - 1)) - 1; > > > > + /* Positive number: high part is all 0; > > + Negative number: if high part is not all 0, high part must be all 1. > > + high part: from sign bit to highest bit. */ > > if ((val & ~mask) && ((val & ~mask) != ~mask)) > > { > > (*_bfd_error_handler) (_("%pB: relocation %s overflow 0x%lx"), > > @@ -1702,123 +1702,27 @@ reloc_bits_pcrel20_s2 (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val) > > return false; > > } > > > > - /* Perform insn bits field. */ > > - val = val & mask; > > - val <<= howto->bitpos; > > - > > - *fix_val = (bfd_vma)val; > > - > > - return true; > > -} > > - > > - > > -/* Adjust val to perform insn > > - R_LARCH_SOP_POP_32_S_10_16_S2 > > - R_LARCH_B16. */ > > -static bool > > -reloc_bits_b16 (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val) > > -{ > > - bfd_signed_vma val = *fix_val; > > - bfd_signed_vma mask = ((bfd_signed_vma)0x1 << howto->bitsize) - 1; > > - > > - if (howto->complain_on_overflow != complain_overflow_signed) > > - return false; > > - > > - /* Judge whether 4 bytes align. */ > > - if (val & ((0x1UL << howto->rightshift) - 1)) > > - return false; > > - > > - val = val >> howto->rightshift; > > - > > - if ((val & ~mask) && ((val & ~mask) != ~mask)) > > - { > > - (*_bfd_error_handler) (_("%pB: relocation %s overflow 0x%lx"), > > - abfd, howto->name, (long) val); > > - bfd_set_error (bfd_error_bad_value); > > - return false; > > - } > > - > > - /* Perform insn bits field. */ > > - val = val & mask; > > - val <<= howto->bitpos; > > - > > - *fix_val = val; > > - > > - return true; > > -} > > - > > -/* Reloc type : > > - R_LARCH_SOP_POP_32_S_0_5_10_16_S2 > > - R_LARCH_B21. */ > > -static bool > > -reloc_bits_b21 (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val) > > -{ > > - bfd_signed_vma val = *fix_val; > > - bfd_signed_vma mask = ((bfd_signed_vma)0x1 << howto->bitsize) - 1; > > - > > - if (howto->complain_on_overflow != complain_overflow_signed) > > - return false; > > - > > - /* Judge whether 4 bytes align. */ > > - if (val & ((0x1UL << howto->rightshift) - 1)) > > - return false; > > - > > val = val >> howto->rightshift; > > - > > - if ((val & ~mask) && ((val & ~mask) != ~mask)) > > - { > > - (*_bfd_error_handler) (_("%pB: relocation %s overflow 0x%lx"), > > - abfd, howto->name, (long) val); > > - bfd_set_error (bfd_error_bad_value); > > - return false; > > - } > > - > > - /* Perform insn bits field. */ > > + /* can delete? */ > > + mask = ((bfd_signed_vma)0x1 << howto->bitsize) - 1; > > val = val & mask; > > > > - /* Perform insn bits field. 15:0<<10, 20:16>>16. */ > > - val = ((val & 0xffff) << 10) | ((val >> 16) & 0x1f); > > - > > - *fix_val = val; > > - > > - return true; > > -} > > - > > -/* Reloc type: > > - R_LARCH_SOP_POP_32_S_0_10_10_16_S2 > > - R_LARCH_B26. */ > > -static bool > > -reloc_bits_b26 (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val) > > -{ > > - bfd_signed_vma val = *fix_val; > > - bfd_signed_vma mask = ((bfd_signed_vma)0x1 << howto->bitsize) - 1; > > - > > - if (howto->complain_on_overflow != complain_overflow_signed) > > - return false; > > - > > - /* Judge whether 4 bytes align. */ > > - if (val & ((0x1UL << howto->rightshift) - 1)) > > - return false; > > - > > - val = val >> howto->rightshift; > > - > > - if ((val & ~mask) && ((val & ~mask) != ~mask)) > > + switch (howto->type) > > { > > - (*_bfd_error_handler) (_("%pB: relocation %s overflow 0x%lx"), > > - abfd, howto->name, (long) val); > > - bfd_set_error (bfd_error_bad_value); > > - return false; > > + case R_LARCH_SOP_POP_32_S_0_10_10_16_S2: > > + case R_LARCH_B26: > > + /* Perform insn bits field. 25:16>>16, 15:0<<10. */ > > + val = ((val & 0xffff) << 10) | ((val >> 16) & 0x3ff); > > + break; > > + case R_LARCH_B21: > > + val = ((val & 0xffff) << 10) | ((val >> 16) & 0x1f); > > + break; > > + default: > > + val <<= howto->bitpos; > > + break; > > } > > > > - > > - /* Perform insn bits field. */ > > - val = val & mask; > > - > > - /* Perform insn bits field. 25:16>>16, 15:0<<10. */ > > - val = ((val & 0xffff) << 10) | ((val >> 16) & 0x3ff); > > - > > *fix_val = val; > > - > > return true; > > } > > > -- Xi Ruoyao <xry111@xry111.site> School of Aerospace Science and Technology, Xidian University ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] LoongArch: Fix immediate overflow check bug 2023-07-22 7:52 ` Xi Ruoyao @ 2023-07-22 8:23 ` Jinyang He 2023-07-24 9:14 ` Nick Clifton 0 siblings, 1 reply; 7+ messages in thread From: Jinyang He @ 2023-07-22 8:23 UTC (permalink / raw) To: Xi Ruoyao, mengqinggang, binutils, Nick Clifton Cc: xuchenghua, chenglulu, liuzhensong, i.swmail, maskray 在 2023/7/22 下午3:52, Xi Ruoyao 写道: > On Fri, 2023-07-21 at 11:56 +0800, mengqinggang wrote: >> Hi >> >> These two patches also need to be merged into the 2.41 branch. Thanks. > +Nick too. > > For 1/2 it should be OK to backport. For 2/2 the subject and the commit > message do not add up: the subject says "Fix immediate overflow check > bug", but the message says "for easier understand". So does 2/2 really > fix a bug? If true we should backport it, otherwise we should not. It is because we want fix bug related to ralaxation. I found it links succeed when relaxation may be wrong due to imm overflow [1]. I think the imm-overflow reporting is urgent and effective way to let we know something wrongs rather than nothing being reported. And for relaxation fixing, mengqinggang has a better way to fix it (, which difficult for me). [1] https://sourceware.org/pipermail/binutils/2023-July/128379.html > >> 在 2023/7/17 下午4:22, mengqinggang 写道: >>> For B16/B21/B26/PCREL20_S2 relocations, if immediate overflow check after >>> rightshift, and the mask need to include sign bit. >>> >>> Now, the immediate overflow check before rightshift for easier understand. >>> >>> bfd/ChangeLog: >>> >>> * elfxx-loongarch.c (reloc_bits_pcrel20_s2): Delete. >>> (reloc_bits_b16): Delete. >>> (reloc_bits_b21): Delete. >>> (reloc_bits_b26): Delete. >>> (reloc_sign_bits): New. >>> --- >>> bfd/elfxx-loongarch.c | 160 +++++++++--------------------------------- >>> 1 file changed, 32 insertions(+), 128 deletions(-) >>> >>> diff --git a/bfd/elfxx-loongarch.c b/bfd/elfxx-loongarch.c >>> index da440d55c3c..2d299050c12 100644 >>> --- a/bfd/elfxx-loongarch.c >>> +++ b/bfd/elfxx-loongarch.c >>> @@ -54,13 +54,7 @@ typedef struct loongarch_reloc_howto_type_struct >>> static bool >>> reloc_bits (bfd *abfd, reloc_howto_type *howto, bfd_vma *val); >>> static bool >>> -reloc_bits_pcrel20_s2 (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val); >>> -static bool >>> -reloc_bits_b16 (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val); >>> -static bool >>> -reloc_bits_b21 (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val); >>> -static bool >>> -reloc_bits_b26 (bfd *abfd, reloc_howto_type *howto, bfd_vma *val); >>> +reloc_sign_bits (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val); >>> >>> static bfd_reloc_status_type >>> loongarch_elf_add_sub_reloc (bfd *, arelent *, asymbol *, void *, >>> @@ -457,7 +451,7 @@ static loongarch_reloc_howto_type loongarch_howto_table[] = >>> 0x3fffc00, /* dst_mask */ >>> false, /* pcrel_offset */ >>> BFD_RELOC_LARCH_SOP_POP_32_S_10_16_S2, /* bfd_reloc_code_real_type */ >>> - reloc_bits_b16, /* adjust_reloc_bits */ >>> + reloc_sign_bits, /* adjust_reloc_bits */ >>> NULL), /* larch_reloc_type_name */ >>> >>> LOONGARCH_HOWTO (R_LARCH_SOP_POP_32_S_5_20, /* type (43). */ >>> @@ -493,7 +487,7 @@ static loongarch_reloc_howto_type loongarch_howto_table[] = >>> false, /* pcrel_offset */ >>> BFD_RELOC_LARCH_SOP_POP_32_S_0_5_10_16_S2, >>> /* bfd_reloc_code_real_type */ >>> - reloc_bits_b21, /* adjust_reloc_bits */ >>> + reloc_sign_bits, /* adjust_reloc_bits */ >>> NULL), /* larch_reloc_type_name */ >>> >>> LOONGARCH_HOWTO (R_LARCH_SOP_POP_32_S_0_10_10_16_S2, /* type (45). */ >>> @@ -511,7 +505,7 @@ static loongarch_reloc_howto_type loongarch_howto_table[] = >>> false, /* pcrel_offset */ >>> BFD_RELOC_LARCH_SOP_POP_32_S_0_10_10_16_S2, >>> /* bfd_reloc_code_real_type */ >>> - reloc_bits_b26, /* adjust_reloc_bits */ >>> + reloc_sign_bits, /* adjust_reloc_bits */ >>> NULL), /* larch_reloc_type_name */ >>> >>> LOONGARCH_HOWTO (R_LARCH_SOP_POP_32_U, /* type (46). */ >>> @@ -766,7 +760,7 @@ static loongarch_reloc_howto_type loongarch_howto_table[] = >>> 0x3fffc00, /* dst_mask. */ >>> false, /* pcrel_offset. */ >>> BFD_RELOC_LARCH_B16, /* bfd_reloc_code_real_type. */ >>> - reloc_bits_b16, /* adjust_reloc_bits. */ >>> + reloc_sign_bits, /* adjust_reloc_bits. */ >>> "b16"), /* larch_reloc_type_name. */ >>> >>> LOONGARCH_HOWTO (R_LARCH_B21, /* type (65). */ >>> @@ -783,7 +777,7 @@ static loongarch_reloc_howto_type loongarch_howto_table[] = >>> 0x3fffc1f, /* dst_mask. */ >>> false, /* pcrel_offset. */ >>> BFD_RELOC_LARCH_B21, /* bfd_reloc_code_real_type. */ >>> - reloc_bits_b21, /* adjust_reloc_bits. */ >>> + reloc_sign_bits, /* adjust_reloc_bits. */ >>> "b21"), /* larch_reloc_type_name. */ >>> >>> LOONGARCH_HOWTO (R_LARCH_B26, /* type (66). */ >>> @@ -800,7 +794,7 @@ static loongarch_reloc_howto_type loongarch_howto_table[] = >>> 0x03ffffff, /* dst_mask. */ >>> false, /* pcrel_offset. */ >>> BFD_RELOC_LARCH_B26, /* bfd_reloc_code_real_type. */ >>> - reloc_bits_b26, /* adjust_reloc_bits. */ >>> + reloc_sign_bits, /* adjust_reloc_bits. */ >>> "b26"), /* larch_reloc_type_name. */ >>> >>> LOONGARCH_HOWTO (R_LARCH_ABS_HI20, /* type (67). */ >>> @@ -1436,7 +1430,7 @@ static loongarch_reloc_howto_type loongarch_howto_table[] = >>> 0x1ffffe0, /* dst_mask. */ >>> false, /* pcrel_offset. */ >>> BFD_RELOC_LARCH_PCREL20_S2, /* bfd_reloc_code_real_type. */ >>> - reloc_bits_pcrel20_s2, /* adjust_reloc_bits. */ >>> + reloc_sign_bits, /* adjust_reloc_bits. */ >>> NULL), /* larch_reloc_type_name. */ >>> >>> /* Canonical Frame Address. */ >>> @@ -1677,12 +1671,14 @@ reloc_bits (bfd *abfd ATTRIBUTE_UNUSED, >>> } >>> >>> static bool >>> -reloc_bits_pcrel20_s2 (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val) >>> +reloc_sign_bits (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val) >>> { >>> + if (howto->complain_on_overflow != complain_overflow_signed) >>> + return false; >>> + >>> bfd_signed_vma val = (bfd_signed_vma)(*fix_val); >>> - bfd_signed_vma mask = ((bfd_signed_vma)0x1 << howto->bitsize) - 1; >>> >>> - /* Check rightshift. */ >>> + /* Check alignment. FIXME: if rightshift is not alingment. */ >>> if (howto->rightshift >>> && (val & ((((bfd_signed_vma) 1) << howto->rightshift) - 1))) >>> { >>> @@ -1692,8 +1688,12 @@ reloc_bits_pcrel20_s2 (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val) >>> return false; >>> } >>> >>> - val = val >> howto->rightshift; >>> + bfd_signed_vma mask = ((bfd_signed_vma)0x1 << (howto->bitsize >>> + + howto->rightshift - 1)) - 1; >>> >>> + /* Positive number: high part is all 0; >>> + Negative number: if high part is not all 0, high part must be all 1. >>> + high part: from sign bit to highest bit. */ >>> if ((val & ~mask) && ((val & ~mask) != ~mask)) >>> { >>> (*_bfd_error_handler) (_("%pB: relocation %s overflow 0x%lx"), >>> @@ -1702,123 +1702,27 @@ reloc_bits_pcrel20_s2 (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val) >>> return false; >>> } >>> >>> - /* Perform insn bits field. */ >>> - val = val & mask; >>> - val <<= howto->bitpos; >>> - >>> - *fix_val = (bfd_vma)val; >>> - >>> - return true; >>> -} >>> - >>> - >>> -/* Adjust val to perform insn >>> - R_LARCH_SOP_POP_32_S_10_16_S2 >>> - R_LARCH_B16. */ >>> -static bool >>> -reloc_bits_b16 (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val) >>> -{ >>> - bfd_signed_vma val = *fix_val; >>> - bfd_signed_vma mask = ((bfd_signed_vma)0x1 << howto->bitsize) - 1; >>> - >>> - if (howto->complain_on_overflow != complain_overflow_signed) >>> - return false; >>> - >>> - /* Judge whether 4 bytes align. */ >>> - if (val & ((0x1UL << howto->rightshift) - 1)) >>> - return false; >>> - >>> - val = val >> howto->rightshift; >>> - >>> - if ((val & ~mask) && ((val & ~mask) != ~mask)) >>> - { >>> - (*_bfd_error_handler) (_("%pB: relocation %s overflow 0x%lx"), >>> - abfd, howto->name, (long) val); >>> - bfd_set_error (bfd_error_bad_value); >>> - return false; >>> - } >>> - >>> - /* Perform insn bits field. */ >>> - val = val & mask; >>> - val <<= howto->bitpos; >>> - >>> - *fix_val = val; >>> - >>> - return true; >>> -} >>> - >>> -/* Reloc type : >>> - R_LARCH_SOP_POP_32_S_0_5_10_16_S2 >>> - R_LARCH_B21. */ >>> -static bool >>> -reloc_bits_b21 (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val) >>> -{ >>> - bfd_signed_vma val = *fix_val; >>> - bfd_signed_vma mask = ((bfd_signed_vma)0x1 << howto->bitsize) - 1; >>> - >>> - if (howto->complain_on_overflow != complain_overflow_signed) >>> - return false; >>> - >>> - /* Judge whether 4 bytes align. */ >>> - if (val & ((0x1UL << howto->rightshift) - 1)) >>> - return false; >>> - >>> val = val >> howto->rightshift; >>> - >>> - if ((val & ~mask) && ((val & ~mask) != ~mask)) >>> - { >>> - (*_bfd_error_handler) (_("%pB: relocation %s overflow 0x%lx"), >>> - abfd, howto->name, (long) val); >>> - bfd_set_error (bfd_error_bad_value); >>> - return false; >>> - } >>> - >>> - /* Perform insn bits field. */ >>> + /* can delete? */ >>> + mask = ((bfd_signed_vma)0x1 << howto->bitsize) - 1; >>> val = val & mask; >>> >>> - /* Perform insn bits field. 15:0<<10, 20:16>>16. */ >>> - val = ((val & 0xffff) << 10) | ((val >> 16) & 0x1f); >>> - >>> - *fix_val = val; >>> - >>> - return true; >>> -} >>> - >>> -/* Reloc type: >>> - R_LARCH_SOP_POP_32_S_0_10_10_16_S2 >>> - R_LARCH_B26. */ >>> -static bool >>> -reloc_bits_b26 (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val) >>> -{ >>> - bfd_signed_vma val = *fix_val; >>> - bfd_signed_vma mask = ((bfd_signed_vma)0x1 << howto->bitsize) - 1; >>> - >>> - if (howto->complain_on_overflow != complain_overflow_signed) >>> - return false; >>> - >>> - /* Judge whether 4 bytes align. */ >>> - if (val & ((0x1UL << howto->rightshift) - 1)) >>> - return false; >>> - >>> - val = val >> howto->rightshift; >>> - >>> - if ((val & ~mask) && ((val & ~mask) != ~mask)) >>> + switch (howto->type) >>> { >>> - (*_bfd_error_handler) (_("%pB: relocation %s overflow 0x%lx"), >>> - abfd, howto->name, (long) val); >>> - bfd_set_error (bfd_error_bad_value); >>> - return false; >>> + case R_LARCH_SOP_POP_32_S_0_10_10_16_S2: >>> + case R_LARCH_B26: >>> + /* Perform insn bits field. 25:16>>16, 15:0<<10. */ >>> + val = ((val & 0xffff) << 10) | ((val >> 16) & 0x3ff); >>> + break; >>> + case R_LARCH_B21: >>> + val = ((val & 0xffff) << 10) | ((val >> 16) & 0x1f); >>> + break; >>> + default: >>> + val <<= howto->bitpos; >>> + break; >>> } >>> >>> - >>> - /* Perform insn bits field. */ >>> - val = val & mask; >>> - >>> - /* Perform insn bits field. 25:16>>16, 15:0<<10. */ >>> - val = ((val & 0xffff) << 10) | ((val >> 16) & 0x3ff); >>> - >>> *fix_val = val; >>> - >>> return true; >>> } >>> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] LoongArch: Fix immediate overflow check bug 2023-07-22 8:23 ` Jinyang He @ 2023-07-24 9:14 ` Nick Clifton 0 siblings, 0 replies; 7+ messages in thread From: Nick Clifton @ 2023-07-24 9:14 UTC (permalink / raw) To: Jinyang He, Xi Ruoyao, mengqinggang Cc: xuchenghua, chenglulu, liuzhensong, i.swmail, maskray, Binutils Hi Jinyang He, Xi Ruoyao, Mengqingang, I have decided to apply both patches to the 2.41 branch. I agree that patch 1/2 was a needed bug fix, and whilst 2/2 is not a real bug-fix, it will help to alter users to the fact that there is a problem and that they need to do something to fix their code. (Also since I was applying patch 1/2 it was easy for me to apply 2/2 at at the same time). Luckily the 2.41 release has not happened yet. It was due to go out today but I have been asked to delay it whilst a problem with another port is resolved. But the delay will not be for long, and the release will happen soon. All of which is to say - please do not expect to get any more LoongArch bug fixes into the 2.41 release... Cheers Nick ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-07-24 9:14 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-07-17 8:22 [PATCH 1/2] LoongArch: Fix instruction immediate bug caused by sign-extend mengqinggang 2023-07-17 8:22 ` [PATCH 2/2] LoongArch: Fix immediate overflow check bug mengqinggang 2023-07-21 3:56 ` mengqinggang 2023-07-22 7:47 ` Xi Ruoyao 2023-07-22 7:52 ` Xi Ruoyao 2023-07-22 8:23 ` Jinyang He 2023-07-24 9:14 ` Nick Clifton
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).