public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] as: fixed internal error when immediate value of relocation overflow.
@ 2023-10-09  2:11 Lulu Cai
  2023-10-10  0:25 ` WANG Xuerui
  2023-10-11  2:20 ` Lulu Cai
  0 siblings, 2 replies; 4+ messages in thread
From: Lulu Cai @ 2023-10-09  2:11 UTC (permalink / raw)
  To: binutils
  Cc: xuchenghua, chenglulu, liuzhensong, mengqinggang, xry111,
	i.swmail, maskray, Lulu Cai

The as reports an internal error when the relocation
overflow or is misaligned. Delete _bfd_error_handler
can avoid it and reports Reloc overflow.
---
 bfd/elfxx-loongarch.c                      | 8 +-------
 gas/testsuite/gas/loongarch/imm_overflow.d | 3 +++
 gas/testsuite/gas/loongarch/imm_overflow.l | 2 ++
 gas/testsuite/gas/loongarch/imm_overflow.s | 4 ++++
 gas/testsuite/gas/loongarch/imm_unalign.d  | 3 +++
 gas/testsuite/gas/loongarch/imm_unalign.l  | 2 ++
 gas/testsuite/gas/loongarch/imm_unalign.s  | 6 ++++++
 7 files changed, 21 insertions(+), 7 deletions(-)
 create mode 100644 gas/testsuite/gas/loongarch/imm_overflow.d
 create mode 100644 gas/testsuite/gas/loongarch/imm_overflow.l
 create mode 100644 gas/testsuite/gas/loongarch/imm_overflow.s
 create mode 100644 gas/testsuite/gas/loongarch/imm_unalign.d
 create mode 100644 gas/testsuite/gas/loongarch/imm_unalign.l
 create mode 100644 gas/testsuite/gas/loongarch/imm_unalign.s

diff --git a/bfd/elfxx-loongarch.c b/bfd/elfxx-loongarch.c
index 0a595eb87d5..c20efe82f31 100644
--- a/bfd/elfxx-loongarch.c
+++ b/bfd/elfxx-loongarch.c
@@ -1671,7 +1671,7 @@ reloc_bits (bfd *abfd ATTRIBUTE_UNUSED,
 }
 
 static bool
-reloc_sign_bits (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val)
+reloc_sign_bits (bfd *abfd ATTRIBUTE_UNUSED, reloc_howto_type *howto, bfd_vma *fix_val)
 {
   if (howto->complain_on_overflow != complain_overflow_signed)
     return false;
@@ -1682,9 +1682,6 @@ reloc_sign_bits (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val)
   if (howto->rightshift
       && (val & ((((bfd_signed_vma) 1) << howto->rightshift) - 1)))
     {
-      (*_bfd_error_handler) (_("%pB: relocation %s right shift %d error 0x%lx"),
-			     abfd, howto->name, howto->rightshift, (long) val);
-      bfd_set_error (bfd_error_bad_value);
       return false;
     }
 
@@ -1696,9 +1693,6 @@ reloc_sign_bits (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val)
      high part: from sign bit to highest bit.  */
   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;
     }
 
