This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[RFA:] Fix libgfortran issues with truncation related to PR35293


Recently, libgfortran had some fd_truncate calls sprinkled, which led to
regressions visible on targets that don't have truncation support, like
generally all newlib targets (except those with "manually" added support
like spu and sh).

While looking into that, it seemed like libgfortran/io/unix.c:fd_truncate
had some bugs that were a bit worse than the lack of handling unusable
truncation-challenged targets, and on closer inspection that impression
became stronger.  The s->special_file case is folded into the *failure*
case instead of the success case (contrary to the comment), and then the
failure case illogically hacked to return success instead of failure
(commit 111924).  Also, it doesn't seem logical to treat the file as empty
for neither failure cases nor special files or to not update the position
markers or the buffer marker.  The patch below fixes these issues.
Unfortunately, few callers of fd_truncate care to check the return value,
so actual file errors will still go undetected.

However, the main purpose of this patch :) is to make a noisy exit for
targets without ftruncate nor chsize support, as requested by Thomas
Koenig.  Bootstrapped and checked on x86_64-unknown-linux-gnu (no
regressions) and crosses to cris-elf, cris-axis-linux-gnu, arm-elf and
mips-elf (the last one failing all/most gfortran tests due to some invalid
memory access for some reason).  On these targets, there's certainly
regressions as none of them support ftruncate (except cris-axis-linux-gnu,
but the simulator doesn't support ftruncate64) as the noisy exit now goes
right through the lack of integrity checks in the testsuite (i.e. there
were false positives), but I'll "fix" that in the next patch.  It's seen
as better this way than breaking all such targets at configure time or to
silently fail or fake success.

Presented as a context diff, as this is one of the cases where a unidiff
is kind-of illegible.  (Beware, at least Fedora subversion-1.2.3-2.1 adds
a "-u" by itself that I had to hack off in my diff-cmd.)

Ok to commit?

libgfortran:

	PR libfortran/35293
	* io/unix.c (fd_truncate): Fold s->special_file case into
	success case of ftruncate/chsize call instead of the failure case.
	Make failure case actually return failure.  Properly update stream
	pointers on failure.  Call runtime_error for targets without
	neither ftruncate nor chsize where such a call would be needed.

Index: io/unix.c
===================================================================
*** io/unix.c	(revision 132865)
--- io/unix.c	(working copy)
*************** fd_truncate (unix_stream * s)
*** 706,721 ****
  
    /* Using ftruncate on a seekable special file (like /dev/null)
       is undefined, so we treat it as if the ftruncate succeeded.  */
  #ifdef HAVE_FTRUNCATE
!   if (s->special_file || ftruncate (s->fd, s->logical_offset))
  #else
! #ifdef HAVE_CHSIZE
!   if (s->special_file || chsize (s->fd, s->logical_offset))
! #endif
  #endif
      {
!       s->physical_offset = s->file_length = 0;
!       return SUCCESS;
      }
  
    s->physical_offset = s->file_length = s->logical_offset;
--- 706,733 ----
  
    /* Using ftruncate on a seekable special file (like /dev/null)
       is undefined, so we treat it as if the ftruncate succeeded.  */
+   if (!s->special_file
+       && (
  #ifdef HAVE_FTRUNCATE
! 	  ftruncate (s->fd, s->logical_offset) != 0
! #elif defined HAVE_CHSIZE
! 	  chsize (s->fd, s->logical_offset) != 0
  #else
! 	  /* If we have neither, always fail and exit, noisily.  */
! 	  runtime_error ("required ftruncate or chsize support not present"), 1
  #endif
+ 	  ))
      {
!       /* The truncation failed and we need to handle this gracefully.
! 	 The file length remains the same, but the file-descriptor
! 	 offset needs adjustment per the successful lseek above.
! 	 (Similarly, the contents of the buffer isn't valid anymore.)
! 	 A ftruncate call does not affect the physical (file-descriptor)
! 	 offset, according to the ftruncate manual, so neither should a
! 	 failed call.  */
!       s->physical_offset = s->logical_offset;
!       s->active = 0;
!       return FAILURE;
      }
  
    s->physical_offset = s->file_length = s->logical_offset;

brgds, H-P


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]