[PATCH v4] ld: Rename a file on Windows fails if target already exists

[Torbjorn SVENSSON]

On 2025-07-22 16:32, Jan Beulich wrote:
> On 22.07.2025 16:14, Torbjörn SVENSSON wrote:
>> Ok for trunk and binutils-2_45-branch?
> 
> Okay for master (iirc Nick gave his okay for the branch already) with a
> few more cosmetics done:

Fixed the cosmetics and pushed as 
233cd5946413108bf4902b22a9cb23ad0a468f5e to master and 
888f048f72609f716dfc6da21f766c22e5866fad to binutils-2_45 branch.

Kind regards,
Torbjörn

> 
>> --- a/ld/ldlang.c
>> +++ b/ld/ldlang.c
>> @@ -43,6 +43,7 @@
>>   #include "hashtab.h"
>>   #include "elf-bfd.h"
>>   #include "bfdver.h"
>> +#include <errno.h>
>>   
>>   #if BFD_SUPPORTS_PLUGINS
>>   #include "plugin.h"
>> @@ -10840,10 +10841,23 @@ cmdline_add_object_only_section (bfd_byte *contents, size_t size)
>>         fatal (_("%P: failed to finish output with object-only section\n"));
>>       }
>>   
>> +  /* ibfd needs to be closed *after* obfd, otherwise ld will crash with a
>> +     segmentation fault.  */
> 
> s/will/may/, to match your observations.
> 
>> +  if (!bfd_close (ibfd))
>> +    {
>> +      einfo (_("%P%F: failed to close input\n"));
>> +    }
> 
> Here and ...
> 
>>     /* Must be freed after bfd_close ().  */
>>     free (isympp);
>>     free (osympp);
>>   
>> +  /* Must unlink to ensure rename works on Windows.  */
>> +  if (unlink (output_filename) && errno != ENOENT)
>> +    {
>> +      einfo (_("%P%F: failed to unlink %s\n"), output_filename);
>> +    }
> 
> ... here you don't really need the figure braces. I won't insist on
> this adjustment, but further down braces aren't used either in similar
> situations.
> 
>> @@ -10854,10 +10868,14 @@ cmdline_add_object_only_section (bfd_byte *contents, size_t size)
>>     return;
>>   
>>   loser:
>> -  free (isympp);
>> -  free (osympp);
> 
> This movement also wants mentioning in the description.
> 
>>     if (obfd)
>>       bfd_close (obfd);
>> +  /* ibfd needs to be closed *after* obfd, otherwise ld will crash with a
>> +     segmentation fault.  */
> 
> Same comment here as above.
> 
> Jan
> 
>> +  if (ibfd)
>> +    bfd_close (ibfd);
>> +  free (isympp);
>> +  free (osympp);
>>     if (ofilename)
>>       {
>>         unlink (ofilename);
>