diff --git a/gas/testsuite/gas/loongarch/imm_overflow.d b/gas/testsuite/gas/loongarch/imm_overflow.d
new file mode 100644
index 00000000000..1ead2c9a74f
--- /dev/null
+++ b/gas/testsuite/gas/loongarch/imm_overflow.d
@@ -0,0 +1,3 @@
+#as:
+#source: imm_overflow.s
+#warning_output: imm_overflow.l
diff --git a/gas/testsuite/gas/loongarch/imm_overflow.l b/gas/testsuite/gas/loongarch/imm_overflow.l
new file mode 100644
index 00000000000..3dd4d90ffa1
--- /dev/null
+++ b/gas/testsuite/gas/loongarch/imm_overflow.l
@@ -0,0 +1,2 @@
+.*Assembler messages:
+.*Warning: Reloc overflow
diff --git a/gas/testsuite/gas/loongarch/imm_overflow.s b/gas/testsuite/gas/loongarch/imm_overflow.s
new file mode 100644
index 00000000000..2be688bd25a
--- /dev/null
+++ b/gas/testsuite/gas/loongarch/imm_overflow.s
@@ -0,0 +1,4 @@
+.L1:
+  nop
+  .fill 0x12345, 4, 0
+  blt $r3, $r4, .L1
diff --git a/gas/testsuite/gas/loongarch/imm_unalign.d b/gas/testsuite/gas/loongarch/imm_unalign.d
new file mode 100644
index 00000000000..df1a8cf6041
--- /dev/null
+++ b/gas/testsuite/gas/loongarch/imm_unalign.d
@@ -0,0 +1,3 @@
+#as:
+#source: imm_unalign.s
+#warning_output: imm_unalign.l
diff --git a/gas/testsuite/gas/loongarch/imm_unalign.l b/gas/testsuite/gas/loongarch/imm_unalign.l
new file mode 100644
index 00000000000..3dd4d90ffa1
--- /dev/null
+++ b/gas/testsuite/gas/loongarch/imm_unalign.l
@@ -0,0 +1,2 @@
+.*Assembler messages:
+.*Warning: Reloc overflow
diff --git a/gas/testsuite/gas/loongarch/imm_unalign.s b/gas/testsuite/gas/loongarch/imm_unalign.s
new file mode 100644
index 00000000000..d97ada04d9f
--- /dev/null
+++ b/gas/testsuite/gas/loongarch/imm_unalign.s
@@ -0,0 +1,6 @@
+.L1:
+  .2byte 0x12
+
+.L2:
+  .fill 1, 4, 0
+  blt $r3, $r4, .L1
-- 
2.31.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] as: fixed internal error when immediate value of relocation overflow.
  2023-10-09  2:11 [PATCH] as: fixed internal error when immediate value of relocation overflow Lulu Cai
@ 2023-10-10  0:25 ` WANG Xuerui
  2023-10-10  9:55   ` Lulu Cai
  2023-10-11  2:20 ` Lulu Cai
  1 sibling, 1 reply; 4+ messages in thread
From: WANG Xuerui @ 2023-10-10  0:25 UTC (permalink / raw)
  To: Lulu Cai, binutils
  Cc: xuchenghua, chenglulu, liuzhensong, mengqinggang, xry111,
	i.swmail, maskray

Hi,

On 10/9/23 10:11, Lulu Cai wrote:
> The as reports an internal error when the relocation
> overflow or is misaligned. Delete _bfd_error_handler
> can avoid it and reports Reloc overflow.
> ---
>   bfd/elfxx-loongarch.c                      | 8 +-------
>   gas/testsuite/gas/loongarch/imm_overflow.d | 3 +++
>   gas/testsuite/gas/loongarch/imm_overflow.l | 2 ++
>   gas/testsuite/gas/loongarch/imm_overflow.s | 4 ++++
>   gas/testsuite/gas/loongarch/imm_unalign.d  | 3 +++
>   gas/testsuite/gas/loongarch/imm_unalign.l  | 2 ++
>   gas/testsuite/gas/loongarch/imm_unalign.s  | 6 ++++++
>   7 files changed, 21 insertions(+), 7 deletions(-)
>   create mode 100644 gas/testsuite/gas/loongarch/imm_overflow.d
>   create mode 100644 gas/testsuite/gas/loongarch/imm_overflow.l
>   create mode 100644 gas/testsuite/gas/loongarch/imm_overflow.s
>   create mode 100644 gas/testsuite/gas/loongarch/imm_unalign.d
>   create mode 100644 gas/testsuite/gas/loongarch/imm_unalign.l
>   create mode 100644 gas/testsuite/gas/loongarch/imm_unalign.s
>
> diff --git a/bfd/elfxx-loongarch.c b/bfd/elfxx-loongarch.c
> index 0a595eb87d5..c20efe82f31 100644
> --- a/bfd/elfxx-loongarch.c
> +++ b/bfd/elfxx-loongarch.c
> @@ -1671,7 +1671,7 @@ reloc_bits (bfd *abfd ATTRIBUTE_UNUSED,
>   }
>   
>   static bool
> -reloc_sign_bits (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val)
> +reloc_sign_bits (bfd *abfd ATTRIBUTE_UNUSED, reloc_howto_type *howto, bfd_vma *fix_val)
>   {
>     if (howto->complain_on_overflow != complain_overflow_signed)
>       return false;
> @@ -1682,9 +1682,6 @@ reloc_sign_bits (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val)
>     if (howto->rightshift
>         && (val & ((((bfd_signed_vma) 1) << howto->rightshift) - 1)))
>       {
> -      (*_bfd_error_handler) (_("%pB: relocation %s right shift %d error 0x%lx"),
> -			     abfd, howto->name, howto->rightshift, (long) val);
> -      bfd_set_error (bfd_error_bad_value);
>         return false;
>       }
>   
> @@ -1696,9 +1693,6 @@ reloc_sign_bits (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val)
>        high part: from sign bit to highest bit.  */
>     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;
>       }
>   

Isn't this only removing the error (downgrading it to a warning it 
seems) but not touching anything else? This will make the programmer 
errors slip through (apparently because no one reads warnings heh, it's 
especially worse for many devs not comfortable with English that I know 
of), and the problematic case you had in mind is still left unfixed.

I think it's better to, at least, add the previously rejected code 
you're meaning to make acceptable, so the intent is clear; then we can 
figure out a better way forward than simply deleting checks.


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] as: fixed internal error when immediate value of relocation overflow.
  2023-10-10  0:25 ` WANG Xuerui
@ 2023-10-10  9:55   ` Lulu Cai
  0 siblings, 0 replies; 4+ messages in thread
From: Lulu Cai @ 2023-10-10  9:55 UTC (permalink / raw)
  To: WANG Xuerui, binutils
  Cc: xuchenghua, chenglulu, liuzhensong, mengqinggang, xry111, maskray

在 2023/10/10 上午8:25, WANG Xuerui 写道:
> Hi,
>
> On 10/9/23 10:11, Lulu Cai wrote:
>> The as reports an internal error when the relocation
>> overflow or is misaligned. Delete _bfd_error_handler
>> can avoid it and reports Reloc overflow.
>> ---
>>   bfd/elfxx-loongarch.c                      | 8 +-------
>>   gas/testsuite/gas/loongarch/imm_overflow.d | 3 +++
>>   gas/testsuite/gas/loongarch/imm_overflow.l | 2 ++
>>   gas/testsuite/gas/loongarch/imm_overflow.s | 4 ++++
>>   gas/testsuite/gas/loongarch/imm_unalign.d  | 3 +++
>>   gas/testsuite/gas/loongarch/imm_unalign.l  | 2 ++
>>   gas/testsuite/gas/loongarch/imm_unalign.s  | 6 ++++++
>>   7 files changed, 21 insertions(+), 7 deletions(-)
>>   create mode 100644 gas/testsuite/gas/loongarch/imm_overflow.d
>>   create mode 100644 gas/testsuite/gas/loongarch/imm_overflow.l
>>   create mode 100644 gas/testsuite/gas/loongarch/imm_overflow.s
>>   create mode 100644 gas/testsuite/gas/loongarch/imm_unalign.d
>>   create mode 100644 gas/testsuite/gas/loongarch/imm_unalign.l
>>   create mode 100644 gas/testsuite/gas/loongarch/imm_unalign.s
>>
>> diff --git a/bfd/elfxx-loongarch.c b/bfd/elfxx-loongarch.c
>> index 0a595eb87d5..c20efe82f31 100644
>> --- a/bfd/elfxx-loongarch.c
>> +++ b/bfd/elfxx-loongarch.c
>> @@ -1671,7 +1671,7 @@ reloc_bits (bfd *abfd ATTRIBUTE_UNUSED,
>>   }
>>     static bool
>> -reloc_sign_bits (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val)
>> +reloc_sign_bits (bfd *abfd ATTRIBUTE_UNUSED, reloc_howto_type 
>> *howto, bfd_vma *fix_val)
>>   {
>>     if (howto->complain_on_overflow != complain_overflow_signed)
>>       return false;
>> @@ -1682,9 +1682,6 @@ reloc_sign_bits (bfd *abfd, reloc_howto_type 
>> *howto, bfd_vma *fix_val)
>>     if (howto->rightshift
>>         && (val & ((((bfd_signed_vma) 1) << howto->rightshift) - 1)))
>>       {
>> -      (*_bfd_error_handler) (_("%pB: relocation %s right shift %d 
>> error 0x%lx"),
>> -                 abfd, howto->name, howto->rightshift, (long) val);
>> -      bfd_set_error (bfd_error_bad_value);
>>         return false;
>>       }
>>   @@ -1696,9 +1693,6 @@ reloc_sign_bits (bfd *abfd, reloc_howto_type 
>> *howto, bfd_vma *fix_val)
>>        high part: from sign bit to highest bit.  */
>>     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;
>>       }
>
> Isn't this only removing the error (downgrading it to a warning it 
> seems) but not touching anything else? This will make the programmer 
> errors slip through (apparently because no one reads warnings heh, 
> it's especially worse for many devs not comfortable with English that 
> I know of), and the problematic case you had in mind is still left 
> unfixed.
>
> I think it's better to, at least, add the previously rejected code 
> you're meaning to make acceptable, so the intent is clear; then we can 
> figure out a better way forward than simply deleting checks.

Both the as and ld call the same function and use _bfd_error_handler to 
output error messages when relocating overflow. However, as passed NULL 
to the function when it was called, resulting in an internal error. ld 
passes a non-null value when called and can output error messages 
without internal errors.
So I consider not simply deleting this function. When as passes null, 
the function is not called and reports a "Reloc overflow" error instead 
of a warning. ld can call this function to output detailed information.


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH] as: fixed internal error when immediate value of relocation overflow.
  2023-10-09  2:11 [PATCH] as: fixed internal error when immediate value of relocation overflow Lulu Cai
  2023-10-10  0:25 ` WANG Xuerui
