[svn-r22072] Purpose: Fix rare corruption bug (HDFFV-7879)

Description:
When using the new object header format, it was possible for corruption to occur
if the first object header chunk changed size such that the lenght of the "chunk
0 size" field changed.  This only occurred if there were messages that had not
been decoded.  The original algorithm that changed the object header chunk size
marked all messages as dirty, causing those that had not been decoded to have
both the raw and native form invalidated.  Changed the algorithm to avoid
marking messages dirty and added assertions to catch the case where messages
are dirtied without being decoded (or recently created) first.

Tested: jam, koala, ostrich (h5committest), durandal

5 files changed, 158 insertions, 12 deletions

diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt
index 7cfb763..8055516 100644
--- a/release_docs/RELEASE.txt
+++ b/release_docs/RELEASE.txt
@@ -365,6 +365,8 @@ Bug Fixes since HDF5-1.8.0 release
     Library
     -------
 
+      - Fixed rare corruption bugs that could occur when using the new object
+            header format. (NAF - 2012/3/15 - HDFFV-7879)
       - Creating a dataset in a read-only file caused seg fault when the file 
             is closed.  It's fixed.  The error stack is returned correctly
             now. (SLU - 2012/1/25. Issue 7756)
diff --git a/src/H5Oalloc.c b/src/H5Oalloc.c
index dfc844d..05322af 100644
--- a/src/H5Oalloc.c
+++ b/src/H5Oalloc.c
@@ -647,17 +647,18 @@ H5O_alloc_extend_chunk(H5F_t *f, hid_t dxpl_id, H5O_t *oh, unsigned chunkno,
     /* Wipe new space for chunk */
     HDmemset(oh->chunk[chunkno].image + old_size, 0, oh->chunk[chunkno].size - old_size);
 
+    /* Move chunk 0 data up if the size flags changed */
+    if(adjust_size_flags)
+        HDmemmove(oh->chunk[0].image + H5O_SIZEOF_HDR(oh) - H5O_SIZEOF_CHKSUM_OH(oh),
+                oh->chunk[0].image + H5O_SIZEOF_HDR(oh) - H5O_SIZEOF_CHKSUM_OH(oh) - extra_prfx_size,
+                old_size - (size_t)H5O_SIZEOF_HDR(oh) + extra_prfx_size);
+
     /* Spin through existing messages, adjusting them */
     for(u = 0; u < oh->nmesgs; u++) {
         /* Adjust raw addresses for messages in this chunk to reflect new 'image' address */
-        if(oh->mesg[u].chunkno == chunkno) {
+        if(oh->mesg[u].chunkno == chunkno)
             oh->mesg[u].raw = oh->chunk[chunkno].image + extra_prfx_size + (oh->mesg[u].raw - old_image);
 
-            /* Flag message as dirty directly */
-            /* (we mark the entire chunk dirty when we update its size) */
-            oh->mesg[u].dirty = TRUE;
-        } /* endif */
-
         /* Find continuation message which points to this chunk and adjust chunk's size */
         /* (Chunk 0 doesn't have a continuation message that points to it and
          * it's size is directly encoded in the object header) */
@@ -1331,8 +1332,9 @@ H5O_move_cont(H5F_t *f, hid_t dxpl_id, H5O_t *oh, unsigned cont_u)
             move_end = cont_msg->raw + cont_msg->raw_size;
             cont_chunkno = cont_msg->chunkno;
 
-            /* Convert continuation message into a null message */
-            if(H5O_release_mesg(f, dxpl_id, oh, cont_msg, TRUE) < 0)
+            /* Convert continuation message into a null message.  Do not delete
+             * the target chunk yet, so we can still copy messages from it. */
+            if(H5O_release_mesg(f, dxpl_id, oh, cont_msg, FALSE) < 0)
                 HGOTO_ERROR(H5E_OHDR, H5E_CANTDELETE, FAIL, "unable to convert into null message")
 
             /* Protect chunk */
@@ -1354,7 +1356,6 @@ H5O_move_cont(H5F_t *f, hid_t dxpl_id, H5O_t *oh, unsigned cont_u)
                         HDmemcpy(move_start, curr_msg->raw - H5O_SIZEOF_MSGHDR_OH(oh), move_size);
                         curr_msg->raw = move_start + H5O_SIZEOF_MSGHDR_OH(oh);
                         curr_msg->chunkno = cont_chunkno;
-                        curr_msg->dirty = TRUE;
                         chk_dirtied = TRUE;
 
                         /* Adjust location to move messages to */
@@ -1362,6 +1363,10 @@ H5O_move_cont(H5F_t *f, hid_t dxpl_id, H5O_t *oh, unsigned cont_u)
                     } /* end else */
                 } /* end if */
 
