From 5e7ed206eacbaf97f92765a393af0acffe931934 Mon Sep 17 00:00:00 2001 From: Quincey Koziol Date: Wed, 23 Jan 2002 16:28:24 -0500 Subject: [svn-r4851] Purpose: Bug Fix Description: When file space was returned to the file space free-list for reuse, occasionally raw data allocations which used space from the free-list would overlap with the metadata accumulator and get over-written with the cached information in the accumulator, corrupting the data. Solution: Check if the space about to be recycled on the free-list is going to be used for raw data and also overlaps with the metadata accumulator cache, avoiding using space that fits those criteria. This fixes bug #701 Platforms tested: FreeBSD 4.5 (sleipnir) --- release_docs/RELEASE.txt | 2 + src/H5FD.c | 205 +++++++++++++++++++++++++++++++---------------- src/H5Fprivate.h | 2 + test/Makefile.in | 8 +- test/testhdf5.c | 5 +- test/testhdf5.h | 2 + test/tfile.c | 4 +- test/tmisc.c | 150 ++++++++++++++++++++++++++++++++++ 8 files changed, 299 insertions(+), 79 deletions(-) create mode 100644 test/tmisc.c diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt index f5246c9..26e36c6 100644 --- a/release_docs/RELEASE.txt +++ b/release_docs/RELEASE.txt @@ -101,6 +101,8 @@ Library option). * Fixed bug where variable-length datatypes for attributes was not working correctly. + * Fixed bug where raw data re-allocated from the free-list would sometimes + overlap with the metadata accumulator and get corrupted. QAK - 1/23/02 Configuration ------------- diff --git a/src/H5FD.c b/src/H5FD.c index f13e028..02a87d2 100644 --- a/src/H5FD.c +++ b/src/H5FD.c @@ -1215,28 +1215,42 @@ H5FD_alloc(H5FD_t *file, H5FD_mem_t type, hsize_t size) need_aligned = file->alignment > 1 && size >= file->threshold; while (cur) { file->maxsize = MAX(file->maxsize, cur->size); - if (need_aligned){ - if ((head = cur->addr % file->alignment) == 0){ + if (need_aligned) { + if ((head = cur->addr % file->alignment) == 0) { /* got aligned address*/ - if (cur->size==size){ + if (cur->size==size) { /* exact match */ ret_value = cur->addr; - if (prev) - prev->next = cur->next; - else - file->fl[mapped_type] = cur->next; - H5MM_xfree(cur); - if (size==file->maxsize) - file->maxsize=0; /*unknown*/ - HRETURN(ret_value); - } - if (cur->size>size){ + + /* + * Make certain we don't hand out a block of raw data + * from the free list which overlaps with the metadata + * aggregation buffer (if it's turned on) + */ + if(type==H5FD_MEM_DRAW && + (file->feature_flags&H5FD_FEAT_ACCUMULATE_METADATA) && + H5F_addr_overlap(ret_value,size,file->accum_loc,file->accum_size)) { + ret_value=HADDR_UNDEF; + } /* end if */ + else { + if (prev) + prev->next = cur->next; + else + file->fl[mapped_type] = cur->next; + H5MM_xfree(cur); + if (size==file->maxsize) + file->maxsize=0; /*unknown*/ + HGOTO_DONE(ret_value); + } /* end else */ + } /* end if */ + if (cur->size>size) { if (!best || !found_aligned || cur->sizesize) { best = cur; found_aligned = 1; - } - } - }else{ + } /* end if */ + } /* end if */ + } /* end if */ + else { /* non-aligned address. * check to see if this block is big enough to skip * to the next aligned address and is still big enough @@ -1249,72 +1263,124 @@ H5FD_alloc(H5FD_t *file, H5FD_mem_t type, hsize_t size) */ head = file->alignment - head; /* actual head size */ if (!found_aligned && - cur->size > head && cur->size-head >= size && - (!best || cur->size < best->size)){ - best =cur; - } - } - }else{ + (cur->size > head && cur->size-head >= size) && + (!best || cur->size < best->size)) { + best = cur; + } /* end if */ + } /* end else */ + } /* end if */ + else { /* !need_aligned */ if (cur->size==size) { ret_value = cur->addr; - if (prev) - prev->next = cur->next; - else - file->fl[mapped_type] = cur->next; - H5MM_xfree(cur); - if (size==file->maxsize) - file->maxsize=0; /*unknown*/ - HRETURN(ret_value); - } else if (cur->size>size && (!best || cur->sizesize)) { - best = cur; - } - } + + /* + * Make certain we don't hand out a block of raw data + * from the free list which overlaps with the metadata + * aggregation buffer (if it's turned on) + */ + if(type==H5FD_MEM_DRAW && + (file->feature_flags&H5FD_FEAT_ACCUMULATE_METADATA) && + H5F_addr_overlap(ret_value,size,file->accum_loc,file->accum_size)) { + ret_value=HADDR_UNDEF; + } /* end if */ + else { + if (prev) + prev->next = cur->next; + else + file->fl[mapped_type] = cur->next; + H5MM_xfree(cur); + if (size==file->maxsize) + file->maxsize=0; /*unknown*/ + HGOTO_DONE(ret_value); + } /* end else */ + } /* end if */ + else + if (cur->size>size && (!best || cur->sizesize)) { + best = cur; + } /* end if */ + } /* end else */ prev = cur; cur = cur->next; - } + } /* end while */ + + /* Couldn't find exact match, use best fitting piece found */ if (best) { if (best->size==file->maxsize) file->maxsize=0; /*unknown*/ - if (!need_aligned || found_aligned){ + if (!need_aligned || found_aligned) { /* free only tail */ ret_value = best->addr; - best->addr += size; - best->size -= size; - HRETURN(ret_value); - }else{ - /* split into 3 pieces. Keep the the head and tail in */ - /* the freelist. */ - H5FD_free_t *tmp = H5MM_malloc(sizeof(H5FD_free_t)); + + /* + * Make certain we don't hand out a block of raw data + * from the free list which overlaps with the metadata + * aggregation buffer (if it's turned on) + */ + if(type==H5FD_MEM_DRAW && + (file->feature_flags&H5FD_FEAT_ACCUMULATE_METADATA) && + H5F_addr_overlap(ret_value,size,file->accum_loc,file->accum_size)) { + ret_value=HADDR_UNDEF; + } /* end if */ + else { + best->addr += size; /* Reduce size of block on free list */ + best->size -= size; + HGOTO_DONE(ret_value); + } /* end else */ + } /* end if */ + else { + /* Split into 3 pieces. */ + /* Keep the the head and tail in the freelist. */ + H5FD_free_t *tmp = NULL; head = file->alignment - (best->addr % file->alignment); ret_value = best->addr + head; + + /* + * Make certain we don't hand out a block of raw data + * from the free list which overlaps with the metadata + * aggregation buffer (if it's turned on) + */ + if(type==H5FD_MEM_DRAW && + (file->feature_flags&H5FD_FEAT_ACCUMULATE_METADATA) && + H5F_addr_overlap(ret_value,size,file->accum_loc,file->accum_size)) { + ret_value=HADDR_UNDEF; + } /* end if */ + else { + + /* Attempt to allocate memory for temporary node */ + tmp = H5MM_malloc(sizeof(H5FD_free_t)); #ifdef H5F_DEBUG - if (H5DEBUG(F)) { - HDfprintf(H5DEBUG(F), - "%s: 3 pieces, begin best->addr=%a, best->size=%Hd, " - "head=%Hd, size=%Hd\n", - FUNC, best->addr, best->size, head, size); - } + if (H5DEBUG(F)) { + HDfprintf(H5DEBUG(F), + "%s: 3 pieces, begin best->addr=%a, best->size=%Hd, " + "head=%Hd, size=%Hd\n", + FUNC, best->addr, best->size, head, size); + } #endif - assert(tmp); /* bark in debug mode */ - if (tmp) { - if ((tmp->size = (best->size - head - size))) { - tmp->addr = best->addr + head + size; - tmp->next = best->next; - best->next = tmp; - } else { - /* no tail piece */ - H5MM_xfree(tmp); - } - } else { - /* cannot keep the tail piece. leak file memory. */ - } - best->size = head; - HRETURN(ret_value); - } - } - } + assert(tmp); /* bark in debug mode */ + if (tmp) { + if ((tmp->size = (best->size - head - size))) { + tmp->addr = best->addr + head + size; + tmp->next = best->next; + best->next = tmp; + } /* end if */ + else { + /* no tail piece */ + H5MM_xfree(tmp); + } /* end else */ + } /* end if */ + else { + /* Cannot keep the tail piece. Leak file memory. */ + /* (Only happens if memory allocation fails) */ + } /* end else */ + best->size = head; + HGOTO_DONE(ret_value); + } /* end else */ + } /* end else */ + } /* end if */ + } /* end if */ + #ifdef H5F_DEBUG if (H5DEBUG(F)) { fprintf(H5DEBUG(F), "%s: Could not allocate from freelists\n", FUNC); @@ -1364,6 +1430,7 @@ H5FD_alloc(H5FD_t *file, H5FD_mem_t type, hsize_t size) ret_value=H5FD_real_alloc(file,type,size); } /* end else */ +done: FUNC_LEAVE(ret_value); } @@ -2029,9 +2096,7 @@ H5FD_read(H5FD_t *file, H5FD_mem_t type, hid_t dxpl_id, haddr_t addr, size_t siz /* Check if this information is in the metadata accumulator */ if((file->feature_flags&H5FD_FEAT_ACCUMULATE_METADATA) && - ((addr>=file->accum_loc && addr <(file->accum_loc+file->accum_size)) - || ((addr+size)>file->accum_loc && (addr+size)<=(file->accum_loc+file->accum_size)) - || (addraccum_loc && (addr+size)>file->accum_loc))) { + H5F_addr_overlap(addr,size,file->accum_loc,file->accum_size)) { unsigned char *read_buf=(unsigned char *)buf; /* Pointer to the buffer being read in */ size_t amount_read; /* Amount to read at a time */ diff --git a/src/H5Fprivate.h b/src/H5Fprivate.h index ed5daf6..2b43757 100644 --- a/src/H5Fprivate.h +++ b/src/H5Fprivate.h @@ -170,6 +170,8 @@ typedef struct H5F_t H5F_t; #define H5F_addr_cmp(X,Y) (H5F_addr_eq(X,Y)?0: \ (H5F_addr_lt(X, Y)?-1:1)) #define H5F_addr_pow2(N) ((haddr_t)1<<(N)) +#define H5F_addr_overlap(O1,L1,O2,L2) ((O1O2) || \ + (O1>=O2 && O1<(O2+L2))) /* size of size_t and off_t as they exist on disk */ #define H5F_SIZEOF_ADDR(F) (H5F_sizeof_addr(F)) diff --git a/test/Makefile.in b/test/Makefile.in index 7d4383f..0dc0b2e 100644 --- a/test/Makefile.in +++ b/test/Makefile.in @@ -48,7 +48,7 @@ MOSTLYCLEAN=cmpd_dset.h5 dataset.h5 extend.h5 istore.h5 tfile1.h5 tfile2.h5 \ tattr.h5 tselect.h5 mtime.h5 unlink.h5 \ fillval_[0-9].h5 fillval.raw mount_[0-9].h5 testmeta.h5 \ ttime.h5 trefer[12].h5 tvltypes.h5 tvlstr.h5 flush.h5 enum1.h5 \ - titerate.h5 ttsafe.h5 tarray1.h5 tgenprop.h5 + titerate.h5 ttsafe.h5 tarray1.h5 tgenprop.h5 tmisc.h5 CLEAN=$(TIMINGS) ## Source and object files for programs... The TEST_SRC list contains all the @@ -61,8 +61,8 @@ TEST_SRC=big.c bittests.c cmpd_dset.c dsets.c dtypes.c extend.c \ istore.c lheap.c links.c mount.c mtime.c ohdr.c stab.c tarray.c \ tattr.c tconfig.c testhdf5.c testmeta.c tfile.c tgenprop.c th5s.c \ titerate.c tmeta.c trefer.c tselect.c ttime.c ttbbt.c tvltypes.c tvlstr.c \ - unlink.c enum.c ttsafe.c ttsafe_dcreate.c ttsafe_error.c ttsafe_cancel.c \ - ttsafe_acreate.c gass_write.c gass_read.c gass_append.c \ + tmisc.c unlink.c enum.c ttsafe.c ttsafe_dcreate.c ttsafe_error.c \ + ttsafe_cancel.c ttsafe_acreate.c gass_write.c gass_read.c gass_append.c \ srb_read.c srb_write.c srb_append.c stream_test.c TEST_OBJ=$(TEST_SRC:.c=.lo) @@ -85,7 +85,7 @@ $(TEST_PROGS): $(LIB) $(LIBHDF5) TESTHDF5_OBJ=testhdf5.lo tarray.lo tattr.lo tconfig.lo tfile.lo tgenprop.lo \ th5s.lo titerate.lo tmeta.lo ttime.lo trefer.lo tselect.lo ttbbt.lo \ - tvltypes.lo tvlstr.lo + tvltypes.lo tvlstr.lo tmisc.lo TTS_OBJ=ttsafe.lo ttsafe_dcreate.lo ttsafe_error.lo ttsafe_cancel.lo \ ttsafe_acreate.lo diff --git a/test/testhdf5.c b/test/testhdf5.c index 798c3ee..190cdf3 100644 --- a/test/testhdf5.c +++ b/test/testhdf5.c @@ -138,9 +138,7 @@ main(int argc, char *argv[]) int Summary = 0; int CleanUp = 1; int Cache = 1; - unsigned major, minor, release; - - + unsigned major, minor, release; #if !(defined MAC || defined __MWERKS__ || defined SYMANTEC_C) /* Un-buffer the stdout and stderr */ @@ -170,6 +168,7 @@ main(int argc, char *argv[]) InitTest("iterate", test_iterate, cleanup_iterate, "Group & Attribute Iteration"); InitTest("array", test_array, cleanup_array, "Array Datatypes"); InitTest("genprop", test_genprop, cleanup_genprop, "Generic Properties"); + InitTest("misc", test_misc, cleanup_misc, "Miscellaneous"); Verbosity = 4; /* Default Verbosity is Low */ H5get_libversion(&major, &minor, &release); diff --git a/test/testhdf5.h b/test/testhdf5.h index ff79e75..0868983 100644 --- a/test/testhdf5.h +++ b/test/testhdf5.h @@ -135,6 +135,7 @@ void test_iterate(void); void test_array(void); void test_genprop(void); void test_configure(void); +void test_misc(void); /* Prototypes for the cleanup routines */ void cleanup_metadata(void); @@ -150,5 +151,6 @@ void cleanup_iterate(void); void cleanup_array(void); void cleanup_genprop(void); void cleanup_configure(void); +void cleanup_misc(void); #endif /* HDF5cleanup_H */ diff --git a/test/tfile.c b/test/tfile.c index 2b1eb28..07f8976 100644 --- a/test/tfile.c +++ b/test/tfile.c @@ -830,9 +830,9 @@ test_obj_count_and_id(hid_t fid1, hid_t fid2, hid_t did, hid_t gid1, /* close the two new files */ ret = H5Fclose(fid3); - CHECK(fid3, FAIL, "H5Fclose"); + CHECK(ret, FAIL, "H5Fclose"); ret = H5Fclose(fid4); - CHECK(fid4, FAIL, "H5Fclose"); + CHECK(ret, FAIL, "H5Fclose"); } /**************************************************************** diff --git a/test/tmisc.c b/test/tmisc.c new file mode 100644 index 0000000..6476c32 --- /dev/null +++ b/test/tmisc.c @@ -0,0 +1,150 @@ +/**************************************************************************** + * NCSA HDF * + * Software Development Group * + * National Center for Supercomputing Applications * + * University of Illinois at Urbana-Champaign * + * 605 E. Springfield, Champaign IL 61820 * + * * + * For conditions of distribution and use, see the accompanying * + * hdf/COPYING file. * + * * + ****************************************************************************/ + +/* $Id$ */ + +/*********************************************************** +* +* Test program: tmisc +* +* Test miscellaneous features not tested elsewhere. Generally +* regression tests for bugs that are reported and don't +* have an existing test to add them to. +* +*************************************************************/ + +#include "hdf5.h" +#include "testhdf5.h" + +#define FILE "tmisc.h5" + +/* Definitions for misc. test #1 */ +#define MISC1_VAL (13417386) /* 0xccbbaa */ +#define MISC1_VAL2 (15654348) /* 0xeeddcc */ +#define MISC1_DSET_NAME "/scalar_set" + +/**************************************************************** +** +** test_misc1(): test unlinking a dataset from a group and immediately +** re-using the dataset name +** +****************************************************************/ +static void +test_misc1(void) +{ + int i; + int i_check; + hid_t file, dataspace, dataset; + herr_t ret; + + /* Output message about test being performed */ + MESSAGE(5, ("Testing Unlinking Dataset and Re-creating It\n")); + + file = H5Fcreate(FILE, H5F_ACC_TRUNC, H5P_DEFAULT, H5P_DEFAULT); + CHECK(file, FAIL, "H5Fcreate"); + + dataspace = H5Screate(H5S_SCALAR); + CHECK(dataspace, FAIL, "H5Screate"); + + /* Write the dataset the first time. */ + dataset = H5Dcreate(file, MISC1_DSET_NAME, H5T_NATIVE_INT, dataspace, H5P_DEFAULT); + CHECK(dataset, FAIL, "H5Dcreate"); + + i = MISC1_VAL; + ret = H5Dwrite(dataset, H5T_NATIVE_INT, H5S_ALL, H5S_ALL, H5P_DEFAULT, &i); + CHECK(ret, FAIL, "H5Dwrite"); + + ret = H5Dclose(dataset); + CHECK(ret, FAIL, "H5Dclose"); + + /* Remove the dataset. */ + ret = H5Gunlink(file, MISC1_DSET_NAME); + CHECK(ret, FAIL, "H5Gunlink"); + + /* Write the dataset for the second time with a different value. */ + dataset = H5Dcreate(file, MISC1_DSET_NAME, H5T_NATIVE_INT, dataspace, H5P_DEFAULT); + CHECK(dataset, FAIL, "H5Dcreate"); + + i = MISC1_VAL2; + ret = H5Dwrite(dataset, H5T_NATIVE_INT, H5S_ALL, H5S_ALL, H5P_DEFAULT, &i); + CHECK(ret, FAIL, "H5Dwrite"); + + ret = H5Dclose(dataset); + CHECK(ret, FAIL, "H5Dclose"); + + ret = H5Sclose(dataspace); + CHECK(ret, FAIL, "H5Sclose"); + + ret = H5Fclose(file); + CHECK(ret, FAIL, "H5Fclose"); + + /* Now, check the value written to the dataset, after it was re-created */ + file = H5Fopen(FILE, H5F_ACC_RDONLY, H5P_DEFAULT); + CHECK(file, FAIL, "H5Fopen"); + + dataspace = H5Screate(H5S_SCALAR); + CHECK(dataspace, FAIL, "H5Screate"); + + dataset = H5Dopen(file, MISC1_DSET_NAME); + CHECK(dataset, FAIL, "H5Dopen"); + + ret = H5Dread(dataset, H5T_NATIVE_INT, H5S_ALL, H5S_ALL, H5P_DEFAULT, &i_check); + CHECK(ret, FAIL, "H5Dread"); + VERIFY(i_check,MISC1_VAL2,"H5Dread"); + + ret = H5Sclose(dataspace); + CHECK(ret, FAIL, "H5Sclose"); + + ret = H5Dclose(dataset); + CHECK(ret, FAIL, "H5Dclose"); + + ret = H5Fclose(file); + CHECK(ret, FAIL, "H5Fclose"); + +} /* end test_misc1() */ + +/**************************************************************** +** +** test_misc(): Main misc. test routine. +** +****************************************************************/ +void +test_misc(void) +{ + /* Output message about test being performed */ + MESSAGE(5, ("Testing Miscellaneous Routines\n")); + + /* Various tests, using the same test file */ + test_misc1(); /* Test unlinking a dataset & immediately re-using name */ + +} /* test_misc() */ + + +/*------------------------------------------------------------------------- + * Function: cleanup_misc + * + * Purpose: Cleanup temporary test files + * + * Return: none + * + * Programmer: Albert Cheng + * July 2, 1998 + * + * Modifications: + * + *------------------------------------------------------------------------- + */ +void +cleanup_misc(void) +{ + remove(FILE); +} -- cgit v0.12