@ 2023-10-11  2:20 ` Lulu Cai
  1 sibling, 0 replies; 4+ messages in thread
From: Lulu Cai @ 2023-10-11  2:20 UTC (permalink / raw)
  To: binutils
  Cc: xuchenghua, chenglulu, liuzhensong, mengqinggang, xry111,
	i.swmail, maskray, Lulu Cai

The as and ld use _bfd_error_handler to output error messages when
checking relocation alignment and relocation overflow. However, the
abfd value passed by as to the function is NULL, resulting in an
internal error. The ld passes a non-null value to the function,
so it can output an error message normally.
---
 bfd/elfxx-loongarch.c                      | 22 ++++++++++++++++------
 gas/config/tc-loongarch.c                  |  2 +-
 gas/testsuite/gas/loongarch/imm_overflow.d |  3 +++
 gas/testsuite/gas/loongarch/imm_overflow.l |  2 ++
 gas/testsuite/gas/loongarch/imm_overflow.s |  4 ++++
 gas/testsuite/gas/loongarch/imm_unalign.d  |  3 +++
 gas/testsuite/gas/loongarch/imm_unalign.l  |  2 ++
 gas/testsuite/gas/loongarch/imm_unalign.s  |  6 ++++++
 8 files changed, 37 insertions(+), 7 deletions(-)
 create mode 100644 gas/testsuite/gas/loongarch/imm_overflow.d
 create mode 100644 gas/testsuite/gas/loongarch/imm_overflow.l
 create mode 100644 gas/testsuite/gas/loongarch/imm_overflow.s
 create mode 100644 gas/testsuite/gas/loongarch/imm_unalign.d
 create mode 100644 gas/testsuite/gas/loongarch/imm_unalign.l
 create mode 100644 gas/testsuite/gas/loongarch/imm_unalign.s

diff --git a/bfd/elfxx-loongarch.c b/bfd/elfxx-loongarch.c
index 0a595eb87d5..a970a257aa9 100644
--- a/bfd/elfxx-loongarch.c
+++ b/bfd/elfxx-loongarch.c
@@ -1682,9 +1682,14 @@ reloc_sign_bits (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val)
   if (howto->rightshift
       && (val & ((((bfd_signed_vma) 1) << howto->rightshift) - 1)))
     {
-      (*_bfd_error_handler) (_("%pB: relocation %s right shift %d error 0x%lx"),
-			     abfd, howto->name, howto->rightshift, (long) val);
-      bfd_set_error (bfd_error_bad_value);
+      /* The as passes NULL casued internal error, so it can not use _bfd_error_handler
+	 output details, ld is not affected.  */
+      if (abfd != NULL)
+	{
+	  (*_bfd_error_handler) (_("%pB: relocation %s right shift %d error 0x%lx"),
+				 abfd, howto->name, howto->rightshift, (long) val);
+	  bfd_set_error (bfd_error_bad_value);
+	}
       return false;
     }
 
@@ -1696,9 +1701,14 @@ reloc_sign_bits (bfd *abfd, reloc_howto_type *howto, bfd_vma *fix_val)
      high part: from sign bit to highest bit.  */
   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);
+      /* The as passes NULL casued internal error, so it can not use _bfd_error_handler
+	 output details, ld is not affected.  */
+      if (abfd != NULL)
+	{
+	  (*_bfd_error_handler) (_("%pB: relocation %s overflow 0x%lx"),
+				 abfd, howto->name, (long) val);
+	  bfd_set_error (bfd_error_bad_value);
+	}
       return false;
     }
 
diff --git a/gas/config/tc-loongarch.c b/gas/config/tc-loongarch.c
index b563982b933..33f3e71ce2f 100644
--- a/gas/config/tc-loongarch.c
+++ b/gas/config/tc-loongarch.c
@@ -1234,7 +1234,7 @@ static void fix_reloc_insn (fixS *fixP, bfd_vma reloc_val, char *buf)
   insn = bfd_getl32 (buf);
 
   if (!loongarch_adjust_reloc_bitsfield (NULL, howto, &reloc_val))
-    as_warn_where (fixP->fx_file, fixP->fx_line, "Reloc overflow");
+    as_bad_where (fixP->fx_file, fixP->fx_line, "Reloc overflow");
 
   insn = (insn & (insn_t)howto->src_mask)
     | ((insn & (~(insn_t)howto->dst_mask)) | reloc_val);
diff --git a/gas/testsuite/gas/loongarch/imm_overflow.d b/gas/testsuite/gas/loongarch/imm_overflow.d
new file mode 100644
index 00000000000..50a65b7c4ae
--- /dev/null
+++ b/gas/testsuite/gas/loongarch/imm_overflow.d
@@ -0,0 +1,3 @@
+#as:
+#source: imm_overflow.s
+#error_output: imm_overflow.l
diff --git a/gas/testsuite/gas/loongarch/imm_overflow.l b/gas/testsuite/gas/loongarch/imm_overflow.l
new file mode 100644
index 00000000000..449b3c2adea
--- /dev/null
+++ b/gas/testsuite/gas/loongarch/imm_overflow.l
@@ -0,0 +1,2 @@
+.*Assembler messages:
+.*Error: Reloc overflow
diff --git a/gas/testsuite/gas/loongarch/imm_overflow.s b/gas/testsuite/gas/loongarch/imm_overflow.s
new file mode 100644
index 00000000000..9aac396ad50
--- /dev/null
+++ b/gas/testsuite/gas/loongarch/imm_overflow.s
@@ -0,0 +1,4 @@
+.L1:
+  nop
+  .fill 0x3ffffff, 4, 0
+  b .L1
diff --git a/gas/testsuite/gas/loongarch/imm_unalign.d b/gas/testsuite/gas/loongarch/imm_unalign.d
new file mode 100644
index 00000000000..1deb502589e
--- /dev/null
+++ b/gas/testsuite/gas/loongarch/imm_unalign.d
@@ -0,0 +1,3 @@
+#as:
+#source: imm_unalign.s
+#error_output: imm_unalign.l
diff --git a/gas/testsuite/gas/loongarch/imm_unalign.l b/gas/testsuite/gas/loongarch/imm_unalign.l
new file mode 100644
index 00000000000..449b3c2adea
--- /dev/null
+++ b/gas/testsuite/gas/loongarch/imm_unalign.l
@@ -0,0 +1,2 @@
+.*Assembler messages:
+.*Error: Reloc overflow
diff --git a/gas/testsuite/gas/loongarch/imm_unalign.s b/gas/testsuite/gas/loongarch/imm_unalign.s
new file mode 100644
index 00000000000..a853bdcbf0a
--- /dev/null
+++ b/gas/testsuite/gas/loongarch/imm_unalign.s
@@ -0,0 +1,6 @@
+.L1:
+  .2byte 0x12
+
+.L2:
+  .fill 1, 4, 0
+  b .L1
-- 
2.36.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2023-10-11  2:20 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-09  2:11 [PATCH] as: fixed internal error when immediate value of relocation overflow Lulu Cai
2023-10-10  0:25 ` WANG Xuerui
2023-10-10  9:55   ` Lulu Cai
2023-10-11  2:20 ` Lulu Cai

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).