diff mbox series

[v3,12/13] RISC-V/bfd: warn about non-boolean unaligned-access attribute

Message ID 3ecb6e90-f46f-46de-ba79-f4188e25e58f@suse.com
State New
Headers
Series RISC-V: assorted fixes and (hopefully) improvements |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_binutils_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_binutils_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_binutils_check--master-arm success Test passed

Commit Message

Jan Beulich June 19, 2026, 11:52 a.m. UTC 
  The attribute being a boolean one, incoming values should be solely 0 or
1. Convert other non-zero values to 1.
---
v3: New.

Comments

Jiawei June 22, 2026, 9:02 a.m. UTC | #1 
On 2026/6/19 19:52, Jan Beulich wrote:
> The attribute being a boolean one, incoming values should be solely 0 or
> 1. Convert other non-zero values to 1.
> ---
> v3: New.
>
> --- a/bfd/elfxx-riscv.c
> +++ b/bfd/elfxx-riscv.c
> @@ -3777,6 +3777,23 @@ riscv_merge_attributes (bfd *ibfd, struc
>   	 initialized.  */
>         out_attr[0].i = 1;
>   
> +      for (i = LEAST_KNOWN_OBJ_ATTRIBUTE; i < NUM_KNOWN_OBJ_ATTRIBUTES; i++)
> +	{
> +	  switch (i)
> +	    {
> +	    case Tag_RISCV_unaligned_access:
> +	      if (out_attr[i].i <= 1)
> +		break;
> +
> +	      _bfd_error_handler
> +		(_("warning: %pB uses non-boolean `unaligned access' attribute; "
> +		   "converting to boolean"),
> +		 ibfd);
> +	      out_attr[i].i = 1;
> +	      break;
> +	    }
> +	}
> +
>         return true;
>       }
>   
> @@ -3865,7 +3882,12 @@ riscv_merge_attributes (bfd *ibfd, struc
>   	break;
>   
>         case Tag_RISCV_unaligned_access:
> -	out_attr[i].i |= in_attr[i].i;
> +	if (in_attr[i].i > 1)
> +	  _bfd_error_handler
> +	    (_("warning: %pB uses non-boolean `unaligned access' attribute; "
> +	       "converting to boolean"),
> +	     ibfd);
> +	out_attr[i].i |= !!in_attr[i].i;
>   	break;
>   
>         case Tag_RISCV_stack_align:

LGTM.

Checking the copied attributes for the first input
object is necessary, since otherwise a non-boolean unaligned_access 
value from
the first object would be carried into the output unchanged.

One very small wording thought: the gas-side diagnostic uses
`unaligned_access', while this one uses `unaligned access'. Either is fine
with me, using the same spelling might make the diagnostics more
consistent, but this does not need to block the patch.

Reviewed-by: Jiawei jiawei@iscas.ac.cn
Jan Beulich June 22, 2026, 9:15 a.m. UTC | #2 
On 22.06.2026 11:02, Jiawei wrote:
> On 2026/6/19 19:52, Jan Beulich wrote:
>> The attribute being a boolean one, incoming values should be solely 0 or
>> 1. Convert other non-zero values to 1.
>> ---
>> v3: New.
>>
>> --- a/bfd/elfxx-riscv.c
>> +++ b/bfd/elfxx-riscv.c
>> @@ -3777,6 +3777,23 @@ riscv_merge_attributes (bfd *ibfd, struc
>>   	 initialized.  */
>>         out_attr[0].i = 1;
>>   
>> +      for (i = LEAST_KNOWN_OBJ_ATTRIBUTE; i < NUM_KNOWN_OBJ_ATTRIBUTES; i++)
>> +	{
>> +	  switch (i)
>> +	    {
>> +	    case Tag_RISCV_unaligned_access:
>> +	      if (out_attr[i].i <= 1)
>> +		break;
>> +
>> +	      _bfd_error_handler
>> +		(_("warning: %pB uses non-boolean `unaligned access' attribute; "
>> +		   "converting to boolean"),
>> +		 ibfd);
>> +	      out_attr[i].i = 1;
>> +	      break;
>> +	    }
>> +	}
>> +
>>         return true;
>>       }
>>   
>> @@ -3865,7 +3882,12 @@ riscv_merge_attributes (bfd *ibfd, struc
>>   	break;
>>   
>>         case Tag_RISCV_unaligned_access:
>> -	out_attr[i].i |= in_attr[i].i;
>> +	if (in_attr[i].i > 1)
>> +	  _bfd_error_handler
>> +	    (_("warning: %pB uses non-boolean `unaligned access' attribute; "
>> +	       "converting to boolean"),
>> +	     ibfd);
>> +	out_attr[i].i |= !!in_attr[i].i;
>>   	break;
>>   
>>         case Tag_RISCV_stack_align:
> 
> LGTM.
> 
> Checking the copied attributes for the first input
> object is necessary, since otherwise a non-boolean unaligned_access 
> value from
> the first object would be carried into the output unchanged.
> 
> One very small wording thought: the gas-side diagnostic uses
> `unaligned_access', while this one uses `unaligned access'. Either is fine
> with me, using the same spelling might make the diagnostics more
> consistent, but this does not need to block the patch.

Oh, thanks for noticing. I've added in the underscores, also for the subsequent
stack-align patch.

> Reviewed-by: Jiawei jiawei@iscas.ac.cn

Thanks.

Jan
diff mbox series

Patch

--- a/bfd/elfxx-riscv.c
+++ b/bfd/elfxx-riscv.c
@@ -3777,6 +3777,23 @@  riscv_merge_attributes (bfd *ibfd, struc
 	 initialized.  */
       out_attr[0].i = 1;
 
+      for (i = LEAST_KNOWN_OBJ_ATTRIBUTE; i < NUM_KNOWN_OBJ_ATTRIBUTES; i++)
+	{
+	  switch (i)
+	    {
+	    case Tag_RISCV_unaligned_access:
+	      if (out_attr[i].i <= 1)
+		break;
+
+	      _bfd_error_handler
+		(_("warning: %pB uses non-boolean `unaligned access' attribute; "
+		   "converting to boolean"),
+		 ibfd);
+	      out_attr[i].i = 1;
+	      break;
+	    }
+	}
+
       return true;
     }
 
@@ -3865,7 +3882,12 @@  riscv_merge_attributes (bfd *ibfd, struc
 	break;
 
       case Tag_RISCV_unaligned_access:
-	out_attr[i].i |= in_attr[i].i;
+	if (in_attr[i].i > 1)
+	  _bfd_error_handler
+	    (_("warning: %pB uses non-boolean `unaligned access' attribute; "
+	       "converting to boolean"),
+	     ibfd);
+	out_attr[i].i |= !!in_attr[i].i;
 	break;
 
       case Tag_RISCV_stack_align: