diff mbox series

ld/pdb: Report failures to write the PDB file

Message ID 20260930193657.733-10-oleg.tolmatcev@gmail.com
State New
Headers
Series ld/pdb: Report failures to write the PDB file |

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

Oleg Tolmatcev Sept. 30, 2026, 7:36 p.m. UTC 
  The streams of a PDB file are only written out when it is closed, so
ignoring the return value of bfd_close means that write errors go
unreported: ld leaves a truncated PDB file behind and exits
successfully.

ld/
	* pdb.c (create_pdb_file): Check the return value of bfd_close.
---
 ld/pdb.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Jan Beulich Oct. 2, 2026, 10:19 a.m. UTC | #1 
On 30.09.2026 21:36, Oleg Tolmatcev wrote:
> The streams of a PDB file are only written out when it is closed, so
> ignoring the return value of bfd_close means that write errors go
> unreported: ld leaves a truncated PDB file behind and exits
> successfully.

Yet then ...

> --- a/ld/pdb.c
> +++ b/ld/pdb.c
> @@ -5410,7 +5410,11 @@ create_pdb_file (bfd *abfd, const char *pdb_name, const unsigned char *guid)
>    ret = true;
>  
>  end:
> -  bfd_close (pdb);
> +  if (!bfd_close (pdb))
> +    {
> +      einfo (_("%P: warning: cannot write PDB file: %E\n"));
> +      ret = false;
> +    }

... why does it only say "warning"? Hmm, the (apparently sole) caller
of the after_write_object_contents() hook (in bfd/coffcode.h) doesn't
check the return value. Hence I guess linking would still succeed?
Wouldn't the (truncated) .pdb file then still be left around?

Jan
Oleg Tolmatcev Oct. 2, 2026, 5:40 p.m. UTC | #2 
пт, 2 окт. 2026 г. в 12:19, Jan Beulich <jbeulich@suse.com>:
>
> On 30.09.2026 21:36, Oleg Tolmatcev wrote:
> > The streams of a PDB file are only written out when it is closed, so
> > ignoring the return value of bfd_close means that write errors go
> > unreported: ld leaves a truncated PDB file behind and exits
> > successfully.
>
> Yet then ...
>
> > --- a/ld/pdb.c
> > +++ b/ld/pdb.c
> > @@ -5410,7 +5410,11 @@ create_pdb_file (bfd *abfd, const char *pdb_name, const unsigned char *guid)
> >    ret = true;
> >
> >  end:
> > -  bfd_close (pdb);
> > +  if (!bfd_close (pdb))
> > +    {
> > +      einfo (_("%P: warning: cannot write PDB file: %E\n"));
> > +      ret = false;
> > +    }
>
> ... why does it only say "warning"? Hmm, the (apparently sole) caller
> of the after_write_object_contents() hook (in bfd/coffcode.h) doesn't
> check the return value. Hence I guess linking would still succeed?
> Wouldn't the (truncated) .pdb file then still be left around?
>
> Jan

Yes it will be left around. I will send a v2 which removes the .pdb
file on failure and aborts.

Oleg
diff mbox series

Patch

diff --git a/ld/pdb.c b/ld/pdb.c
index b349de94263..3885340d567 100644
--- a/ld/pdb.c
+++ b/ld/pdb.c
@@ -5410,7 +5410,11 @@  create_pdb_file (bfd *abfd, const char *pdb_name, const unsigned char *guid)
   ret = true;
 
 end:
-  bfd_close (pdb);
+  if (!bfd_close (pdb))
+    {
+      einfo (_("%P: warning: cannot write PDB file: %E\n"));
+      ret = false;
+    }
 
   htab_delete (strings.hashmap);