From a56b9bde18a2baac1b4e9b8f31ba1d7ff8f4ca71 Mon Sep 17 00:00:00 2001 From: Zane Shelley Date: Tue, 31 Jan 2017 10:21:32 -0600 Subject: [PATCH] PRD: cleaned BitString::Pattern() This function had a off-by-one error that could access memory beyond the available memory space. Change-Id: I762d0e24f0cc7ecd42e7f9393b0dc5b3b8bddefc RTC: 167819 Reviewed-on: http://ralgit01.raleigh.ibm.com/gerrit1/35688 Tested-by: Jenkins Server Reviewed-by: Benjamin J. Weisenbeck Reviewed-by: Caleb N. Palmer Reviewed-by: Zane C. Shelley Reviewed-on: http://ralgit01.raleigh.ibm.com/gerrit1/36201 Tested-by: FSP CI Jenkins Tested-by: Jenkins OP Build CI --- .../prdf/common/framework/register/iipscr.C | 4 +- .../register/prdfErrorRegisterMask.C | 6 +- .../register/prdfHomRegisterAccess.C | 4 +- .../framework/register/prdfOperatorRegister.H | 4 +- .../register/prdfResetErrorRegister.C | 4 +- .../framework/register/prdfResetOperators.H | 6 +- .../prdf/common/plat/p9/prdfP9ProcDomain.C | 6 +- src/usr/diag/prdf/common/util/prdfBitKey.C | 8 +- src/usr/diag/prdf/common/util/prdfBitString.C | 98 +++++-------- src/usr/diag/prdf/common/util/prdfBitString.H | 134 ++++++++---------- .../prdf/test/prdfTest_ScomAccessInterface.H | 4 +- 11 files changed, 111 insertions(+), 167 deletions(-) diff --git a/src/usr/diag/prdf/common/framework/register/iipscr.C b/src/usr/diag/prdf/common/framework/register/iipscr.C index fac4cb854f5..127992be5a0 100755 --- a/src/usr/diag/prdf/common/framework/register/iipscr.C +++ b/src/usr/diag/prdf/common/framework/register/iipscr.C @@ -197,13 +197,13 @@ void SCAN_COMM_REGISTER_CLASS::ClearBit void SCAN_COMM_REGISTER_CLASS::clearAllBits() { BIT_STRING_CLASS & bitString = AccessBitString(); - bitString.Pattern( 0, bitString.getBitLen(), 0x00000000, 32 ); + bitString.clearAll(); } void SCAN_COMM_REGISTER_CLASS::setAllBits() { BIT_STRING_CLASS & bitString = AccessBitString(); - bitString.Pattern( 0, bitString.getBitLen(), 0xffffffff, 32 ); + bitString.setAll(); } //------------------------------------------------------------------------------ diff --git a/src/usr/diag/prdf/common/framework/register/prdfErrorRegisterMask.C b/src/usr/diag/prdf/common/framework/register/prdfErrorRegisterMask.C index f237ccdce52..64aa1000930 100755 --- a/src/usr/diag/prdf/common/framework/register/prdfErrorRegisterMask.C +++ b/src/usr/diag/prdf/common/framework/register/prdfErrorRegisterMask.C @@ -5,7 +5,7 @@ /* */ /* OpenPOWER HostBoot Project */ /* */ -/* Contributors Listed Below - COPYRIGHT 2012,2014 */ +/* Contributors Listed Below - COPYRIGHT 2012,2017 */ /* [+] International Business Machines Corp. */ /* */ /* */ @@ -84,7 +84,7 @@ bitString(r.GetBitLength()), bitStringMask(r.GetBitLength()), xMaskScr(maskScr) { - bitStringMask.Pattern(0); + bitStringMask.clearAll(); } ErrorRegisterMask::ErrorRegisterMask @@ -100,7 +100,7 @@ bitString(r.GetBitLength()), bitStringMask(r.GetBitLength()), xMaskScr(maskScr) { - bitStringMask.Pattern(0); // clear software mask + bitStringMask.clearAll(); // clear software mask } // ********************************************************************** diff --git a/src/usr/diag/prdf/common/framework/register/prdfHomRegisterAccess.C b/src/usr/diag/prdf/common/framework/register/prdfHomRegisterAccess.C index d5627bb6acb..c022cdec825 100755 --- a/src/usr/diag/prdf/common/framework/register/prdfHomRegisterAccess.C +++ b/src/usr/diag/prdf/common/framework/register/prdfHomRegisterAccess.C @@ -5,7 +5,7 @@ /* */ /* OpenPOWER HostBoot Project */ /* */ -/* Contributors Listed Below - COPYRIGHT 2012,2016 */ +/* Contributors Listed Below - COPYRIGHT 2012,2017 */ /* [+] International Business Machines Corp. */ /* */ /* */ @@ -200,7 +200,7 @@ uint32_t ScomAccessor::Access(TargetHandle_t i_target, } case MopRegisterAccess::READ: - bs.Pattern(0x00000000); // clear all bits + bs.clearAll(); // clear all bits rc = PRDF::PlatServices::getScom(i_target, bs, registerId); diff --git a/src/usr/diag/prdf/common/framework/register/prdfOperatorRegister.H b/src/usr/diag/prdf/common/framework/register/prdfOperatorRegister.H index 01780e37e46..093205985e3 100755 --- a/src/usr/diag/prdf/common/framework/register/prdfOperatorRegister.H +++ b/src/usr/diag/prdf/common/framework/register/prdfOperatorRegister.H @@ -5,7 +5,7 @@ /* */ /* OpenPOWER HostBoot Project */ /* */ -/* Contributors Listed Below - COPYRIGHT 2012,2016 */ +/* Contributors Listed Below - COPYRIGHT 2012,2017 */ /* [+] International Business Machines Corp. */ /* */ /* */ @@ -122,7 +122,7 @@ class SummaryRegister : public SCAN_COMM_REGISTER_CLASS const BIT_STRING_CLASS * GetBitString( ATTENTION_TYPE i_type = INVALID_ATTENTION_TYPE) const { - iv_bs->Clear(); + iv_bs->clearAll(); PRDF::BitString tmp = *iv_child->GetBitString(i_type); diff --git a/src/usr/diag/prdf/common/framework/register/prdfResetErrorRegister.C b/src/usr/diag/prdf/common/framework/register/prdfResetErrorRegister.C index 24064490aab..2c50bdda8b8 100755 --- a/src/usr/diag/prdf/common/framework/register/prdfResetErrorRegister.C +++ b/src/usr/diag/prdf/common/framework/register/prdfResetErrorRegister.C @@ -5,7 +5,7 @@ /* */ /* OpenPOWER HostBoot Project */ /* */ -/* Contributors Listed Below - COPYRIGHT 2012,2015 */ +/* Contributors Listed Below - COPYRIGHT 2012,2017 */ /* [+] International Business Machines Corp. */ /* */ /* */ @@ -190,7 +190,7 @@ int32_t AndResetErrorRegister::Reset(const BitKey & bit_list, if(bl_length !=0) { BIT_STRING_BUFFER_CLASS bs(xAndResetScr.GetBitLength()); - bs.Pattern(0xffffffff,32); // set to all ones + bs.setAll(); // set to all ones uint32_t i; for(i = 0; i < bl_length; ++i) // Turn off all bits used to isolate problem { diff --git a/src/usr/diag/prdf/common/framework/register/prdfResetOperators.H b/src/usr/diag/prdf/common/framework/register/prdfResetOperators.H index 9cf18750b69..d31ad955692 100755 --- a/src/usr/diag/prdf/common/framework/register/prdfResetOperators.H +++ b/src/usr/diag/prdf/common/framework/register/prdfResetOperators.H @@ -5,7 +5,7 @@ /* */ /* OpenPOWER HostBoot Project */ /* */ -/* Contributors Listed Below - COPYRIGHT 2012,2015 */ +/* Contributors Listed Below - COPYRIGHT 2012,2017 */ /* [+] International Business Machines Corp. */ /* */ /* */ @@ -188,7 +188,7 @@ class AndOperator : public RegisterResetOperator if(bl_length != 0) // Check for bits to reset { BIT_STRING_BUFFER_CLASS bs(writeReg->GetBitLength()); - bs.Pattern(0xffffffff, 32); // set all to 1's. + bs.setAll(); // set all to 1's. uint32_t i; for(i = 0; i < bl_length; ++i) // Turn off all bits specified @@ -309,7 +309,7 @@ class NotOperator : public RegisterResetOperator else // RESETOPERATOR_MASK { BIT_STRING_BUFFER_CLASS bs(writeReg->GetBitLength()); - bs.Pattern(0xffffffff, 32); // set all to 1's. + bs.setAll(); // set all to 1's. writeReg->SetBitString(&bs); // Copy bit-string to register. rc = writeReg->Write(); // Write hardware } diff --git a/src/usr/diag/prdf/common/plat/p9/prdfP9ProcDomain.C b/src/usr/diag/prdf/common/plat/p9/prdfP9ProcDomain.C index 23ea0fade74..1b4be25914f 100644 --- a/src/usr/diag/prdf/common/plat/p9/prdfP9ProcDomain.C +++ b/src/usr/diag/prdf/common/plat/p9/prdfP9ProcDomain.C @@ -214,17 +214,17 @@ void ProcDomain::SortForXstop() // Get initial list (all chips). BIT_STRING_BUFFER_CLASS l_current(GetSize()); - l_current.Pattern(0,GetSize(),0xFFFFFFFF, 32); // turn on all bits. + l_current.setAll(); // turn on all bits. // Do reduction. // When done, l_prev will have the minimal list. BIT_STRING_BUFFER_CLASS l_prev(GetSize()); - l_prev.Clear(); + l_prev.clearAll(); while ((!(l_current == l_prev)) && (!l_current.IsZero())) { l_prev = l_current; - l_current.Clear(); + l_current.clearAll(); for (uint32_t i = 0; i < GetSize(); i++) { diff --git a/src/usr/diag/prdf/common/util/prdfBitKey.C b/src/usr/diag/prdf/common/util/prdfBitKey.C index f7d21110377..c8fdbedd72d 100755 --- a/src/usr/diag/prdf/common/util/prdfBitKey.C +++ b/src/usr/diag/prdf/common/util/prdfBitKey.C @@ -137,7 +137,7 @@ BitKey & BitKey::operator=(const BitKey & bit_list) if(iv_Capacity) { BitString bs(iv_Capacity,DataPtr()); - bs.Pattern(0x00000000); + bs.clearAll(); } ReAllocate(bit_list.iv_Capacity); if(IsDirect()) // implies bit_list is also direct @@ -167,7 +167,7 @@ BitKey & BitKey::operator=(const BitString & bit_string) if(iv_Capacity) { BitString bs(iv_Capacity,DataPtr()); - bs.Pattern(0x00000000); + bs.clearAll(); } ReAllocate(bit_string.getBitLen()); BitString dbs(iv_Capacity,DataPtr()); @@ -182,7 +182,7 @@ BitKey & BitKey::operator=(const char * string_ptr) if(iv_Capacity) { BitString bs(iv_Capacity,DataPtr()); - bs.Pattern(0x00000000); + bs.clearAll(); } while(*string_ptr != '\0') @@ -360,7 +360,7 @@ void BitKey::ReAllocate(uint32_t i_len) { uint32_t * newBuffer = new uint32_t[wordsize]; BitString dbs(iv_Capacity,newBuffer); - dbs.Pattern(0x00000000); + dbs.clearAll(); BitString sbs(oldSize,oldPtr); dbs.SetBits(sbs); iv_storage1 = 0; diff --git a/src/usr/diag/prdf/common/util/prdfBitString.C b/src/usr/diag/prdf/common/util/prdfBitString.C index fad644a0879..16c4b97f803 100755 --- a/src/usr/diag/prdf/common/util/prdfBitString.C +++ b/src/usr/diag/prdf/common/util/prdfBitString.C @@ -42,6 +42,35 @@ namespace PRDF // BitString class //############################################################################## +void BitString::setPattern( uint32_t i_sPos, uint32_t i_sLen, + CPU_WORD i_pattern, uint32_t i_pLen ) +{ + PRDF_ASSERT(nullptr != getBufAddr()); // must to have a valid address + PRDF_ASSERT(0 < i_sLen); // must have at least one bit + PRDF_ASSERT(i_sPos + i_sLen <= getBitLen()); // field must be within range + PRDF_ASSERT(0 < i_pLen); // must have at least one bit + PRDF_ASSERT(i_pLen <= CPU_WORD_BIT_LEN); // i_pLen length must be valid + + // Get a bit string for the pattern subset (right justified). + BitStringOffset bso ( CPU_WORD_BIT_LEN - i_pLen, i_pLen, &i_pattern ); + + // Iterate the range in chunks the size of i_pLen. + uint32_t endPos = i_sPos + i_sLen; + for ( uint32_t pos = i_sPos; pos < endPos; pos += i_pLen ) + { + // The true chunk size is either i_pLen or the leftovers at the end. + uint32_t len = std::min( i_pLen, endPos - pos ); + + // Get this chunk's pattern value, truncate (left justified) if needed. + CPU_WORD pattern = bso.GetField( 0, len ); + + // Set the pattern in this string. + SetField( pos, len, pattern ); + } +} + +//------------------------------------------------------------------------------ + uint32_t BitString::GetSetCount(uint32_t bit_position, uint32_t leng ) const @@ -221,69 +250,6 @@ void BitString::SetBits } } -// ------------------------------------------------------------------------------------------------ - -// Function Specification ////////////////////////////////////////// -// -// Title: Pattern -// -// Purpose: This function sets the the specified bits with the -// specifed pattern. The number of bits sets is -// specified by the length and begins at the specified -// offest. The pattern is repeated as often as necessary -// as specified by the pattern_bit_length. -// -// Side-effects: Bit String may be modified. -// -// Dependencies: Parameters must specifiy valid bits in both the -// bit string and the pattern. -// -// Time Complexity: O(m) where m is the number of bits to modify -// (parameter l) -// -// Examples: o(0), l(10), pattern(0xA), pattern_bit_length(4) -// Old String: 0000000000 -// New String: 1010101010 -// -// o(3), l(4), pattern(0x3), pattern_bit_length(3) -// Old String: 0001001000 -// New String: 0000110000 -// -// End Function Specification ////////////////////////////////////// - -void BitString::Pattern -( - uint32_t o, - uint32_t l, - CPU_WORD pattern, - uint32_t pattern_bit_length - ) -{ - PRDF_ASSERT(((o + l) <= iv_bitLen) && - (pattern_bit_length <= CPU_WORD_BIT_LEN)); - - uint32_t current_offset; - - // current_offset = offset + o; - current_offset = o; - while(true) - { - if(l > pattern_bit_length) - { - /* Set values using full CPU_WORDs */ - SetField(current_offset, pattern_bit_length, pattern); - l -= pattern_bit_length; - current_offset += pattern_bit_length; - } - else - { - /* Set value in remainder of last CPU_WORD */ - SetField(current_offset, l, pattern); - break; - } - } -} - // Function Specification ////////////////////////////////////////// // // Title: Is Set @@ -588,7 +554,7 @@ BitStringBuffer BitString::operator>>(uint32_t count) const { BitStringBuffer l_bsb(this->getBitLen()); BitString * l_bsbp = &l_bsb; // dg03a - stupid trick to get to GetRelativePosition() - // l_bsb.Clear(); + // l_bsb.clearAll(); if(count < this->getBitLen()) { //bso overlays bsb at offset = count @@ -605,7 +571,7 @@ BitStringBuffer BitString::operator>>(uint32_t count) const BitStringBuffer BitString::operator<<(uint32_t count) const { BitStringBuffer l_bsb(this->getBitLen()); - // l_bsb.Clear(); + // l_bsb.clearAll(); if(count < this->getBitLen()) { // bso overlays *this at offset = count @@ -686,7 +652,7 @@ void BitStringBuffer::initBuffer() setBufAddr( new CPU_WORD[ getNumCpuWords(getBitLen()) ] ); // Clear the new buffer. - Clear(); + if ( !IsZero() ) clearAll(); } /*--------------------------------------------------------------------*/ diff --git a/src/usr/diag/prdf/common/util/prdfBitString.H b/src/usr/diag/prdf/common/util/prdfBitString.H index dbc6c224222..2fe911b47d8 100755 --- a/src/usr/diag/prdf/common/util/prdfBitString.H +++ b/src/usr/diag/prdf/common/util/prdfBitString.H @@ -138,6 +138,62 @@ class BitString return (i_bitLen + i_offset + CPU_WORD_BIT_LEN-1) / CPU_WORD_BIT_LEN; } + /** @brief Sets the entire bit string to 1's. */ + void setAll() { setPattern(CPU_WORD_MASK); } + + /** @brief Sets the entire bit string to 0's. */ + void clearAll() { setPattern(0); } + + /** + * @brief Sets a range within the string based on the pattern and length + * provided. + * @param i_sPos Starting position of this string. + * @param i_sLen The length of the target range. + * @param i_pattern The pattern to set (right justified). + * @param i_pLen The length of the pattern. + * @pre nullptr != getBufAddr() + * @pre 0 < i_sLen + * @pre i_sPos + i_sLen <= getBitLen() + * @pre 0 < i_pLen <= CPU_WORD_BIT_LEN + * @post The pattern is repeated/truncated as needed. + * + * Examples: i_sPos(0), i_sLen(10), i_pattern(0xA), i_pLen(4) + * Old String: 0000000000 + * New String: 1010101010 + * + * i_sPos(3), i_sLen(4), i_pattern(0x3), i_pLen(3) + * Old String: 0001001000 + * New String: 0000110000 + */ + void setPattern( uint32_t i_sPos, uint32_t i_sLen, + CPU_WORD i_pattern, uint32_t i_pLen ); + + /** + * @brief Sets entire string based on the pattern and length provided. + * @param i_pattern The pattern to set (right justified). + * @param i_pLen The length of the pattern. + * @note See definition above for prerequisites. + * @post The entire string is filled with the pattern. + * @post The pattern is repeated/truncated as needed. + */ + void setPattern( CPU_WORD i_pattern, uint32_t i_pLen ) + { + setPattern( 0, getBitLen(), i_pattern, i_pLen ); + } + + /** + * @brief Sets entire string based on the pattern provided (length of + * CPU_WORD). + * @param i_pattern The pattern to set. + * @note See definition above for prerequisites. + * @post The entire string is filled with the pattern. + * @post The pattern is repeated/truncated as needed. + */ + void setPattern( CPU_WORD i_pattern ) + { + setPattern( i_pattern, CPU_WORD_BIT_LEN ); + } + /*! Comparison \remarks The bitstrings must be the same length and have the same bits set to be equal @@ -236,45 +292,6 @@ class BitString unsigned int len, unsigned int dpos = 0); - /*! - Set bits in this string based on the range and pattern provided - \param iPos: bit pos in this string to start - \param iLen: # of bits to modify - \param iPattern: Pattern to set - \param iPatternLen: # of bit in pattern to use (right justfied) - \pre (iPos + iLen) <= getBitLen() - \post Range of specified bits filled with pattern. The pattern is repeated as needed - \verbatim - Examples: iPos(0), iLen(10), iPattern(0xA), iPatternLen(4) - Old String: 0000000000 - New String: 1010101010 - - iPos(3), iLen(4), iPattern(0x3), iPatternLen(3) - Old String: 0001001000 - New String: 0000110000 - \endverbatim - */ - void Pattern(uint32_t iPos, - uint32_t iLen, - CPU_WORD iPattern, - uint32_t pattern_bit_length); - - /*! - Set entire string based on the pattern provided - \param iPattern: Pattern to set - \param iPatternLen: # of bit in pattern to use (right justfied) - \post BitString is filled with pattern. The pattern is repeated/truncated as needed - */ - void Pattern(CPU_WORD iPattern, - uint32_t iPatternLen); - - /*! - Set entire string based on the pattern provided - \param iPattern: Pattern to set - \post BitString is filled with pattern. The pattern is repeated/truncated as needed - */ - void Pattern(CPU_WORD pattern); - /*! Query if bit is set (1) \returns [true|false] @@ -296,12 +313,6 @@ class BitString */ void Clear(uint32_t bit_position); - /*! - Clear the entire bit string - \post IsZero() == true - */ - void Clear(void) { Pattern(0); } - /*! Test equivalence \returns [true | false] @@ -625,39 +636,6 @@ BitString & BitString::operator= return(*this); } -// Function Specification ////////////////////////////////////////// -// -// Title: Pattern -// -// Purpose: This function sets this entire Bit String with the -// specifed pattern. The pattern is repeated as often as -// necessary as specified by the pattern_bit_length. -// -// Side-effects: Bit String is be modified. -// -// Dependencies: None. -// -// Time Complexity: O(m) where m is the number of bits to modify -// (parameter l) -// -// Examples: See Pattern(uint32_t, uint32_t, CPU_WORD, uint32_t) -// -// End Function Specification ////////////////////////////////////// - -inline -void BitString::Pattern(CPU_WORD pattern,uint32_t pattern_bit_length) -{ - Pattern(0, getBitLen(), pattern, pattern_bit_length); -} - - -inline -void BitString::Pattern(CPU_WORD pattern) -{ - Pattern(0, getBitLen(), pattern, CPU_WORD_SIZE); -} - - inline uint32_t BitString::GetSetCount(void) const { return(GetSetCount(0, getBitLen())); diff --git a/src/usr/diag/prdf/test/prdfTest_ScomAccessInterface.H b/src/usr/diag/prdf/test/prdfTest_ScomAccessInterface.H index 492e5e37662..b5c73081c83 100644 --- a/src/usr/diag/prdf/test/prdfTest_ScomAccessInterface.H +++ b/src/usr/diag/prdf/test/prdfTest_ScomAccessInterface.H @@ -5,7 +5,7 @@ /* */ /* OpenPOWER HostBoot Project */ /* */ -/* Contributors Listed Below - COPYRIGHT 2016 */ +/* Contributors Listed Below - COPYRIGHT 2016,2017 */ /* [+] International Business Machines Corp. */ /* */ /* */ @@ -99,7 +99,7 @@ public: TS_FAIL("Unexpected error calling PRDF::PlatServices::putScom"); break; } - bs.Clear(); + bs.clearAll(); rc = PRDF::PlatServices::getScom(target, bs, address);