Fix segfault in XDD when buffer file is smaller than reqsize with -da…

…tapattern wholefile (#38)

This commit rewrites the logic for handling the -datapattern wholefile
option to ensure that even when the blocksize (bs) is smaller than the
request size (reqsize), the code runs without segmentation faults. The
changes include calculating the offset within the data pattern buffer,
allocating a buffer if not already allocated, and copying data in chunks
from the data pattern to the buffer until the entire transfer size is
copied. The offset is correctly calculated and reset after the first
iteration. In addition, a test case also added to verify that the code
correctly handles cases where the blocksize is smaller than the request
size. The test compares the input file and output file to ensure they
match and that the offset is correctly calculated with different numbers
of queue depth (qd).

Signed-off-by: Cade Gore <79346293+cadegore@users.noreply.github.com>

src/common/datapatterns.c

@@ -18,6 +18,7 @@
 #include <assert.h>
 #include "xint.h"
 #include <sys/sysinfo.h>
+#include <sys/param.h>
 
 #define WHOLEFILE_MAX_SIZE_RAM 0.5 // Currently only allow for 50% of RAM
 
@@ -46,22 +47,8 @@ xdd_datapattern_buffer_init(worker_data_t *wdp) {
 	tdp = wdp->wd_tdp;
     dpp = tdp->td_dpp;
 	if (dpp->data_pattern_options & DP_WHOLEFILE_PATTERN) { // Using the whole contents of the file
-		if (tdp->td_dpp->data_pattern_length < ((size_t)tdp->td_xfer_size)) {
-			memset(wdp->wd_task.task_datap,'\0',tdp->td_xfer_size);
-			ucp = (unsigned char *)wdp->wd_task.task_datap;
-			if (dpp->data_pattern_options & DP_REPLICATE_PATTERN) { // Replicate the pattern throughout the buffer
-				remaining_length = tdp->td_xfer_size;
-				while (remaining_length) {
-					if (dpp->data_pattern_length < remaining_length)
-						pattern_length = dpp->data_pattern_length;
-					else pattern_length = remaining_length;
-					memcpy(ucp,dpp->data_pattern,pattern_length);
-					remaining_length -= pattern_length;
-					ucp += pattern_length;
-				}
-			}
-		} else if (tdp->td_dpp->data_pattern_length % tdp->td_xfer_size) {
-			/* 
+		if (tdp->td_dpp->data_pattern_length % tdp->td_xfer_size) {
+			/*
 			 * We will be assign data using each worker_data_t's task_byte_offset during target passes,
 			 * so we just need to check if transfer size is evenly divisable by the
 			 * the target file size here.
@@ -341,7 +328,27 @@ xdd_set_datapattern_from_file_descriptor(target_data_t *tdp, int fd, char *filen
 error:
 	free(dp->data_pattern);
 	return 1;
-} // End of xdd_set_datapattern_from_file_descriptor() 
+} // End of xdd_set_datapattern_from_file_descriptor()
+
+/*----------------------------------------------------------------------------*/
+/* xdd_datapattern_copy_data_to_buffer() - This function copies data from the
+ * target's data pattern buffer to the worker's buffer.
+ */
+static void
+xdd_datapattern_copy_data_to_buffer(unsigned char *buff_ptr,
+						   const unsigned char *data_pattern,
+						   size_t buffer_size, off_t offset,
+						   size_t remaining_size) {
+	size_t memcpy_size;
+
+	while (remaining_size > 0) {
+		memcpy_size = MIN(remaining_size, buffer_size - offset);
+		memcpy(buff_ptr, &(data_pattern[offset]), memcpy_size);
+		buff_ptr += memcpy_size;
+		remaining_size -= memcpy_size;
+		offset = 0;
+	}
+} // End of xdd_datapattern_copy_data_to_buffer()
 
 /*----------------------------------------------------------------------------*/
 /* xdd_datapattern_get_datap_from_offset() - This subroutine will return the
@@ -354,9 +361,13 @@ xdd_datapattern_get_datap_from_offset(worker_data_t *wdp) {
 	unsigned char *buff = NULL;
 	off_t offset = 0;
 	size_t memcpy_size = 0;
+	size_t remaining_size = 0;
+	size_t buffer_size = 0;
 
 	tdp = wdp->wd_tdp;
-
+	// Get the size of the datapattern buffer
+	buffer_size = tdp->td_dpp->data_pattern_length;
+
 	if (tdp->td_dpp->data_pattern_options & DP_WHOLEFILE_PATTERN) {
 		/*
 		 * In the event of a whole file data pattern we can wind up with offsets that
@@ -388,22 +399,32 @@ xdd_datapattern_get_datap_from_offset(worker_data_t *wdp) {
 		 *
 		 * In the unaligned case we will allocate the IO buffer for the worker and copy
 		 * the correct data to it.
-		 */  
-		offset = wdp->wd_task.task_byte_offset % tdp->td_dpp->data_pattern_length;
-		if ((tdp->td_dpp->data_pattern_length - offset) < (size_t)tdp->td_xfer_size) { 
-			// The target's xfer_size does not evenly divide into the whole file's
-			// size, so we have to allocate a buffer (if not already allocated) to
-			// hold the data an copy it over.
-			if (!wdp->wd_bufp_allocated) {
-				buff = xdd_init_io_buffers(wdp);
-				wdp->wd_task.task_datap = buff;
-			} else {
-				buff = wdp->wd_task.task_datap;
-			}
-			memcpy_size = tdp->td_dpp->data_pattern_length - offset; 
-			memcpy(buff, &(tdp->td_dpp->data_pattern[offset]), memcpy_size);
-			memcpy(&buff[memcpy_size], tdp->td_dpp->data_pattern, (tdp->td_xfer_size - memcpy_size));
-		} else {
+		 */
+
+		// Calculate the offset within the datapattern buffer
+		offset = wdp->wd_task.task_byte_offset % buffer_size;
+		// Set the remaining size to the transfer size
+		remaining_size = tdp->td_xfer_size;
+
+		// If the buffer has already allocated, we can copy the data
+		// into buffer. We also handle the offset to ensure the data
+		// is being copied correctly
+		if (wdp->wd_bufp_allocated) {
+			buff = wdp->wd_task.task_datap;
+			unsigned char *buff_ptr = buff;
+			xdd_datapattern_copy_data_to_buffer(buff_ptr, tdp->td_dpp->data_pattern, buffer_size, offset, remaining_size);
+		}
+		// The target's xfer_size does not evenly divide into the whole file's
+		// size, so we have to allocate a buffer (if not already allocated) to
+		// hold the data an copy it over.
+		else if ((buffer_size - offset) < remaining_size) {
+			buff = xdd_init_io_buffers(wdp);
+			wdp->wd_task.task_datap = buff;
+			unsigned char *buff_ptr = buff;
+			xdd_datapattern_copy_data_to_buffer(buff_ptr, tdp->td_dpp->data_pattern, buffer_size, offset, remaining_size);
+		}
+		// Data can be directly used
+		else {
 			buff = &(tdp->td_dpp->data_pattern[offset]);
 		}
 	} else {

tests/functional/test_xdd_file_datapattern.sh

@@ -2,18 +2,20 @@
 #
 # Acceptance test for XDD.
 #
-# Validate the funtionality of -datapattern file/wholefile option by creating a file, setting the datapattern
-# from that file and then verifying the contents match
+# Validate the funtionality of -datapattern file/wholefile option by creating a file,
+# setting the datapattern from that file and then verifying the contents match.
+# Additionally, ensure that when using the wholefile option, XDD does not crash (segfault)
+# if the buffer file is smaller than the requested size (reqsize)
 #
-# Description - terminates XDD after a given amount of seconds have passed
+# Description - Verifies XDD's -datapattern wholefile functionality.
 #
 # Get absolute path to script
-SCRIPT=${BASH_SOURCE[0]}
+SCRIPT="${BASH_SOURCE[0]}"
 SCRIPTPATH=$(dirname "${SCRIPT}")
 
 # Source the test configuration environment
-source "${SCRIPTPATH}"/../test_config
-source "${SCRIPTPATH}"/../common.sh 
+source "${SCRIPTPATH}/../test_config"
+source "${SCRIPTPATH}/../common.sh"
 
 # Perform pre-test
 initialize_test
@@ -23,34 +25,101 @@ log_file="$(get_log_file)"
 input_file="${test_dir}/data1.dat"
 output_file="${test_dir}/data2.dat"
 
-# Create datapattern input file using dd
-dd if=/dev/urandom of="${input_file}" bs=4k count=2
+# Function to test XDD with file-based datapattern
+# Args:
+#   $1: blocksize to create the input file
+#   $2: count (number of blocks to write)
+#   $3: reqsize (size of each I/O request)
+#   $4: numreqs (total number of I/O requests)
+#   $5: queuedepth (number of concurrent requests)
+test_xdd_file_datapattern() {
+    local bs=$1
+    local count=$2
+    local reqsize=$3
+    local numreqs=$4
+    local qd=$5
 
-# Write out file using xdd with -datapattern file
-"${XDDTEST_XDD_EXE}" -op write -reqsize 8 -numreqs 1 -datapattern file "${input_file}" -targets 1 "${output_file}" -qd 1 -passes 1
-# Verify the contents of the file match
-if ! cmp "${input_file}" "${output_file}"
-then
-    echo "Error when comparing files ${input_file} and ${output_file} with -datapattern file" > "${log_file}"
-    cmp "${input_file}" "${output_file}" >> "${log_file}"
-    rm "${input_file}" "${output_file}"
-    finalize_test 1 "Issue with setting datapattern from file with -datapattern file. Contents don't match."
-fi
-
-rm "${output_file}"
-# Write out file using xdd with -datapattern wholefile
-# In this caes we will use two threads to write out the data in serial ordering as the
-# contents of the file will be distributed evenly between both threads.
-"${XDDTEST_XDD_EXE}" -op write -reqsize 4 -numreqs 2 -datapattern wholefile "${input_file}" -targets 1 "${output_file}" -serialordering -qd 2 -passes 1
-# Verify the contents of the file match
-if ! cmp "${input_file}" "${output_file}"
-then
-    echo "Error when comparing files ${input_file} and ${output_file} with -datapattern wholefile" > "${log_file}"
-    cmp "${input_file}" "${output_file}" >> "${log_file}"
+    # Create datapattern input file using dd
+    dd if=/dev/urandom of="${input_file}" bs="${bs}" count="${count}"
+
+    # Write out file using xdd with -datapattern file
+    "${XDDTEST_XDD_EXE}" -op write -reqsize "${reqsize}" -numreqs "${numreqs}" \
+        -datapattern wholefile "${input_file}" -targets 1 "${output_file}" \
+        -looseordering -qd "${qd}" -passes 1
+
+    # Check if the input file is smaller than the output file
+    input_size=$(stat -c%s "${input_file}")
+    output_size=$(stat -c%s "${output_file}")
+
+    # If the input_file is less than output_file
+    # Append the difference to input_file
+    # so when it compares both files, they have the same size
+    # If the input and output files differ in size, they are guaranteed not to match.
+    if ((input_size < output_size)); then
+        dd if="${input_file}" of="${input_file}" \
+            bs=1k \
+            count=$(((output_size - input_size) / 1024)) \
+            oflag=append conv=notrunc
+    elif ((input_size > output_size)); then
+        # Log an error if the input file is already larger in size
+        echo "Error: Input file (${input_size} bytes) is larger than output file. No appending needed." >"${log_file}"
+        finalize_test 1 "Failure: Input file is too large for comparison with the output file."
+        rm "${input_file}" "${output_file}"
+        return
+    fi
+
+    # Verify the contents of the file match
+    if ! cmp "${input_file}" "${output_file}"; then
+        echo "Error when comparing files ${input_file} and ${output_file} with -datapattern file" >"${log_file}"
+        cmp "${input_file}" "${output_file}" >>"${log_file}"
+        rm "${input_file}" "${output_file}"
+        finalize_test 1 "Issue with setting datapattern from file with -datapattern file. Contents don't match."
+    fi
     rm "${input_file}" "${output_file}"
-    finalize_test 1 "Issue with setting datapattern from file with -datapattern file. Contents don't match."
-fi
-rm "${input_file}" "${output_file}"
+}
+
+#
+# Tests below test when blocksize is the same size as the request size
+#
+# Verfies that XDD handles -datapattern wholefile correctly with
+# Using dd: bs=4k, count=2 to create a 8KB input file
+# Using XDD: reqsize=8 blocks, numreqs=1, qd=1 (one thread)
+test_xdd_file_datapattern 4k 2 8 1 1
+
+# Verifies that XDD handles -datapattern wholefile correctly with
+# Using dd: bs=4k, count=2 to create a 8KB input file
+# Using XDD: reqsize=4 blocks, numreqs=2m, qd=2 (two threads)
+test_xdd_file_datapattern 4k 2 4 2 2
+
+#
+# Tests below test when blocksize is smaller than request size
+#
+# Verifies that XDD handles cases where reqsize > blocksize
+# This test confirms that XDD doesn't encounter issues when the request size
+# is larger than the input file size. (#38)
+# Using dd: bs=6k, count=1 to create a 6KB input file.
+# Using XDD: reqsize=7 blocks, numreqs=1, qd=1
+test_xdd_file_datapattern 6k 1 7 1 1
+
+# Verifies correct behavior when reqsize is not a multiple of blocksize
+# This test uses two threads with loose ordering to check proper data
+# distribution across threads.
+# Using dd: bs=5k, count=2 to create a 10KB input file.
+# Using XDD: reqsize=7 blocks, numreqs=2, qd=2 (two threads).
+test_xdd_file_datapattern 5k 2 7 2 2
+
+# Verifies correct behavior when reqsize is a multiple of blocksize
+# This test also uses two threads with loose ordering for overlapping I/O operations.
+# Using dd: bs=6k, count=4 to create a 24KB input file.
+# Using XDD: reqsize=8 blocks, numreqs=3, qd=2.
+test_xdd_file_datapattern 6k 4 8 3 2
+
+# Verifies that XDD handles very large reqsize without segmentation faults
+# Specifically tests when reqsize is significantly larger than blocksize.
+# Using dd: bs=1k, count=1 to create a 1KB input file.
+# Using XDD: reqsize=256 blocks (1MB), numreqs=4, qd=3.
+# This ensures XDD can handle writing large data even if the input file is small.
+test_xdd_file_datapattern 1k 1 256 4 3
 
 # Test passed
 finalize_test 0