From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.loongson.cn (mail.loongson.cn [114.242.206.163]) by sourceware.org (Postfix) with ESMTP id 14ABE3858C53 for ; Fri, 21 Jul 2023 03:56:03 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 14ABE3858C53 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=loongson.cn Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=loongson.cn Received: from loongson.cn (unknown [10.20.4.171]) by gateway (Coremail) with SMTP id _____8AxEvDRAbpkb+UHAA--.19034S3; Fri, 21 Jul 2023 11:56:01 +0800 (CST) Received: from [10.20.4.171] (unknown [10.20.4.171]) by localhost.localdomain (Coremail) with SMTP id AQAAf8Dx5szQAbpkUPE1AA--.62475S3; Fri, 21 Jul 2023 11:56:01 +0800 (CST) Subject: Re: [PATCH 2/2] LoongArch: Fix immediate overflow check bug To: binutils@sourceware.org Cc: xuchenghua@loongson.cn, chenglulu@loongson.cn, liuzhensong@loongson.cn, xry111@xry111.site, i.swmail@xen0n.name, maskray@google.com, hejinyang@loongson.cn References: <20230717082229.2149099-1-mengqinggang@loongson.cn> <20230717082229.2149099-2-mengqinggang@loongson.cn> From: mengqinggang Message-ID: <9f523a9f-8414-49d2-b3db-303171bcc389@loongson.cn> Date: Fri, 21 Jul 2023 11:56:00 +0800 User-Agent: Mozilla/5.0 (X11; Linux mips64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 MIME-Version: 1.0 In-Reply-To: <20230717082229.2149099-2-mengqinggang@loongson.cn> Content-Type: text/plain; charset=gbk; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US X-CM-TRANSID:AQAAf8Dx5szQAbpkUPE1AA--.62475S3 X-CM-SenderInfo: 5phqw15lqjwttqj6z05rqj20fqof0/ X-Coremail-Antispam: 1Uk129KBj93XoWxuFWUZr17KFW5WrWrCrWDWrX_yoWfCr17pr ZIqw1kGrs3Ar4xWFyYvFn5uF1rWws7KFyxXrZFv34I9a4fXr9xXrWUG3yrGF4DGryjqr47 XFZ0vryUZw4UJagCm3ZEXasCq-sJn29KB7ZKAUJUUUU5529EdanIXcx71UUUUU7KY7ZEXa sCq-sGcSsGvfJ3Ic02F40EFcxC0VAKzVAqx4xG6I80ebIjqfuFe4nvWSU5nxnvy29KBjDU 0xBIdaVrnRJUUUvFb4IE77IF4wAFF20E14v26r1j6r4UM7CY07I20VC2zVCF04k26cxKx2 IYs7xG6rWj6s0DM7CIcVAFz4kK6r1j6r18M28lY4IEw2IIxxk0rwA2F7IY1VAKz4vEj48v e4kI8wA2z4x0Y4vE2Ix0cI8IcVAFwI0_JFI_Gr1l84ACjcxK6xIIjxv20xvEc7CjxVAFwI 0_Jr0_Gr1l84ACjcxK6I8E87Iv67AKxVWxJVW8Jr1l84ACjcxK6I8E87Iv6xkF7I0E14v2 6r4UJVWxJr1le2I262IYc4CY6c8Ij28IcVAaY2xG8wAqjxCEc2xF0cIa020Ex4CE44I27w Aqx4xG64xvF2IEw4CE5I8CrVC2j2WlYx0E2Ix0cI8IcVAFwI0_JF0_Jw1lYx0Ex4A2jsIE 14v26r1j6r4UMcvjeVCFs4IE7xkEbVWUJVW8JwACjcxG0xvEwIxGrwCYjI0SjxkI62AI1c AE67vIY487MxAIw28IcxkI7VAKI48JMxC20s026xCaFVCjc4AY6r1j6r4UMI8I3I0E5I8C rVAFwI0_Jr0_Jr4lx2IqxVCjr7xvwVAFwI0_JrI_JrWlx4CE17CEb7AF67AKxVWUtVW8Zw CIc40Y0x0EwIxGrwCI42IY6xIIjxv20xvE14v26r1j6r1xMIIF0xvE2Ix0cI8IcVCY1x02 67AKxVWUJVW8JwCI42IY6xAIw20EY4v20xvaj40_Jr0_JF4lIxAIcVC2z280aVAFwI0_Jr 0_Gr1lIxAIcVC2z280aVCY1x0267AKxVWUJVW8JbIYCTnIWIevJa73UjIFyTuYvjxUwMKu UUUUU X-Spam-Status: No, score=-9.8 required=5.0 tests=BAYES_00,GIT_PATCH_0,KAM_DMARC_STATUS,MIME_CHARSET_FARAWAY,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: 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; > } >