From 99f85cbf15a637768c8df977ddc5eebe7683a60d Mon Sep 17 00:00:00 2001 From: Dana Robinson Date: Sun, 8 Dec 2019 21:52:29 -0800 Subject: Fixed bugs in pread/pwrite I/O in VFDs. Fixes HDFFV-10945. --- release_docs/RELEASE.txt | 15 +++++++++++++++ src/H5FDcore.c | 12 +++++++----- src/H5FDlog.c | 10 ++++++---- src/H5FDsec2.c | 14 ++++++++------ 4 files changed, 36 insertions(+), 15 deletions(-) diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt index f668207..3ddb9c7 100644 --- a/release_docs/RELEASE.txt +++ b/release_docs/RELEASE.txt @@ -609,6 +609,21 @@ Bug Fixes since HDF5-1.10.3 release (DER - 2019/08/27, HDFFV-10892) + - Torn pread/pwrite I/O would result in read and write corruption. + + In the sec2, log, and core (with backing store) virtual file drivers + (VFDs), the read and write calls incorrectly reset the offset parameter + on torn pread and pwrite operations (i.e., I/O operations which fail to + be written atomically by the OS). For this bug to occur, pread/pwrite + have to be configured (this is the default if they are present on the + system) and the pread/pwrite operation has to fail to transfer all + the bytes, resulting in a multiple pread/pwrite calls. + + This feature was initially enabled in HDF5 1.10.5 so the bug is + limited to that version. + + (DER - 2019/12/09, HDFFV-10945) + Java Library: ---------------- diff --git a/src/H5FDcore.c b/src/H5FDcore.c index 6db8af6..552b7ca 100644 --- a/src/H5FDcore.c +++ b/src/H5FDcore.c @@ -342,6 +342,7 @@ H5FD__core_write_to_bstore(H5FD_core_t *file, haddr_t addr, size_t size) unsigned char *ptr = file->mem + addr; /* mutable pointer into the * buffer (can't change mem) */ + HDoff_t offset = (HDoff_t)addr; /* Offset to write at */ herr_t ret_value = SUCCEED; /* Return value */ FUNC_ENTER_NOAPI_NOINIT @@ -358,7 +359,6 @@ H5FD__core_write_to_bstore(H5FD_core_t *file, haddr_t addr, size_t size) h5_posix_io_t bytes_in = 0; /* # of bytes to write */ h5_posix_io_ret_t bytes_wrote = -1; /* # of bytes written */ - HDoff_t offset = (HDoff_t)addr; /* Trying to write more bytes than the return type can handle is * undefined behavior in POSIX. @@ -371,7 +371,8 @@ H5FD__core_write_to_bstore(H5FD_core_t *file, haddr_t addr, size_t size) do { #ifdef H5_HAVE_PREADWRITE bytes_wrote = HDpwrite(file->fd, ptr, bytes_in, offset); - offset += bytes_wrote; + if(bytes_wrote > 0) + offset += bytes_wrote; #else bytes_wrote = HDwrite(file->fd, ptr, bytes_in); #endif /* H5_HAVE_PREADWRITE */ @@ -856,12 +857,12 @@ H5FD__core_open(const char *name, unsigned flags, hid_t fapl_id, haddr_t maxaddr * partial results, and the end of the file. */ - uint8_t *mem = file->mem; /* memory pointer for writes */ + uint8_t *mem = file->mem; /* memory pointer for writes */ + HDoff_t offset = (HDoff_t)0; /* offset for reading */ while(size > 0) { h5_posix_io_t bytes_in = 0; /* # of bytes to read */ h5_posix_io_ret_t bytes_read = -1; /* # of bytes actually read */ - HDoff_t offset = (HDoff_t)0; /* Trying to read more bytes than the return type can handle is * undefined behavior in POSIX. @@ -874,7 +875,8 @@ H5FD__core_open(const char *name, unsigned flags, hid_t fapl_id, haddr_t maxaddr do { #ifdef H5_HAVE_PREADWRITE bytes_read = HDpread(file->fd, mem, bytes_in, offset); - offset += bytes_read; + if(bytes_read > 0) + offset += bytes_read; #else bytes_read = HDread(file->fd, mem, bytes_in); #endif /* H5_HAVE_PREADWRITE */ diff --git a/src/H5FDlog.c b/src/H5FDlog.c index ac5667f..60255e5 100644 --- a/src/H5FDlog.c +++ b/src/H5FDlog.c @@ -1171,6 +1171,7 @@ H5FD_log_read(H5FD_t *_file, H5FD_mem_t type, hid_t H5_ATTR_UNUSED dxpl_id, hadd #ifdef H5_HAVE_GETTIMEOFDAY struct timeval timeval_start, timeval_stop; #endif /* H5_HAVE_GETTIMEOFDAY */ + HDoff_t offset = (HDoff_t)addr; herr_t ret_value = SUCCEED; /* Return value */ FUNC_ENTER_NOAPI_NOINIT @@ -1255,7 +1256,6 @@ H5FD_log_read(H5FD_t *_file, H5FD_mem_t type, hid_t H5_ATTR_UNUSED dxpl_id, hadd h5_posix_io_t bytes_in = 0; /* # of bytes to read */ h5_posix_io_ret_t bytes_read = -1; /* # of bytes actually read */ - HDoff_t offset = (HDoff_t)addr; /* Trying to read more bytes than the return type can handle is * undefined behavior in POSIX. @@ -1268,7 +1268,8 @@ H5FD_log_read(H5FD_t *_file, H5FD_mem_t type, hid_t H5_ATTR_UNUSED dxpl_id, hadd do { #ifdef H5_HAVE_PREADWRITE bytes_read = HDpread(file->fd, buf, bytes_in, offset); - offset += bytes_read; + if(bytes_read > 0) + offset += bytes_read; #else bytes_read = HDread(file->fd, buf, bytes_in); #endif /* H5_HAVE_PREADWRITE */ @@ -1382,6 +1383,7 @@ H5FD_log_write(H5FD_t *_file, H5FD_mem_t type, hid_t H5_ATTR_UNUSED dxpl_id, had #ifdef H5_HAVE_GETTIMEOFDAY struct timeval timeval_start, timeval_stop; #endif /* H5_HAVE_GETTIMEOFDAY */ + HDoff_t offset = (HDoff_t)addr; herr_t ret_value = SUCCEED; /* Return value */ FUNC_ENTER_NOAPI_NOINIT @@ -1471,7 +1473,6 @@ H5FD_log_write(H5FD_t *_file, H5FD_mem_t type, hid_t H5_ATTR_UNUSED dxpl_id, had h5_posix_io_t bytes_in = 0; /* # of bytes to write */ h5_posix_io_ret_t bytes_wrote = -1; /* # of bytes written */ - HDoff_t offset = (HDoff_t)addr; /* Trying to write more bytes than the return type can handle is * undefined behavior in POSIX. @@ -1484,7 +1485,8 @@ H5FD_log_write(H5FD_t *_file, H5FD_mem_t type, hid_t H5_ATTR_UNUSED dxpl_id, had do { #ifdef H5_HAVE_PREADWRITE bytes_wrote = HDpwrite(file->fd, buf, bytes_in, offset); - offset += bytes_wrote; + if(bytes_wrote > 0) + offset += bytes_wrote; #else bytes_wrote = HDwrite(file->fd, buf, bytes_in); #endif /* H5_HAVE_PREADWRITE */ diff --git a/src/H5FDsec2.c b/src/H5FDsec2.c index 0054a86..a393490 100644 --- a/src/H5FDsec2.c +++ b/src/H5FDsec2.c @@ -669,6 +669,7 @@ H5FD_sec2_read(H5FD_t *_file, H5FD_mem_t H5_ATTR_UNUSED type, hid_t H5_ATTR_UNUS haddr_t addr, size_t size, void *buf /*out*/) { H5FD_sec2_t *file = (H5FD_sec2_t *)_file; + HDoff_t offset = (HDoff_t)addr; herr_t ret_value = SUCCEED; /* Return value */ FUNC_ENTER_NOAPI_NOINIT @@ -687,7 +688,7 @@ H5FD_sec2_read(H5FD_t *_file, H5FD_mem_t H5_ATTR_UNUSED type, hid_t H5_ATTR_UNUS if(addr != file->pos || OP_READ != file->op) { if(HDlseek(file->fd, (HDoff_t)addr, SEEK_SET) < 0) HSYS_GOTO_ERROR(H5E_IO, H5E_SEEKERROR, FAIL, "unable to seek to proper position") - } /* end if */ + } #endif /* H5_HAVE_PREADWRITE */ /* Read data, being careful of interrupted system calls, partial results, @@ -697,7 +698,6 @@ H5FD_sec2_read(H5FD_t *_file, H5FD_mem_t H5_ATTR_UNUSED type, hid_t H5_ATTR_UNUS h5_posix_io_t bytes_in = 0; /* # of bytes to read */ h5_posix_io_ret_t bytes_read = -1; /* # of bytes actually read */ - HDoff_t offset = (HDoff_t)addr; /* Trying to read more bytes than the return type can handle is * undefined behavior in POSIX. @@ -710,7 +710,8 @@ H5FD_sec2_read(H5FD_t *_file, H5FD_mem_t H5_ATTR_UNUSED type, hid_t H5_ATTR_UNUS do { #ifdef H5_HAVE_PREADWRITE bytes_read = HDpread(file->fd, buf, bytes_in, offset); - offset += bytes_read; + if(bytes_read > 0) + offset += bytes_read; #else bytes_read = HDread(file->fd, buf, bytes_in); #endif /* H5_HAVE_PREADWRITE */ @@ -773,6 +774,7 @@ H5FD_sec2_write(H5FD_t *_file, H5FD_mem_t H5_ATTR_UNUSED type, hid_t H5_ATTR_UNU haddr_t addr, size_t size, const void *buf) { H5FD_sec2_t *file = (H5FD_sec2_t *)_file; + HDoff_t offset = (HDoff_t)addr; herr_t ret_value = SUCCEED; /* Return value */ FUNC_ENTER_NOAPI_NOINIT @@ -791,7 +793,7 @@ H5FD_sec2_write(H5FD_t *_file, H5FD_mem_t H5_ATTR_UNUSED type, hid_t H5_ATTR_UNU if(addr != file->pos || OP_WRITE != file->op) { if(HDlseek(file->fd, (HDoff_t)addr, SEEK_SET) < 0) HSYS_GOTO_ERROR(H5E_IO, H5E_SEEKERROR, FAIL, "unable to seek to proper position") - } /* end if */ + } #endif /* H5_HAVE_PREADWRITE */ /* Write the data, being careful of interrupted system calls and partial @@ -801,7 +803,6 @@ H5FD_sec2_write(H5FD_t *_file, H5FD_mem_t H5_ATTR_UNUSED type, hid_t H5_ATTR_UNU h5_posix_io_t bytes_in = 0; /* # of bytes to write */ h5_posix_io_ret_t bytes_wrote = -1; /* # of bytes written */ - HDoff_t offset = (HDoff_t)addr; /* Trying to write more bytes than the return type can handle is * undefined behavior in POSIX. @@ -814,7 +815,8 @@ H5FD_sec2_write(H5FD_t *_file, H5FD_mem_t H5_ATTR_UNUSED type, hid_t H5_ATTR_UNU do { #ifdef H5_HAVE_PREADWRITE bytes_wrote = HDpwrite(file->fd, buf, bytes_in, offset); - offset += bytes_wrote; + if(bytes_wrote > 0) + offset += bytes_wrote; #else bytes_wrote = HDwrite(file->fd, buf, bytes_in); #endif /* H5_HAVE_PREADWRITE */ -- cgit v0.12