[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

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 */