[v3,1/3] aarch64: ld: Fix scanning of GNU properties for AARCH64_FEATURE_1_AND

Message ID 20250402130526.1405277-2-yury.khrustalev@arm.com
State Committed
Headers
Series aarch64: Fix scanning of GNU properties |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-arm success Build passed
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_check--master-arm success Test passed

Commit Message

Yury Khrustalev April 2, 2025, 1:05 p.m. UTC
  Fixes [1]. Previously iteration over GNU properties of an input file
could stop before reaching GNU_PROPERTY_AARCH64_FEATURE_1_AND which
would result in incorrect inference of properties of the output file.

In the particular use case described in [1], the memory seal property
GNU_PROPERTY_MEMORY_SEAL with number 3, if present in the input file,
prevented reading information from GNU_PROPERTY_AARCH64_FEATURE_1_AND
property due to filtering by property number.

[1] PR32818 https://sourceware.org/bugzilla/show_bug.cgi?id=32818
---
 bfd/elfxx-aarch64.c | 31 +++++++++++--------------------
 1 file changed, 11 insertions(+), 20 deletions(-)
  

Comments

Richard Earnshaw April 17, 2025, 10:29 a.m. UTC | #1
On 02/04/2025 14:05, Yury Khrustalev wrote:
> Fixes [1]. Previously iteration over GNU properties of an input file
> could stop before reaching GNU_PROPERTY_AARCH64_FEATURE_1_AND which
> would result in incorrect inference of properties of the output file.
> 
> In the particular use case described in [1], the memory seal property
> GNU_PROPERTY_MEMORY_SEAL with number 3, if present in the input file,
> prevented reading information from GNU_PROPERTY_AARCH64_FEATURE_1_AND
> property due to filtering by property number.
> 
> [1] PR32818 https://sourceware.org/bugzilla/show_bug.cgi?id=32818
> ---
>   bfd/elfxx-aarch64.c | 31 +++++++++++--------------------
>   1 file changed, 11 insertions(+), 20 deletions(-)
> 
> diff --git a/bfd/elfxx-aarch64.c b/bfd/elfxx-aarch64.c
> index 45a02058e81..c8477d2f3d1 100644
> --- a/bfd/elfxx-aarch64.c
> +++ b/bfd/elfxx-aarch64.c
> @@ -930,28 +930,19 @@ _bfd_aarch64_elf_link_setup_gnu_properties (struct bfd_link_info *info)
>        GNU properties (if found).  */
>     bfd *pbfd = _bfd_elf_link_setup_gnu_properties (info);
>   
> -  /* If pbfd has any GNU_PROPERTY_AARCH64_FEATURE_1_AND properties, update
> -     outprop accordingly.  */
>     if (pbfd != NULL)
>       {
> -      /* The property list is sorted in order of type.  */
> -      for (elf_property_list *p = elf_properties (pbfd);
> -	   (p != NULL)
> -	   && (GNU_PROPERTY_AARCH64_FEATURE_1_AND <= p->property.pr_type);
> -	   p = p->next)
> -	{
> -	  /* This merge of features should happen only once as all the identical
> -	     properties are supposed to have been merged at this stage by
> -	     _bfd_elf_link_setup_gnu_properties().  */
> -	  if (p->property.pr_type == GNU_PROPERTY_AARCH64_FEATURE_1_AND)
> -	    {
> -	      outprop = (p->property.u.number
> -			 & (GNU_PROPERTY_AARCH64_FEATURE_1_BTI
> -			  | GNU_PROPERTY_AARCH64_FEATURE_1_PAC
> -			  | GNU_PROPERTY_AARCH64_FEATURE_1_GCS));
> -	      break;
> -	    }
> -	}
> +      elf_property_list *p, *plist = elf_properties (pbfd);

Please split the declaration of p and the declaration+definition of 
plist into separate statements.

OK with that change.  Nice clean-up, thanks.

R.

> +
> +      /* If pbfd has any GNU_PROPERTY_AARCH64_FEATURE_1_AND properties, update
> +	 outprop accordingly.  */
> +      if ((p = _bfd_elf_find_property (plist,
> +				       GNU_PROPERTY_AARCH64_FEATURE_1_AND, NULL))
> +				       != NULL)
> +        outprop = p->property.u.number
> +		  & (GNU_PROPERTY_AARCH64_FEATURE_1_BTI
> +		     | GNU_PROPERTY_AARCH64_FEATURE_1_PAC
> +		     | GNU_PROPERTY_AARCH64_FEATURE_1_GCS);
>       }
>   
>     tdata->gnu_property_aarch64_feature_1_and = outprop;
  

Patch

diff --git a/bfd/elfxx-aarch64.c b/bfd/elfxx-aarch64.c
index 45a02058e81..c8477d2f3d1 100644
--- a/bfd/elfxx-aarch64.c
+++ b/bfd/elfxx-aarch64.c
@@ -930,28 +930,19 @@  _bfd_aarch64_elf_link_setup_gnu_properties (struct bfd_link_info *info)
      GNU properties (if found).  */
   bfd *pbfd = _bfd_elf_link_setup_gnu_properties (info);
 
-  /* If pbfd has any GNU_PROPERTY_AARCH64_FEATURE_1_AND properties, update
-     outprop accordingly.  */
   if (pbfd != NULL)
     {
-      /* The property list is sorted in order of type.  */
-      for (elf_property_list *p = elf_properties (pbfd);
-	   (p != NULL)
-	   && (GNU_PROPERTY_AARCH64_FEATURE_1_AND <= p->property.pr_type);
-	   p = p->next)
-	{
-	  /* This merge of features should happen only once as all the identical
-	     properties are supposed to have been merged at this stage by
-	     _bfd_elf_link_setup_gnu_properties().  */
-	  if (p->property.pr_type == GNU_PROPERTY_AARCH64_FEATURE_1_AND)
-	    {
-	      outprop = (p->property.u.number
-			 & (GNU_PROPERTY_AARCH64_FEATURE_1_BTI
-			  | GNU_PROPERTY_AARCH64_FEATURE_1_PAC
-			  | GNU_PROPERTY_AARCH64_FEATURE_1_GCS));
-	      break;
-	    }
-	}
+      elf_property_list *p, *plist = elf_properties (pbfd);
+
+      /* If pbfd has any GNU_PROPERTY_AARCH64_FEATURE_1_AND properties, update
+	 outprop accordingly.  */
+      if ((p = _bfd_elf_find_property (plist,
+				       GNU_PROPERTY_AARCH64_FEATURE_1_AND, NULL))
+				       != NULL)
+        outprop = p->property.u.number
+		  & (GNU_PROPERTY_AARCH64_FEATURE_1_BTI
+		     | GNU_PROPERTY_AARCH64_FEATURE_1_PAC
+		     | GNU_PROPERTY_AARCH64_FEATURE_1_GCS);
     }
 
   tdata->gnu_property_aarch64_feature_1_and = outprop;