+            /* Delete the target chunk */
+            if(H5O_chunk_delete(f, dxpl_id, oh, deleted_chunkno) < 0)
+                HGOTO_ERROR(H5E_OHDR, H5E_CANTDELETE, FAIL, "unable to remove chunk from cache")
+
             HDassert(move_start <= (move_end + gap_size));
 
             /* Check if there is space remaining in the continuation message */
diff --git a/src/H5Odbg.c b/src/H5Odbg.c
index ce8b870..5c07b64 100644
--- a/src/H5Odbg.c
+++ b/src/H5Odbg.c
@@ -167,6 +167,8 @@ H5O_assert(const H5O_t *oh)
             H5O_cont_t *cont = (H5O_cont_t *)curr_msg->native;
             hbool_t found_chunk = FALSE;        /* Found a chunk that matches */
 
+            HDassert(cont);
+
             /* Increment # of continuation messages found */
             cont_msgs_found++;
 
@@ -186,6 +188,9 @@ H5O_assert(const H5O_t *oh)
         else {
             meta_space += (size_t)H5O_SIZEOF_MSGHDR_OH(oh);
             mesg_space += curr_msg->raw_size;
+
+            /* Make sure the message has a native form if it is marked dirty */
+            HDassert(curr_msg->native || !curr_msg->dirty);
         } /* end else */
 
         /* Make certain that the message is in a valid chunk */
diff --git a/src/H5Omessage.c b/src/H5Omessage.c
index a3d02d1..f12c835 100644
--- a/src/H5Omessage.c
+++ b/src/H5Omessage.c
@@ -2161,9 +2161,8 @@ H5O_msg_flush(H5F_t *f, H5O_t *oh, H5O_mesg_t *mesg)
     /* Make certain that null messages aren't in chunks w/gaps */
     if(H5O_NULL_ID == msg_id)
         HDassert(oh->chunk[mesg->chunkno].gap == 0);
-
-    /* Unknown messages should always have a native pointer */
-    if(mesg->type == H5O_MSG_UNKNOWN)
+    else
+        /* Non-null messages should always have a native pointer */
         HDassert(mesg->native);
 #endif /* NDEBUG */
 
diff --git a/test/tattr.c b/test/tattr.c
index 82873b5..1285f0e 100644
--- a/test/tattr.c
+++ b/test/tattr.c
@@ -128,6 +128,8 @@ float attr_data5=(float)-5.123;        /* Test data for 5th attribute */
 #define ATTR7_NAME      "attr 1 - 000000"
 #define ATTR8_NAME      "attr 2"
 
+#define LINK1_NAME      "Link1"
+
 #define NATTR_MANY_OLD  350
 #define NATTR_MANY_NEW  35000
 
