diff mbox series

[1/2] RISC-V: Release subset lists on all paths when the linker merges arch attributes

Message ID 20260706114428.3967240-2-ycl669@andestech.com
State New
Headers
Series RISC-V: Check for conflicting extensions when the linker merges arch attributes |

Checks

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

Commit Message

Ethan Y. C. Liang July 6, 2026, 11:44 a.m. UTC 
  riscv_merge_arch_attr_info returned early on the error paths without
releasing in_subsets, out_subsets and merged_subsets, so the nodes
already added to them were leaked.  The leaked nodes of in_subsets
and out_subsets also carry over into the next merge.

Route every post-parse error path through a single cleanup exit that
releases all three subset lists, and drop the now-redundant reset of
merged_subsets at entry.
---
 bfd/elfxx-riscv.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

Comments

Nelson Chu July 7, 2026, 1:01 a.m. UTC | #1 
This looks Okay, thanks.

Nelson

On Mon, Jul 6, 2026 at 7:46 PM Ethan Y. C. Liang <ycl669@andestech.com>
wrote:

> riscv_merge_arch_attr_info returned early on the error paths without
> releasing in_subsets, out_subsets and merged_subsets, so the nodes
> already added to them were leaked.  The leaked nodes of in_subsets
> and out_subsets also carry over into the next merge.
>
> Route every post-parse error path through a single cleanup exit that
> releases all three subset lists, and drop the now-redundant reset of
> merged_subsets at entry.
> ---
>  bfd/elfxx-riscv.c | 21 +++++++++++----------
>  1 file changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c
> index 961c44a9206..fa2f1759f31 100644
> --- a/bfd/elfxx-riscv.c
> +++ b/bfd/elfxx-riscv.c
> @@ -3691,10 +3691,9 @@ riscv_merge_arch_attr_info (bfd *ibfd, char
> *in_arch, char *out_arch,
>  {
>    riscv_subset_t *in, *out;
>    static char *merged_arch_str = NULL;
> +  char *result = NULL;
>
>    unsigned xlen_in, xlen_out;
> -  merged_subsets.head = NULL;
> -  merged_subsets.tail = NULL;
>
>    riscv_parse_subset_t riscv_rps_ld_in =
>      {&in_subsets, _bfd_error_handler, &xlen_in, NULL, false};
> @@ -3710,9 +3709,9 @@ riscv_merge_arch_attr_info (bfd *ibfd, char
> *in_arch, char *out_arch,
>
>    /* Parse subset from ISA string.  */
>    if (!riscv_parse_subset (&riscv_rps_ld_in, in_arch))
> -    return NULL;
> +    goto cleanup;
>    if (!riscv_parse_subset (&riscv_rps_ld_out, out_arch))
> -    return NULL;
> +    goto cleanup;
>
>    /* Checking XLEN.  */
>    if (xlen_out != xlen_in)
> @@ -3720,7 +3719,7 @@ riscv_merge_arch_attr_info (bfd *ibfd, char
> *in_arch, char *out_arch,
>        _bfd_error_handler
>         (_("error: %pB: ISA string of input (%s) doesn't match "
>            "output (%s)"), ibfd, in_arch, out_arch);
> -      return NULL;
> +      goto cleanup;
>      }
>
>    /* Merge subset list.  */
> @@ -3729,18 +3728,18 @@ riscv_merge_arch_attr_info (bfd *ibfd, char
> *in_arch, char *out_arch,
>
>    /* Merge standard extension.  */
>    if (!riscv_merge_std_ext (ibfd, in_arch, out_arch, &in, &out))
> -    return NULL;
> +    goto cleanup;
>
>    /* Merge all non-single letter extensions with single call.  */
>    if (!riscv_merge_multi_letter_ext (&in, &out))
> -    return NULL;
> +    goto cleanup;
>
>    if (xlen_in != xlen_out)
>      {
>        _bfd_error_handler
>         (_("error: %pB: XLEN of input (%u) doesn't match "
>            "output (%u)"), ibfd, xlen_in, xlen_out);
> -      return NULL;
> +      goto cleanup;
>      }
>
>    if (xlen_in != arch_size)
> @@ -3748,7 +3747,7 @@ riscv_merge_arch_attr_info (bfd *ibfd, char
> *in_arch, char *out_arch,
>        _bfd_error_handler
>         (_("error: %pB: unsupported XLEN (%u), you might be "
>            "using wrong emulation"), ibfd, xlen_in);
> -      return NULL;
> +      goto cleanup;
>      }
>
>    /* Free the previous merged_arch_str which called xmalloc.  */
> @@ -3756,13 +3755,15 @@ riscv_merge_arch_attr_info (bfd *ibfd, char
> *in_arch, char *out_arch,
>
>    merged_arch_str = riscv_arch_str (arch_size, &merged_subsets,
>                                     false/* update */);
> +  result = merged_arch_str;
>
> + cleanup:
>    /* Release the subset lists.  */
>    riscv_release_subset_list (&in_subsets);
>    riscv_release_subset_list (&out_subsets);
>    riscv_release_subset_list (&merged_subsets);
>
> -  return merged_arch_str;
> +  return result;
>  }
>
>  /* Merge object attributes from IBFD into output_bfd of INFO.
> --
> 2.54.0
>
>
Nelson Chu July 9, 2026, 1:10 a.m. UTC | #2 
Sorry, forgot to commit.  Committed, thanks.

Nelson

On Tue, Jul 7, 2026 at 9:01 AM Nelson Chu <nelson.chu@sifive.com> wrote:

> This looks Okay, thanks.
>
> Nelson
>
> On Mon, Jul 6, 2026 at 7:46 PM Ethan Y. C. Liang <ycl669@andestech.com>
> wrote:
>
>> riscv_merge_arch_attr_info returned early on the error paths without
>> releasing in_subsets, out_subsets and merged_subsets, so the nodes
>> already added to them were leaked.  The leaked nodes of in_subsets
>> and out_subsets also carry over into the next merge.
>>
>> Route every post-parse error path through a single cleanup exit that
>> releases all three subset lists, and drop the now-redundant reset of
>> merged_subsets at entry.
>> ---
>>  bfd/elfxx-riscv.c | 21 +++++++++++----------
>>  1 file changed, 11 insertions(+), 10 deletions(-)
>>
>> diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c
>> index 961c44a9206..fa2f1759f31 100644
>> --- a/bfd/elfxx-riscv.c
>> +++ b/bfd/elfxx-riscv.c
>> @@ -3691,10 +3691,9 @@ riscv_merge_arch_attr_info (bfd *ibfd, char
>> *in_arch, char *out_arch,
>>  {
>>    riscv_subset_t *in, *out;
>>    static char *merged_arch_str = NULL;
>> +  char *result = NULL;
>>
>>    unsigned xlen_in, xlen_out;
>> -  merged_subsets.head = NULL;
>> -  merged_subsets.tail = NULL;
>>
>>    riscv_parse_subset_t riscv_rps_ld_in =
>>      {&in_subsets, _bfd_error_handler, &xlen_in, NULL, false};
>> @@ -3710,9 +3709,9 @@ riscv_merge_arch_attr_info (bfd *ibfd, char
>> *in_arch, char *out_arch,
>>
>>    /* Parse subset from ISA string.  */
>>    if (!riscv_parse_subset (&riscv_rps_ld_in, in_arch))
>> -    return NULL;
>> +    goto cleanup;
>>    if (!riscv_parse_subset (&riscv_rps_ld_out, out_arch))
>> -    return NULL;
>> +    goto cleanup;
>>
>>    /* Checking XLEN.  */
>>    if (xlen_out != xlen_in)
>> @@ -3720,7 +3719,7 @@ riscv_merge_arch_attr_info (bfd *ibfd, char
>> *in_arch, char *out_arch,
>>        _bfd_error_handler
>>         (_("error: %pB: ISA string of input (%s) doesn't match "
>>            "output (%s)"), ibfd, in_arch, out_arch);
>> -      return NULL;
>> +      goto cleanup;
>>      }
>>
>>    /* Merge subset list.  */
>> @@ -3729,18 +3728,18 @@ riscv_merge_arch_attr_info (bfd *ibfd, char
>> *in_arch, char *out_arch,
>>
>>    /* Merge standard extension.  */
>>    if (!riscv_merge_std_ext (ibfd, in_arch, out_arch, &in, &out))
>> -    return NULL;
>> +    goto cleanup;
>>
>>    /* Merge all non-single letter extensions with single call.  */
>>    if (!riscv_merge_multi_letter_ext (&in, &out))
>> -    return NULL;
>> +    goto cleanup;
>>
>>    if (xlen_in != xlen_out)
>>      {
>>        _bfd_error_handler
>>         (_("error: %pB: XLEN of input (%u) doesn't match "
>>            "output (%u)"), ibfd, xlen_in, xlen_out);
>> -      return NULL;
>> +      goto cleanup;
>>      }
>>
>>    if (xlen_in != arch_size)
>> @@ -3748,7 +3747,7 @@ riscv_merge_arch_attr_info (bfd *ibfd, char
>> *in_arch, char *out_arch,
>>        _bfd_error_handler
>>         (_("error: %pB: unsupported XLEN (%u), you might be "
>>            "using wrong emulation"), ibfd, xlen_in);
>> -      return NULL;
>> +      goto cleanup;
>>      }
>>
>>    /* Free the previous merged_arch_str which called xmalloc.  */
>> @@ -3756,13 +3755,15 @@ riscv_merge_arch_attr_info (bfd *ibfd, char
>> *in_arch, char *out_arch,
>>
>>    merged_arch_str = riscv_arch_str (arch_size, &merged_subsets,
>>                                     false/* update */);
>> +  result = merged_arch_str;
>>
>> + cleanup:
>>    /* Release the subset lists.  */
>>    riscv_release_subset_list (&in_subsets);
>>    riscv_release_subset_list (&out_subsets);
>>    riscv_release_subset_list (&merged_subsets);
>>
>> -  return merged_arch_str;
>> +  return result;
>>  }
>>
>>  /* Merge object attributes from IBFD into output_bfd of INFO.
>> --
>> 2.54.0
>>
>>
diff mbox series

Patch

diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c
index 961c44a9206..fa2f1759f31 100644
--- a/bfd/elfxx-riscv.c
+++ b/bfd/elfxx-riscv.c
@@ -3691,10 +3691,9 @@  riscv_merge_arch_attr_info (bfd *ibfd, char *in_arch, char *out_arch,
 {
   riscv_subset_t *in, *out;
   static char *merged_arch_str = NULL;
+  char *result = NULL;
 
   unsigned xlen_in, xlen_out;
-  merged_subsets.head = NULL;
-  merged_subsets.tail = NULL;
 
   riscv_parse_subset_t riscv_rps_ld_in =
     {&in_subsets, _bfd_error_handler, &xlen_in, NULL, false};
@@ -3710,9 +3709,9 @@  riscv_merge_arch_attr_info (bfd *ibfd, char *in_arch, char *out_arch,
 
   /* Parse subset from ISA string.  */
   if (!riscv_parse_subset (&riscv_rps_ld_in, in_arch))
-    return NULL;
+    goto cleanup;
   if (!riscv_parse_subset (&riscv_rps_ld_out, out_arch))
-    return NULL;
+    goto cleanup;
 
   /* Checking XLEN.  */
   if (xlen_out != xlen_in)
@@ -3720,7 +3719,7 @@  riscv_merge_arch_attr_info (bfd *ibfd, char *in_arch, char *out_arch,
       _bfd_error_handler
 	(_("error: %pB: ISA string of input (%s) doesn't match "
 	   "output (%s)"), ibfd, in_arch, out_arch);
-      return NULL;
+      goto cleanup;
     }
 
   /* Merge subset list.  */
@@ -3729,18 +3728,18 @@  riscv_merge_arch_attr_info (bfd *ibfd, char *in_arch, char *out_arch,
 
   /* Merge standard extension.  */
   if (!riscv_merge_std_ext (ibfd, in_arch, out_arch, &in, &out))
-    return NULL;
+    goto cleanup;
 
   /* Merge all non-single letter extensions with single call.  */
   if (!riscv_merge_multi_letter_ext (&in, &out))
-    return NULL;
+    goto cleanup;
 
   if (xlen_in != xlen_out)
     {
       _bfd_error_handler
 	(_("error: %pB: XLEN of input (%u) doesn't match "
 	   "output (%u)"), ibfd, xlen_in, xlen_out);
-      return NULL;
+      goto cleanup;
     }
 
   if (xlen_in != arch_size)
@@ -3748,7 +3747,7 @@  riscv_merge_arch_attr_info (bfd *ibfd, char *in_arch, char *out_arch,
       _bfd_error_handler
 	(_("error: %pB: unsupported XLEN (%u), you might be "
 	   "using wrong emulation"), ibfd, xlen_in);
-      return NULL;
+      goto cleanup;
     }
 
   /* Free the previous merged_arch_str which called xmalloc.  */
@@ -3756,13 +3755,15 @@  riscv_merge_arch_attr_info (bfd *ibfd, char *in_arch, char *out_arch,
 
   merged_arch_str = riscv_arch_str (arch_size, &merged_subsets,
 				    false/* update */);
+  result = merged_arch_str;
 
+ cleanup:
   /* Release the subset lists.  */
   riscv_release_subset_list (&in_subsets);
   riscv_release_subset_list (&out_subsets);
   riscv_release_subset_list (&merged_subsets);
 
-  return merged_arch_str;
+  return result;
 }
 
 /* Merge object attributes from IBFD into output_bfd of INFO.