From d849a118c28f7bcb57798c1222b539adeb8dbe2b Mon Sep 17 00:00:00 2001 From: Neil Fortner Date: Thu, 15 Mar 2012 13:42:51 -0500 Subject: [svn-r22070] Purpose: Fix rare corruption bug Description: When using the new object header format and adding an attribute with a size near 64K, it was possible for file corruption to occur. This happened only if the first object header chunk was smaller than 256 bytes and then grew to larger than 64K after the attribute was added. Tested: ostrich, jam, koala (h5committest), durandal --- src/H5Oalloc.c | 23 ++++--- src/H5Odbg.c | 26 +++++--- test/tattr.c | 207 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 235 insertions(+), 21 deletions(-) diff --git a/src/H5Oalloc.c b/src/H5Oalloc.c index a21c2b8..dfc844d 100644 --- a/src/H5Oalloc.c +++ b/src/H5Oalloc.c @@ -559,26 +559,27 @@ H5O_alloc_extend_chunk(H5F_t *f, hid_t dxpl_id, H5O_t *oh, unsigned chunkno, /* Check for changing the chunk #0 data size enough to need adjusting the flags */ if(oh->version > H5O_VERSION_1 && chunkno == 0) { uint64_t chunk0_size; /* Size of chunk 0's data */ + size_t orig_prfx_size = (size_t)1 << (oh->flags & H5O_HDR_CHUNK0_SIZE); /* Original prefix size */ HDassert(oh->chunk[0].size >= (size_t)H5O_SIZEOF_HDR(oh)); chunk0_size = oh->chunk[0].size - (size_t)H5O_SIZEOF_HDR(oh); - /* Check for moving from a 1-byte to a 2-byte size encoding */ - if(chunk0_size <= 255 && (chunk0_size + delta) > 255) { - extra_prfx_size = 1; - new_size_flags = H5O_HDR_CHUNK0_2; + /* Check for moving to a 8-byte size encoding */ + if(orig_prfx_size < 8 && (chunk0_size + delta) > 4294967295) { + extra_prfx_size = 8 - orig_prfx_size; + new_size_flags = H5O_HDR_CHUNK0_8; adjust_size_flags = TRUE; } /* end if */ - /* Check for moving from a 2-byte to a 4-byte size encoding */ - else if(chunk0_size <= 65535 && (chunk0_size + delta) > 65535) { - extra_prfx_size = 2; + /* Check for moving to a 4-byte size encoding */ + else if(orig_prfx_size < 4 && (chunk0_size + delta) > 65535) { + extra_prfx_size = 4 - orig_prfx_size; new_size_flags = H5O_HDR_CHUNK0_4; adjust_size_flags = TRUE; } /* end if */ - /* Check for moving from a 4-byte to a 8-byte size encoding */ - else if(chunk0_size <= 4294967295 && (chunk0_size + delta) > 4294967295) { - extra_prfx_size = 4; - new_size_flags = H5O_HDR_CHUNK0_8; + /* Check for moving to a 2-byte size encoding */ + else if(orig_prfx_size < 2 && (chunk0_size + delta) > 255) { + extra_prfx_size = 2 - orig_prfx_size; + new_size_flags = H5O_HDR_CHUNK0_2; adjust_size_flags = TRUE; } /* end if */ } /* end if */ diff --git a/src/H5Odbg.c b/src/H5Odbg.c index bd30b90..ce8b870 100644 --- a/src/H5Odbg.c +++ b/src/H5Odbg.c @@ -106,7 +106,7 @@ H5O_assert(const H5O_t *oh) /* Initialize the tracking variables */ hdr_size = 0; - meta_space = H5O_SIZEOF_HDR(oh) + (H5O_SIZEOF_CHKHDR_OH(oh) * (oh->nchunks - 1)); + meta_space = (size_t)H5O_SIZEOF_HDR(oh) + (size_t)(H5O_SIZEOF_CHKHDR_OH(oh) * (oh->nchunks - 1)); mesg_space = 0; free_space = 0; @@ -140,7 +140,7 @@ H5O_assert(const H5O_t *oh) /* Check for correct chunk #0 size flags */ if(oh->version > H5O_VERSION_1) { - uint64_t chunk0_size = oh->chunk[0].size - H5O_SIZEOF_HDR(oh); + uint64_t chunk0_size = oh->chunk[0].size - (size_t)H5O_SIZEOF_HDR(oh); if(chunk0_size <= 255) HDassert((oh->flags & H5O_HDR_CHUNK0_SIZE) == H5O_HDR_CHUNK0_1); @@ -154,9 +154,15 @@ H5O_assert(const H5O_t *oh) /* Loop over all messages in object header */ for(u = 0, curr_msg = &oh->mesg[0]; u < oh->nmesgs; u++, curr_msg++) { + uint8_t *curr_hdr; /* Start of current message header */ + size_t curr_tot_size; /* Total size of current message (including header) */ + + curr_hdr = curr_msg->raw - H5O_SIZEOF_MSGHDR_OH(oh); + curr_tot_size = curr_msg->raw_size + (size_t)H5O_SIZEOF_MSGHDR_OH(oh); + /* Accumulate information, based on the type of message */ if(H5O_NULL_ID == curr_msg->type->id) - free_space += H5O_SIZEOF_MSGHDR_OH(oh) + curr_msg->raw_size; + free_space += curr_tot_size; else if(H5O_CONT_ID == curr_msg->type->id) { H5O_cont_t *cont = (H5O_cont_t *)curr_msg->native; hbool_t found_chunk = FALSE; /* Found a chunk that matches */ @@ -175,10 +181,10 @@ H5O_assert(const H5O_t *oh) } /* end for */ HDassert(found_chunk); - meta_space += H5O_SIZEOF_MSGHDR_OH(oh) + curr_msg->raw_size; + meta_space += curr_tot_size; } /* end if */ else { - meta_space += H5O_SIZEOF_MSGHDR_OH(oh); + meta_space += (size_t)H5O_SIZEOF_MSGHDR_OH(oh); mesg_space += curr_msg->raw_size; } /* end else */ @@ -190,17 +196,19 @@ H5O_assert(const H5O_t *oh) HDassert(oh->chunk[curr_msg->chunkno].gap == 0); /* Make certain that the message is completely in a chunk message area */ - HDassert(curr_msg->raw_size <= (oh->chunk[curr_msg->chunkno].size) - (H5O_SIZEOF_CHKSUM_OH(oh) + oh->chunk[curr_msg->chunkno].gap)); + HDassert(curr_tot_size <= (oh->chunk[curr_msg->chunkno].size) - (H5O_SIZEOF_CHKSUM_OH(oh) + oh->chunk[curr_msg->chunkno].gap)); if(curr_msg->chunkno == 0) - HDassert(curr_msg->raw >= oh->chunk[curr_msg->chunkno].image + (H5O_SIZEOF_HDR(oh) - H5O_SIZEOF_CHKSUM_OH(oh))); + HDassert(curr_hdr >= oh->chunk[curr_msg->chunkno].image + (H5O_SIZEOF_HDR(oh) - H5O_SIZEOF_CHKSUM_OH(oh))); else - HDassert(curr_msg->raw >= oh->chunk[curr_msg->chunkno].image + (H5O_SIZEOF_CHKHDR_OH(oh) - H5O_SIZEOF_CHKSUM_OH(oh))); + HDassert(curr_hdr >= oh->chunk[curr_msg->chunkno].image + (H5O_SIZEOF_CHKHDR_OH(oh) - H5O_SIZEOF_CHKSUM_OH(oh))); HDassert(curr_msg->raw + curr_msg->raw_size <= (oh->chunk[curr_msg->chunkno].image + oh->chunk[curr_msg->chunkno].size) - (H5O_SIZEOF_CHKSUM_OH(oh) + oh->chunk[curr_msg->chunkno].gap)); /* Make certain that no other messages overlap this message */ for(v = 0, tmp_msg = &oh->mesg[0]; v < oh->nmesgs; v++, tmp_msg++) { if(u != v) - HDassert(!(tmp_msg->raw >= curr_msg->raw && tmp_msg->raw < (curr_msg->raw + curr_msg->raw_size))); + HDassert(!((tmp_msg->raw - H5O_SIZEOF_MSGHDR_OH(oh)) >= curr_hdr + && (tmp_msg->raw - H5O_SIZEOF_MSGHDR_OH(oh)) + < (curr_hdr + curr_tot_size))); } /* end for */ } /* end for */ diff --git a/test/tattr.c b/test/tattr.c index 60d4ddb..82873b5 100644 --- a/test/tattr.c +++ b/test/tattr.c @@ -10216,7 +10216,208 @@ test_attr_bug6(hid_t fcpl, hid_t fapl) ret = H5Sclose(sid); CHECK(ret, FAIL, "H5Sclose"); -} +} /* test_attr_bug6() */ + +/**************************************************************** +** +** test_attr_bug7(): Test basic H5A (attribute) code. +** (Really tests object header allocation code). +** Tests creating and deleting attributes in such a way as +** to change the size of the "chunk #0 size" field. +** Includes testing "skipping" a possible size of the +** field, i.e. going from 1 to 4 bytes or 4 to 1 byte. +** +****************************************************************/ +static void +test_attr_bug7(hid_t fcpl, hid_t fapl) +{ + hid_t fid; /* File ID */ + hid_t aid; /* Attribute ID */ + hid_t sid; /* Dataspace ID */ + hid_t tid; /* Datatype ID */ + hsize_t dims_s = 140; /* Small attribute dimensions */ + hsize_t dims_l = 65480; /* Large attribute dimensions */ + H5A_info_t ainfo; /* Attribute info */ + herr_t ret; /* Generic return status */ + + /* Output message about test being performed */ + MESSAGE(5, ("Testing adding and deleting large attributes\n")); + + + /* Create committed datatype to operate on. Use a committed datatype so that + * there is nothing after the object header and the first chunk can expand and + * contract as necessary. */ + fid = H5Fcreate(FILENAME, H5F_ACC_TRUNC, fcpl, fapl); + CHECK(fid, FAIL, "H5Fcreate"); + tid = H5Tcopy(H5T_STD_I32LE); + CHECK(tid, FAIL, "H5Tcopy"); + ret = H5Tcommit2(fid, TYPE1_NAME, tid, H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT); + CHECK(ret, FAIL, "H5Tcommit2"); + + /* + * Create small attribute + */ + sid = H5Screate_simple(1, &dims_s, NULL); + CHECK(sid, FAIL, "H5Screate_simple"); + aid = H5Acreate2(tid, ATTR1_NAME, H5T_STD_I8LE, sid, H5P_DEFAULT, H5P_DEFAULT); + CHECK(aid, FAIL, "H5Acreate2"); + + /* Close file */ + ret = H5Aclose(aid); + CHECK(ret, FAIL, "H5Aclose"); + ret = H5Fclose(fid); + CHECK(ret, FAIL, "H5Fclose"); + ret = H5Tclose(tid); + CHECK(ret, FAIL, "H5Tclose"); + + /* Open file */ + fid = H5Fopen(FILENAME, H5F_ACC_RDWR, fapl); + CHECK(fid, FAIL, "H5Fopen"); + + /* Check attribute */ + tid = H5Topen2(fid, TYPE1_NAME, H5P_DEFAULT); + CHECK(tid, FAIL, "H5Topen2"); + ret = H5Aget_info_by_name(tid, ".", ATTR1_NAME, &ainfo, H5P_DEFAULT); + CHECK(ret, FAIL, "H5Aget_info_by_name"); + if(ainfo.data_size != dims_s) + TestErrPrintf("attribute data size different: data_size=%llu, should be %llu\n", (long long unsigned)ainfo.data_size, (long long unsigned)dims_s); + + /* + * Create another small attribute. Should cause chunk size field to expand by + * 1 byte (1->2). + */ + aid = H5Acreate2(tid, ATTR2_NAME, H5T_STD_I8LE, sid, H5P_DEFAULT, H5P_DEFAULT); + CHECK(aid, FAIL, "H5Acreate2"); + + /* Close file */ + ret = H5Aclose(aid); + CHECK(ret, FAIL, "H5Aclose"); + ret = H5Fclose(fid); + CHECK(ret, FAIL, "H5Fclose"); + ret = H5Tclose(tid); + CHECK(ret, FAIL, "H5Tclose"); + + /* Open file */ + fid = H5Fopen(FILENAME, H5F_ACC_RDWR, fapl); + CHECK(fid, FAIL, "H5Fopen"); + + /* Check attributes */ + tid = H5Topen2(fid, TYPE1_NAME, H5P_DEFAULT); + CHECK(tid, FAIL, "H5Topen2"); + ret = H5Aget_info_by_name(tid, ".", ATTR1_NAME, &ainfo, H5P_DEFAULT); + CHECK(ret, FAIL, "H5Aget_info_by_name"); + if(ainfo.data_size != dims_s) + TestErrPrintf("attribute data size different: data_size=%llu, should be %llu\n", (long long unsigned)ainfo.data_size, (long long unsigned)dims_s); + ret = H5Aget_info_by_name(tid, ".", ATTR2_NAME, &ainfo, H5P_DEFAULT); + CHECK(ret, FAIL, "H5Aget_info_by_name"); + if(ainfo.data_size != dims_s) + TestErrPrintf("attribute data size different: data_size=%llu, should be %llu\n", (long long unsigned)ainfo.data_size, (long long unsigned)dims_s); + + /* + * Create large attribute. Should cause chunk size field to expand by 2 bytes + * (2->4). + */ + ret = H5Sset_extent_simple(sid, 1, &dims_l, NULL); + CHECK(ret, FAIL, "H5Sset_extent_simple"); + aid = H5Acreate2(tid, ATTR3_NAME, H5T_STD_I8LE, sid, H5P_DEFAULT, H5P_DEFAULT); + CHECK(aid, FAIL, "H5Acreate2"); + + /* Close file */ + ret = H5Aclose(aid); + CHECK(ret, FAIL, "H5Aclose"); + ret = H5Fclose(fid); + CHECK(ret, FAIL, "H5Fclose"); + ret = H5Tclose(tid); + CHECK(ret, FAIL, "H5Tclose"); + + /* Open file */ + fid = H5Fopen(FILENAME, H5F_ACC_RDWR, fapl); + CHECK(fid, FAIL, "H5Fopen"); + + /* Check attributes */ + tid = H5Topen2(fid, TYPE1_NAME, H5P_DEFAULT); + CHECK(tid, FAIL, "H5Topen2"); + ret = H5Aget_info_by_name(tid, ".", ATTR1_NAME, &ainfo, H5P_DEFAULT); + CHECK(ret, FAIL, "H5Aget_info_by_name"); + if(ainfo.data_size != dims_s) + TestErrPrintf("attribute data size different: data_size=%llu, should be %llu\n", (long long unsigned)ainfo.data_size, (long long unsigned)dims_s); + ret = H5Aget_info_by_name(tid, ".", ATTR2_NAME, &ainfo, H5P_DEFAULT); + CHECK(ret, FAIL, "H5Aget_info_by_name"); + if(ainfo.data_size != dims_s) + TestErrPrintf("attribute data size different: data_size=%llu, should be %llu\n", (long long unsigned)ainfo.data_size, (long long unsigned)dims_s); + ret = H5Aget_info_by_name(tid, ".", ATTR3_NAME, &ainfo, H5P_DEFAULT); + CHECK(ret, FAIL, "H5Aget_info_by_name"); + if(ainfo.data_size != dims_l) + TestErrPrintf("attribute data size different: data_size=%llu, should be %llu\n", (long long unsigned)ainfo.data_size, (long long unsigned)dims_l); + + /* + * Delete last two attributes - should merge into a null message that is too + * large, causing the chunk size field to shrink by 3 bytes (4->1). + */ + ret = H5Sset_extent_simple(sid, 1, &dims_l, NULL); + CHECK(ret, FAIL, "H5Sset_extent_simple"); + ret = H5Adelete(tid, ATTR2_NAME); + CHECK(ret, FAIL, "H5Adelete"); + ret = H5Adelete(tid, ATTR3_NAME); + CHECK(ret, FAIL, "H5Adelete"); + + /* Close file */ + ret = H5Fclose(fid); + CHECK(ret, FAIL, "H5Fclose"); + ret = H5Tclose(tid); + CHECK(ret, FAIL, "H5Tclose"); + + /* Open file */ + fid = H5Fopen(FILENAME, H5F_ACC_RDWR, fapl); + CHECK(fid, FAIL, "H5Fopen"); + + /* Check attribute */ + tid = H5Topen2(fid, TYPE1_NAME, H5P_DEFAULT); + CHECK(tid, FAIL, "H5Topen2"); + ret = H5Aget_info_by_name(tid, ".", ATTR1_NAME, &ainfo, H5P_DEFAULT); + CHECK(ret, FAIL, "H5Aget_info_by_name"); + if(ainfo.data_size != dims_s) + TestErrPrintf("attribute data size different: data_size=%llu, should be %llu\n", (long long unsigned)ainfo.data_size, (long long unsigned)dims_s); + + /* + * Create large attribute. Should cause chunk size field to expand by 3 bytes + * (1->4). + */ + aid = H5Acreate2(tid, ATTR2_NAME, H5T_STD_I8LE, sid, H5P_DEFAULT, H5P_DEFAULT); + CHECK(aid, FAIL, "H5Acreate2"); + + /* Close file */ + ret = H5Aclose(aid); + CHECK(ret, FAIL, "H5Aclose"); + ret = H5Fclose(fid); + CHECK(ret, FAIL, "H5Fclose"); + ret = H5Tclose(tid); + CHECK(ret, FAIL, "H5Tclose"); + + /* Open file */ + fid = H5Fopen(FILENAME, H5F_ACC_RDWR, fapl); + CHECK(fid, FAIL, "H5Fopen"); + + /* Check attributes */ + tid = H5Topen2(fid, TYPE1_NAME, H5P_DEFAULT); + CHECK(tid, FAIL, "H5Topen2"); + ret = H5Aget_info_by_name(tid, ".", ATTR1_NAME, &ainfo, H5P_DEFAULT); + CHECK(ret, FAIL, "H5Aget_info_by_name"); + if(ainfo.data_size != dims_s) + TestErrPrintf("attribute data size different: data_size=%llu, should be %llu\n", (long long unsigned)ainfo.data_size, (long long unsigned)dims_s); + ret = H5Aget_info_by_name(tid, ".", ATTR2_NAME, &ainfo, H5P_DEFAULT); + CHECK(ret, FAIL, "H5Aget_info_by_name"); + if(ainfo.data_size != dims_l) + TestErrPrintf("attribute data size different: data_size=%llu, should be %llu\n", (long long unsigned)ainfo.data_size, (long long unsigned)dims_l); + + /* Close IDs */ + ret = H5Tclose(tid); + CHECK(ret, FAIL, "H5Tclose"); + ret = H5Sclose(sid); + CHECK(ret, FAIL, "H5Sclose"); + ret = H5Fclose(fid); + CHECK(ret, FAIL, "H5Fclose"); +} /* test_attr_bug7() */ /**************************************************************** ** @@ -10364,6 +10565,7 @@ test_attr(void) test_attr_bug4(my_fcpl, my_fapl); /* Test attributes on named datatypes */ test_attr_bug5(my_fcpl, my_fapl); /* Test opening/closing attributes through different file handles */ test_attr_bug6(my_fcpl, my_fapl); /* Test reading empty attribute */ + test_attr_bug7(my_fcpl, my_fapl); /* Test creating and deleting large attributes in ohdr chunk 0 */ } /* end for */ } /* end if */ else { @@ -10388,6 +10590,9 @@ test_attr(void) test_attr_bug4(fcpl, my_fapl); /* Test attributes on named datatypes */ test_attr_bug5(fcpl, my_fapl); /* Test opening/closing attributes through different file handles */ test_attr_bug6(fcpl, my_fapl); /* Test reading empty attribute */ + /* Skip test_attr_bug7 because it is specific to the new object + * header format and in fact fails if used with the old format, due + * to the attributes being larger than 64K */ } /* end else */ } /* end for */ -- cgit v0.12