Failure with PR16566 new test on Mingw hosts

[Alan Modra]

On Tue, May 23, 2023 at 05:31:37PM +0200, Clément Chigot wrote:
> Hey Alan
> 
> On Tue, May 23, 2023 at 3:11 PM Alan Modra <amodra@gmail.com> wrote:
> >
> > On Tue, May 23, 2023 at 10:48:06AM +0200, Clément Chigot via Binutils wrote:
> > > I don't know why but this seems to be triggered by the new call to
> > > bfd_canonicalize_symtab introduced when we are logging the local
> > > symbols [1].
> > > Any idea about what's going on ?
> >
> > The output file is opened "w+b".  ldwrite() will cause some of the
> > file to be written, but not all is output until bfd_close().  So
> > reading the symbols means the file IO is write, read, write.  Wild
> > guess: no fseek or similar between changes in direction?  bfd_seek
> > doesn't call fseek if the file position doesn't move.
> 
> Nice guess ! It seems to be the issue. Forcing a fseek is fixing it.
> I have a quick workaround which resets the value of
> "link_info.output_bfd->where" after the new bfd_canonicalize_symtab
> call.

The following is a better fix, I think.  I didn't see much impact on
"time make check" by doing this.

diff --git a/bfd/bfdio.c b/bfd/bfdio.c
index 75a3309c582..22c39a7b0cc 100644
--- a/bfd/bfdio.c
+++ b/bfd/bfdio.c
@@ -456,10 +456,6 @@ bfd_seek (bfd *abfd, file_ptr position, int direction)
   if (direction != SEEK_CUR)
     position += offset;
 
-  if ((direction == SEEK_CUR && position == 0)
-      || (direction == SEEK_SET && (ufile_ptr) position == abfd->where))
-    return 0;
-
   result = abfd->iovec->bseek (abfd, position, direction);
   if (result != 0)
     {

> However, I'm not sure what that means. Is it possible to write/read a
> bfd without using bfd_fwrite, bfd_fread ? I didn't find any. But that
> would create a gap between the "where" field and the actual file
> pointer.
> Otherwise, we should probably enforce a fseek when the direction has
> changed. Even if Windows docs says that both fread and fwrite should
> update the file pointer something might be wrong here.

https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/fopen-wfopen
says fseek (or fsetpos or rewind) is necessary when changing from
reading to writing, and the same or fflush when reading after writing.

https://pubs.opengroup.org/onlinepubs/9699919799/functions/fopen.html
says the same.

> Another interesting thing I've discovered. The call to
> bfd_canonicalize_symtab allocates some contents for the strtab section
> header which is then written when the bfd is being closed. In
> elf.c:_bfd_elf_write_object_contents, i_shdrp[<strtab_count>].contents
> is not anymore NULL.
> AFAICT, that part was already written before during the final link
> part. I'm not sure if it has any impact, we are probably writing the
> same stuff again. But I think it's worth mentioning it.

Yes, I think it would be safer to close the output file and reopen for
lang_map if reporting local syms per section in the map is important.

-- 
Alan Modra
Australia Development Lab, IBM