diff --git a/src/jrd/btr.cpp b/src/jrd/btr.cpp index 43a8cc794a0..61fb180a37f 100644 --- a/src/jrd/btr.cpp +++ b/src/jrd/btr.cpp @@ -233,10 +233,14 @@ namespace if (traNumber && rpt->getTransaction() != traNumber) { - const auto backId = rpt->irt_backslot; - fb_assert(backId < root->irt_count); + const auto backSlot = root->getBackSlot(rpt); + if (!backSlot) + { + CCH_unwind(tdbb, false); + BUGCHECK(204); // msg 204 index inconsistent + } - return &root->irt_rpt[backId]; + return backSlot; } } @@ -3187,10 +3191,15 @@ static ModifyIrtRepeatValue modifyIrtRepeat(thread_db* tdbb, index_root_page::ir if (state != tra_committed && state != tra_dead) return ModifyIrtRepeatValue::Skip; - CCH_MARK(tdbb, window); const auto backId = irt_desc->irt_backslot; - fb_assert(backId < root->irt_count); - const auto backIrtDesc = &root->irt_rpt[backId]; + const auto backIrtDesc = root->getBackSlot(irt_desc); + if (!backIrtDesc) + { + CCH_RELEASE(tdbb, window); + BUGCHECK(204); // msg 204 index inconsistent + } + + CCH_MARK(tdbb, window); if (state == tra_committed) { diff --git a/src/jrd/ods.h b/src/jrd/ods.h index 76e0b4f16bf..41cfc61f77a 100644 --- a/src/jrd/ods.h +++ b/src/jrd/ods.h @@ -449,6 +449,12 @@ struct index_root_page void setEmpty(); } irt_rpt[1]; + // Return the backing slot referenced by a migrating index slot, + // or NULL when the slot is not in the irt_migrate state or the + // stored slot number points outside this page + const irt_repeat* getBackSlot(const irt_repeat* rpt) const; + irt_repeat* getBackSlot(const irt_repeat* rpt); + static_assert(sizeof(struct irt_repeat) == 24, "struct irt_repeat size mismatch"); static_assert(offsetof(struct irt_repeat, irt_transaction) == 0, "irt_transaction offset mismatch"); static_assert(offsetof(struct irt_repeat, irt_page_num) == 8, "irt_root offset mismatch"); @@ -775,6 +781,22 @@ inline void index_root_page::irt_repeat::setMigrate(const Jrd::PageNumber& rootP setState(irt_migrate); } +inline const index_root_page::irt_repeat* index_root_page::getBackSlot(const irt_repeat* rpt) const +{ + fb_assert(rpt->getState() == irt_migrate); + + if (rpt->getState() != irt_migrate || rpt->irt_backslot >= irt_count) + return nullptr; + + return &irt_rpt[rpt->irt_backslot]; +} + +inline index_root_page::irt_repeat* index_root_page::getBackSlot(const irt_repeat* rpt) +{ + return const_cast( + static_cast(this)->getBackSlot(rpt)); +} + inline constexpr int STUFF_COUNT = 4; inline constexpr ULONG END_LEVEL = ~0; diff --git a/src/jrd/tests/IndexRootPageTest.cpp b/src/jrd/tests/IndexRootPageTest.cpp new file mode 100644 index 00000000000..96172203355 --- /dev/null +++ b/src/jrd/tests/IndexRootPageTest.cpp @@ -0,0 +1,91 @@ +#include "firebird.h" +#include "boost/test/unit_test.hpp" +#include "../jrd/ods.h" + +using namespace Firebird; +using namespace Jrd; +using namespace Ods; + +BOOST_AUTO_TEST_SUITE(EngineSuite) +BOOST_AUTO_TEST_SUITE(IndexRootPageSuite) + + +BOOST_AUTO_TEST_SUITE(BackSlotTests) + +// Index root page image with room for 16 slots +union RootPageImage +{ + index_root_page page; + UCHAR bytes[sizeof(index_root_page) + 15 * sizeof(index_root_page::irt_repeat)]; +}; + +// Zero-filled image, so every test case starts from a clean page +static index_root_page* makeRootPage(unsigned count) +{ + static RootPageImage image; + + memset(&image, 0, sizeof(image)); + image.page.irt_relation = 1; + image.page.irt_count = count; + return &image.page; +} + +// Bring a slot into the irt_migrate state through the regular state +// transitions, ending with the given backing slot number +static index_root_page::irt_repeat* makeMigratedSlot(index_root_page::irt_repeat* slot, USHORT backSlot) +{ + slot->setInProgress(12); + slot->setInProgress(PageNumber(1, 77), 12); + slot->setKill(); + slot->setRollback(12); + slot->setMigrate(PageNumber(1, 99), 12, backSlot); + return slot; +} + +BOOST_AUTO_TEST_CASE(BackSlotInRange) +{ + auto root = makeRootPage(4); + + makeMigratedSlot(&root->irt_rpt[1], 2); + BOOST_TEST(root->getBackSlot(&root->irt_rpt[1]) == &root->irt_rpt[2]); + + root->irt_rpt[1].setMigrate(PageNumber(1, 99), 12, 3); // last valid slot + BOOST_TEST(root->getBackSlot(&root->irt_rpt[1]) == &root->irt_rpt[3]); +} + +BOOST_AUTO_TEST_CASE(BackSlotOutOfRange) +{ + auto root = makeRootPage(4); + + makeMigratedSlot(&root->irt_rpt[0], 4); // == irt_count + BOOST_TEST(root->getBackSlot(&root->irt_rpt[0]) == nullptr); + + root->irt_rpt[0].setMigrate(PageNumber(1, 99), 12, 0xFFFF); + BOOST_TEST(root->getBackSlot(&root->irt_rpt[0]) == nullptr); + + root->irt_count = 0; // no slot may resolve anymore + BOOST_TEST(root->getBackSlot(&root->irt_rpt[0]) == nullptr); +} + +BOOST_AUTO_TEST_CASE(BackSlotNotMigrating) +{ + auto root = makeRootPage(4); + + // slot in the regular working state: the backing slot number is + // meaningless and must not resolve even when in range + auto slot = &root->irt_rpt[1]; + slot->setInProgress(12); + slot->setInProgress(PageNumber(1, 77), 12); + slot->setKill(); + slot->setRollback(12); + slot->setNormal(); + slot->irt_backslot = 2; + + BOOST_TEST(root->getBackSlot(slot) == nullptr); +} + +BOOST_AUTO_TEST_SUITE_END() // BackSlotTests + + +BOOST_AUTO_TEST_SUITE_END() // IndexRootPageSuite +BOOST_AUTO_TEST_SUITE_END() // EngineSuite