@@ -10421,6 +10423,137 @@ test_attr_bug7(hid_t fcpl, hid_t fapl)
 
 /****************************************************************
 **
+**  test_attr_bug8(): Test basic H5A (attribute) code.
+**      (Really tests object header code).
+**      Tests adding a link and attribute to a group in such a
+**      way as to cause the "chunk #0 size" field to expand
+**      when some object header messages are not loaded into
+**      cache.  Before the bug was fixed, this would prevent
+**      these messages from being shifted to the correct
+**      position as the expansion algorithm marked them dirty,
+**      invalidating the raw form, when there was no native
+**      form to encode.
+**
+****************************************************************/
+static void
+test_attr_bug8(hid_t fcpl, hid_t fapl)
+{
+    hid_t   fid;            /* File ID */
+    hid_t   aid;            /* Attribute ID */
+    hid_t   sid;            /* Dataspace ID */
+    hid_t   gid;            /* Group ID */
+    hid_t   oid;            /* Object ID */
+    hsize_t dims = 256;     /* Attribute dimensions */
+    H5O_info_t oinfo;       /* Object info */
+    H5A_info_t ainfo;       /* Attribute info */
+    haddr_t root_addr;      /* Root group address */
+    herr_t  ret;            /* Generic return status */
+
+    /* Output message about test being performed */
+    MESSAGE(5, ("Testing attribute expanding object header with undecoded messages\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");
+    gid = H5Gcreate2(fid, GROUP1_NAME, H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT);
+    CHECK(gid, FAIL, "H5Gcreate2");
+
+    /* Get root group address */
+    ret = H5Oget_info(fid, &oinfo);
+    CHECK(ret, FAIL, "H5Oget_info");
+    root_addr = oinfo.addr;
+
+    /*
+     * Create link to root group
+     */
+    ret = H5Lcreate_hard(fid, "/", gid, LINK1_NAME, H5P_DEFAULT, H5P_DEFAULT);
+    CHECK(ret, FAIL, "H5Lcreate_hard");
+
+    /* Close file */
+    ret = H5Fclose(fid);
+    CHECK(ret, FAIL, "H5Fclose");
+    ret = H5Gclose(gid);
+    CHECK(ret, FAIL, "H5Gclose");
+
+    /* Open file */
+    fid = H5Fopen(FILENAME, H5F_ACC_RDONLY, fapl);
+    CHECK(fid, FAIL, "H5Fopen");
+
+    /* Check link */
+    gid = H5Gopen2(fid, GROUP1_NAME, H5P_DEFAULT);
+    CHECK(gid, FAIL, "H5Gopen2");
+    oid = H5Oopen(gid, LINK1_NAME, H5P_DEFAULT);
+    CHECK(oid, FAIL, "H5Oopen");
+    ret = H5Oget_info(oid, &oinfo);
+    CHECK(ret, FAIL, "H5Oget_info");
+    if(oinfo.addr != root_addr)
+        TestErrPrintf("incorrect link target address: addr: %llu, expected: %llu\n", (long long unsigned)oinfo.addr, (long long unsigned)root_addr);
+
+    /* Close file */
+    ret = H5Fclose(fid);
+    CHECK(ret, FAIL, "H5Fclose");
+    ret = H5Gclose(gid);
+    CHECK(ret, FAIL, "H5Gclose");
+    ret = H5Oclose(oid);
+    CHECK(ret, FAIL, "H5Oclose");
+
+    /* Open file */
+    fid = H5Fopen(FILENAME, H5F_ACC_RDWR, fapl);
+    CHECK(fid, FAIL, "H5Fopen");
+
+    /*
+     * Create attribute.  Should cause chunk size field to expand by 1 byte
+     * (1->2).
+     */
+    gid = H5Gopen2(fid, GROUP1_NAME, H5P_DEFAULT);
+    CHECK(gid, FAIL, "H5Gopen2");
+    sid = H5Screate_simple(1, &dims, NULL);
+    CHECK(sid, FAIL, "H5Screate_simple");
+    aid = H5Acreate2(gid, 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 = H5Gclose(gid);
+    CHECK(ret, FAIL, "H5Gclose");
+
+    /* Open file */
+    fid = H5Fopen(FILENAME, H5F_ACC_RDONLY, fapl);
+    CHECK(fid, FAIL, "H5Fopen");
+
+    /* Check link and attribute */
+    gid = H5Gopen2(fid, GROUP1_NAME, H5P_DEFAULT);
+    CHECK(gid, FAIL, "H5Gopen2");
+    oid = H5Oopen(gid, LINK1_NAME, H5P_DEFAULT);
+    CHECK(oid, FAIL, "H5Oopen");
+    ret = H5Oget_info(oid, &oinfo);
+    CHECK(ret, FAIL, "H5Oget_info");
+    if(oinfo.addr != root_addr)
+        TestErrPrintf("incorrect link target address: addr: %llu, expected: %llu\n", (long long unsigned)oinfo.addr, (long long unsigned)root_addr);
+    ret = H5Aget_info_by_name(gid, ".", ATTR1_NAME, &ainfo, H5P_DEFAULT);
+    CHECK(ret, FAIL, "H5Aget_info_by_name");
+    if(ainfo.data_size != dims)
+        TestErrPrintf("attribute data size different: data_size=%llu, should be %llu\n", (long long unsigned)ainfo.data_size, (long long unsigned)dims);
+
+    /* Close IDs */
+    ret = H5Oclose(oid);
+    CHECK(ret, FAIL, "H5Oclose");
+    ret = H5Gclose(gid);
+    CHECK(ret, FAIL, "H5Gclose");
+    ret = H5Sclose(sid);
+    CHECK(ret, FAIL, "H5Sclose");
+    ret = H5Fclose(fid);
+    CHECK(ret, FAIL, "H5Fclose");
+}   /* test_attr_bug8() */
+
+/****************************************************************
+**
 **  test_attr(): Main H5A (attribute) testing routine.
 **
 ****************************************************************/
@@ -10566,6 +10699,7 @@ test_attr(void)
                 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 */
+                test_attr_bug8(my_fcpl, my_fapl);               /* Test attribute expanding object header with undecoded messages */
             } /* end for */
         } /* end if */
         else {
@@ -10593,6 +10727,7 @@ test_attr(void)
             /* 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 */
+            test_attr_bug8(fcpl, my_fapl);                      /* Test attribute expanding object header with undecoded messages */
         } /* end else */
     } /* end for */