From b043126e5e502d198b222139677bc08c9915a12b Mon Sep 17 00:00:00 2001 From: Neil Fortner Date: Mon, 7 Jan 2019 15:20:11 -0600 Subject: Delay checking if decoded message's "shareable" flag is appropriate for the message type until we've verified we understand the message type. Reduce size of H5O_msg_class_g to *not* include space for H5O_BOGUS_INVALID. Make bogus messages shareable. Add new bogus message test with shareable messages to cover the formerly problematic code. Re-run gen_bogus.c to add this test case and also to fix the bogus_invalid messages that were no longer H5O_BOGUS_INVLAID due to a new message class being added in a previous commit. Added comment to remind developers to run gen_bogus.c when adding a new message class. --- src/H5Obogus.c | 4 ++-- src/H5Ocache.c | 17 ++++++++++++----- src/H5Omessage.c | 25 ++++++++++++++++--------- src/H5Opkg.h | 2 +- src/H5Oprivate.h | 5 +++-- test/gen_bogus.c | 8 ++++++++ test/ohdr.c | 10 ++++++++++ test/tbogus.h5 | Bin 4512 -> 5056 bytes 8 files changed, 52 insertions(+), 19 deletions(-) diff --git a/src/H5Obogus.c b/src/H5Obogus.c index fbf04f2..f3cca0f 100644 --- a/src/H5Obogus.c +++ b/src/H5Obogus.c @@ -49,7 +49,7 @@ const H5O_msg_class_t H5O_MSG_BOGUS_VALID[1] = {{ H5O_BOGUS_VALID_ID, /*message id number */ "bogus valid", /*message name for debugging */ 0, /*native message size */ - 0, /* messages are sharable? */ + H5O_SHARE_IS_SHARABLE, /* messages are sharable? */ H5O__bogus_decode, /*decode message */ H5O_bogus_encode, /*encode message */ NULL, /*copy the native value */ @@ -73,7 +73,7 @@ const H5O_msg_class_t H5O_MSG_BOGUS_INVALID[1] = {{ H5O_BOGUS_INVALID_ID, /*message id number */ "bogus invalid", /*message name for debugging */ 0, /*native message size */ - 0, /* messages are sharable? */ + H5O_SHARE_IS_SHARABLE, /* messages are sharable? */ H5O__bogus_decode, /*decode message */ H5O_bogus_encode, /*encode message */ NULL, /*copy the native value */ diff --git a/src/H5Ocache.c b/src/H5Ocache.c index fba4f6e..23c38b9 100644 --- a/src/H5Ocache.c +++ b/src/H5Ocache.c @@ -1402,10 +1402,9 @@ H5O__chunk_deserialize(H5O_t *oh, haddr_t addr, size_t len, const uint8_t *image HGOTO_ERROR(H5E_OHDR, H5E_CANTLOAD, FAIL, "bad flag combination for message") if((flags & H5O_MSG_FLAG_WAS_UNKNOWN) && !(flags & H5O_MSG_FLAG_MARK_IF_UNKNOWN)) HGOTO_ERROR(H5E_OHDR, H5E_CANTLOAD, FAIL, "bad flag combination for message") - if((flags & H5O_MSG_FLAG_SHAREABLE) - && H5O_msg_class_g[id] - && !(H5O_msg_class_g[id]->share_flags & H5O_SHARE_IS_SHARABLE)) - HGOTO_ERROR(H5E_OHDR, H5E_CANTLOAD, FAIL, "message of unsharable class flagged as sharable") + /* Delay checking the "shareable" flag until we've made sure id + * references a valid message class that this version of the library + * knows about */ /* Reserved bytes/creation index */ if(oh->version == H5O_VERSION_1) @@ -1517,9 +1516,17 @@ H5O__chunk_deserialize(H5O_t *oh, haddr_t addr, size_t len, const uint8_t *image mesgs_modified = TRUE; } /* end if */ } /* end if */ - else + else { + /* Check for message of unshareable class marked as "shareable" + */ + if((flags & H5O_MSG_FLAG_SHAREABLE) + && H5O_msg_class_g[id] + && !(H5O_msg_class_g[id]->share_flags & H5O_SHARE_IS_SHARABLE)) + HGOTO_ERROR(H5E_OHDR, H5E_CANTLOAD, FAIL, "message of unshareable class flagged as shareable") + /* Set message class for "known" messages */ mesg->type = H5O_msg_class_g[id]; + } /* end else */ /* Do some inspection/interpretation of new messages from this chunk */ /* (detect continuation messages, ref. count messages, etc.) */ diff --git a/src/H5Omessage.c b/src/H5Omessage.c index d6e3dce..18f3706 100644 --- a/src/H5Omessage.c +++ b/src/H5Omessage.c @@ -1598,16 +1598,23 @@ H5O_msg_is_shared(unsigned type_id, const void *mesg) FUNC_ENTER_NOAPI_NOINIT_NOERR /* Check args */ - HDassert(type_id < NELMTS(H5O_msg_class_g)); - type = H5O_msg_class_g[type_id]; /* map the type ID to the actual type object */ - HDassert(type); - HDassert(mesg); - - /* If messages in a class aren't sharable, then obviously this message isn't shared! :-) */ - if(type->share_flags & H5O_SHARE_IS_SHARABLE) - ret_value = H5O_IS_STORED_SHARED(((const H5O_shared_t *)mesg)->type); - else +#ifdef H5O_ENABLE_BOGUS + if(type_id >= NELMTS(H5O_msg_class_g)) ret_value = FALSE; + else +#endif /* H5O_ENABLE_BOGUS */ + { + HDassert(type_id < NELMTS(H5O_msg_class_g)); + type = H5O_msg_class_g[type_id]; /* map the type ID to the actual type object */ + HDassert(type); + HDassert(mesg); + + /* If messages in a class aren't sharable, then obviously this message isn't shared! :-) */ + if(type->share_flags & H5O_SHARE_IS_SHARABLE) + ret_value = H5O_IS_STORED_SHARED(((const H5O_shared_t *)mesg)->type); + else + ret_value = FALSE; + } /* end block/else */ FUNC_LEAVE_NOAPI(ret_value) } /* end H5O_msg_is_shared() */ diff --git a/src/H5Opkg.h b/src/H5Opkg.h index dbc8089..668f4f0 100644 --- a/src/H5Opkg.h +++ b/src/H5Opkg.h @@ -29,7 +29,7 @@ #define H5O_NMESGS 8 /*initial number of messages */ #define H5O_NCHUNKS 2 /*initial number of chunks */ #define H5O_MIN_SIZE 22 /* Min. obj header data size (must be big enough for a message prefix and a continuation message) */ -#define H5O_MSG_TYPES 27 /* # of types of messages */ +#define H5O_MSG_TYPES 26 /* # of types of messages */ #define H5O_MAX_CRT_ORDER_IDX 65535 /* Max. creation order index value */ /* Versions of object header structure */ diff --git a/src/H5Oprivate.h b/src/H5Oprivate.h index e0926e2..5987ecf 100644 --- a/src/H5Oprivate.h +++ b/src/H5Oprivate.h @@ -227,11 +227,11 @@ typedef struct H5O_copy_t { * Note: Must increment H5O_MSG_TYPES in H5Opkg.h and update H5O_msg_class_g * in H5O.c when creating a new message type. Also bump the value of * H5O_BOGUS_INVALID_ID, below, to be one greater than the value of - * H5O_UNKNOWN_ID. + * H5O_UNKNOWN_ID, and re-run gen_bogus.c. * * (this should never exist in a file) */ -#define H5O_BOGUS_INVALID_ID 0x001A /* "Bogus invalid" Message. */ +#define H5O_BOGUS_INVALID_ID 0x001a /* "Bogus invalid" Message. */ /* Shared object message types. * Shared objects can be committed, in which case the shared message contains @@ -651,6 +651,7 @@ typedef struct H5O_layout_t { */ #define H5O_BOGUS_VALUE 0xdeadbeef typedef struct H5O_bogus_t { + H5O_shared_t sh_loc; /* Shared message info (must be first) */ unsigned u; /* Hold the bogus info */ } H5O_bogus_t; #endif /* H5O_ENABLE_BOGUS */ diff --git a/test/gen_bogus.c b/test/gen_bogus.c index ec29eb9..237b024 100644 --- a/test/gen_bogus.c +++ b/test/gen_bogus.c @@ -89,6 +89,14 @@ generate_datasets(hid_t loc_id, unsigned bogus_id) if((did = H5Dcreate2(loc_id, "Dataset4", H5T_NATIVE_INT, sid, H5P_DEFAULT, dcpl, H5P_DEFAULT)) < 0) goto error; if(H5Dclose(did) < 0) goto error; + /* Set "shareable" message flag for bogus message */ + bogus_flags = H5O_MSG_FLAG_SHAREABLE; + if(H5Pset(dcpl, H5O_BOGUS_MSG_FLAGS_NAME, &bogus_flags) < 0) goto error; + + /* Create fourth dataset, with "shareable" message flag */ + if((did = H5Dcreate2(loc_id, "Dataset5", H5T_NATIVE_INT, sid, H5P_DEFAULT, dcpl, H5P_DEFAULT)) < 0) goto error; + if(H5Dclose(did) < 0) goto error; + /* Close dataset creation property list */ if(H5Pclose(dcpl) < 0) goto error; diff --git a/test/ohdr.c b/test/ohdr.c index 25413cb..85554a5 100644 --- a/test/ohdr.c +++ b/test/ohdr.c @@ -521,6 +521,16 @@ test_unknown(unsigned bogus_id, char *filename, hid_t fapl) PASSED(); + TESTING("object with unknown header message & 'shareable' flag set"); + + /* Open the dataset with the unknown header message, adn "shareable" flag */ + if((did = H5Dopen2(loc_bogus, "Dataset5", H5P_DEFAULT)) < 0) + FAIL_STACK_ERROR + if(H5Dclose(did) < 0) + FAIL_STACK_ERROR + + PASSED(); + TESTING("object in r/o file with unknown header message & 'fail if unknown and open for write' flag set"); /* Open the dataset with the unknown header message, and "fail if unknown and open for write" flag */ diff --git a/test/tbogus.h5 b/test/tbogus.h5 index 91ab76f..efcbd79 100644 Binary files a/test/tbogus.h5 and b/test/tbogus.h5 differ -- cgit v0.12