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

Nelson Chu via Binutils <[email protected]>
Newsgroups gmane.comp.gnu.binutils
Message-ID <CAOvetVByQnEDrf5zfTzX6z-LASOP_EZwZTcNDy8aJnE1ASc9xw@mail.gmail.com>
Sorry, forgot to commit.  Committed, thanks.

Nelson

On Tue, Jul 7, 2026 at 9:01 AM Nelson Chu <[email protected]> wrote:

> This looks Okay, thanks.
>
> Nelson
>
> On Mon, Jul 6, 2026 at 7:46 PM Ethan Y. C. Liang <[email protected]>
> 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
>>
>>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.