Change SegmentTable semantics to respect unset entries

Accesses to unset entries is now clearly defined as returning a 0'd out value, the prior behavior would be to optimize sets for border segments to use L2 atomicity when the specific segment had no L1 entries set. This would lead to any future lookups of offsets within the same L2 segment but a different L1 entry to incorrectly return an inaccurate value as the only prior guarantee was that lookups after setting a segment would return the same value as was set but lacked the guarantee for unset segments to also consistently return unset values.

This could lead to issues in practical usages such as the `BufferManager` lookups returning the existence of a `Buffer` at a location falsely even though the segment was never set to the value, this was problematic as raw pointers were utilized and bound checks would lead to a segmentation fault.

This commit fixes this issue by introducing this guarantee and refactoring the class accordingly, it also deletes the `Set` method for setting a single entry as the meaning is ambiguous and it's functionality was more akin to the past guarantee and no longer makes sense.

Co-authored-by: PixelyIon <pixelyion@protonmail.com>
This commit is contained in:
Billy Laws 2022-08-06 14:37:37 +01:00 committed by PixelyIon
parent 36b8d3c445
commit 99b5fc35c6
No known key found for this signature in database
GPG Key ID: 11BC6C3201BC2C05

View File

@ -26,9 +26,8 @@ namespace skyline {
* @brief An entry in a segment table level aside from the lowest level which directly holds the type, this has an associated segment and a flag if the lookup should move to a higher granularity (and the corresponding lower level) * @brief An entry in a segment table level aside from the lowest level which directly holds the type, this has an associated segment and a flag if the lookup should move to a higher granularity (and the corresponding lower level)
*/ */
struct alignas(8) RangeEntry { struct alignas(8) RangeEntry {
bool valid; //!< If the associated segment is valid, the entry must not be accessed without checking validity first and an invalid entry implies going to the next level
SegmentType segment; //!< The segment associated with the entry, this is 0'd out if the entry is unset SegmentType segment; //!< The segment associated with the entry, this is 0'd out if the entry is unset
bool valid; //!< If the associated segment is valid, the entry must not be accessed without checking validity first
bool level1Set; //!< If to ignore this level and look in the next level table instead
}; };
static constexpr size_t L2Size{1 << L2Bits}, L2Entries{util::DivideCeil(Size, L2Size)}, L1inL2Count{L1Size / L2Size}; static constexpr size_t L2Size{1 << L2Bits}, L2Entries{util::DivideCeil(Size, L2Size)}, L1inL2Count{L1Size / L2Size};
@ -84,31 +83,6 @@ namespace skyline {
return level1Table[index >> L1Bits]; return level1Table[index >> L1Bits];
} }
/**
* @brief Sets a single segment at the specified index to the supplied value
*/
void Set(size_t index, SegmentType segment) {
auto &l2Entry{level2Table[index >> L2Bits]};
if (l2Entry.valid || l2Entry.level1Set) {
if (l2Entry.segment == segment)
return;
l2Entry.valid = false;
l2Entry.level1Set = true;
size_t l1L2Start{(index >> L2Bits) << (L2Bits - L1Bits)};
for (size_t i{l1L2Start}; i < l1L2Start + L1inL2Count; i++)
level1Table[i] = l2Entry.segment;
level1Table[index >> L1Bits] = segment;
} else if (l2Entry.level1Set) {
level1Table[index >> L1Bits] = segment;
} else {
l2Entry.segment = segment;
l2Entry.valid = true;
l2Entry.level1Set = true;
}
}
/** /**
* @brief Sets a segment of segments between the start and end to the supplied value * @brief Sets a segment of segments between the start and end to the supplied value
*/ */
@ -121,7 +95,6 @@ namespace skyline {
auto &l2Entry{level2Table[start >> L2Bits]}; auto &l2Entry{level2Table[start >> L2Bits]};
if (l2Entry.valid) { if (l2Entry.valid) {
l2Entry.valid = false; l2Entry.valid = false;
l2Entry.level1Set = true;
size_t l1L2Start{(start >> L2Bits) << (L2Bits - L1Bits)}; size_t l1L2Start{(start >> L2Bits) << (L2Bits - L1Bits)};
for (size_t i{l1L2Start}; i < l1StartPaddingStart; i++) for (size_t i{l1L2Start}; i < l1StartPaddingStart; i++)
@ -133,10 +106,6 @@ namespace skyline {
size_t l1L2End{l2AlignedAddress >> L1Bits}; size_t l1L2End{l2AlignedAddress >> L1Bits};
for (size_t i{l1StartPaddingEnd}; i < l1L2End; i++) for (size_t i{l1StartPaddingEnd}; i < l1L2End; i++)
level1Table[i] = l2Entry.segment; level1Table[i] = l2Entry.segment;
} else if (!l2Entry.level1Set) {
l2Entry.segment = segment;
l2Entry.valid = true;
l2Entry.level1Set = false;
} else { } else {
for (size_t i{l1StartPaddingStart}; i < l1StartPaddingEnd; i++) for (size_t i{l1StartPaddingStart}; i < l1StartPaddingEnd; i++)
level1Table[i] = segment; level1Table[i] = segment;
@ -152,7 +121,6 @@ namespace skyline {
auto &l2Entry{level2Table[i]}; auto &l2Entry{level2Table[i]};
l2Entry.segment = segment; l2Entry.segment = segment;
l2Entry.valid = true; l2Entry.valid = true;
l2Entry.level1Set = false;
} }
size_t l1EndPaddingStart{l2IndexEnd << (L2Bits - L1Bits)}; size_t l1EndPaddingStart{l2IndexEnd << (L2Bits - L1Bits)};
@ -161,17 +129,12 @@ namespace skyline {
auto &l2Entry{level2Table[l2IndexEnd]}; auto &l2Entry{level2Table[l2IndexEnd]};
if (l2Entry.valid) { if (l2Entry.valid) {
l2Entry.valid = false; l2Entry.valid = false;
l2Entry.level1Set = true;
for (size_t i{l1EndPaddingStart}; i < l1EndPaddingEnd; i++) for (size_t i{l1EndPaddingStart}; i < l1EndPaddingEnd; i++)
level1Table[i] = segment; level1Table[i] = segment;
for (size_t i{l1EndPaddingEnd}; i < l1EndPaddingStart + L1inL2Count; i++) for (size_t i{l1EndPaddingEnd}; i < l1EndPaddingStart + L1inL2Count; i++)
level1Table[i] = l2Entry.segment; level1Table[i] = l2Entry.segment;
} else if (!l2Entry.level1Set) {
l2Entry.segment = segment;
l2Entry.valid = true;
l2Entry.level1Set = false;
} else { } else {
for (size_t i{l1EndPaddingStart}; i < l1EndPaddingEnd; i++) for (size_t i{l1EndPaddingStart}; i < l1EndPaddingEnd; i++)
level1Table[i] = segment; level1Table[i] = segment;