From: Date: November 11 2008 12:41pm Subject: bzr commit into mysql-5.1 branch (frazer:2733) Bug#39645 List-Archive: http://lists.mysql.com/commits/58442 X-Bug: 39645 Message-Id: <200811111141.mABBf0iC009283@forth.ndb.mysql.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit #At file:///home/frazer/bzr/mysql-5.1-telco-6.2/ 2733 Frazer Clement 2008-11-11 Bug# 39645 Losing size info for VARCHAR/CHAR for disk-data could segfault API apps VAR* types are stored fixed-size on disk. This var->fixed mapping occurs at the NDBAPI level, and some evidence of it can leak out to NDBAPI applications. This patch modifies the implementation of a number of NDBAPI methods so that disk-based VAR* types display the same behaviour as memory based VAR* types. From mysql-5.1-telco-6.4, the handling of VAR* types on disk is changed by WL4499, and this patch becomes unnecessary. HugoTest table D2 is augmented with some VARCHAR and LONGVARCHAR columns. modified: storage/ndb/src/ndbapi/NdbDictionaryImpl.hpp storage/ndb/src/ndbapi/NdbInterpretedCode.cpp storage/ndb/src/ndbapi/NdbOperationDefine.cpp storage/ndb/src/ndbapi/NdbOperationExec.cpp storage/ndb/src/ndbapi/NdbOperationInt.cpp storage/ndb/src/ndbapi/NdbRecAttr.cpp storage/ndb/test/src/NDBT_Tables.cpp === modified file 'storage/ndb/src/ndbapi/NdbDictionaryImpl.hpp' --- a/storage/ndb/src/ndbapi/NdbDictionaryImpl.hpp 2008-05-16 13:08:36 +0000 +++ b/storage/ndb/src/ndbapi/NdbDictionaryImpl.hpp 2008-11-11 11:40:42 +0000 @@ -143,6 +143,11 @@ public: // Get total length in bytes, used by NdbOperation bool get_var_length(const void* value, Uint32& len) const; + + // Get significant and send length in bytes, accounting for bug39645 + bool get_var_length_bug39645(const void* value, + Uint32& sigLen, + Uint32& sendLen) const; }; class NdbTableImpl : public NdbDictionary::Table, public NdbDictObjectImpl { @@ -866,6 +871,7 @@ NdbColumnImpl::get_var_length(const void { DBUG_ENTER("NdbColumnImpl::get_var_length"); Uint32 max_len = m_attrSize * m_arraySize; + switch (m_arrayType) { case NDB_ARRAYTYPE_SHORT_VAR: len = 1 + *((Uint8*)value); @@ -883,6 +889,70 @@ NdbColumnImpl::get_var_length(const void DBUG_RETURN(len <= max_len); } +// Useful until 6.4 where WL4499 removes the need. +inline +bool +NdbColumnImpl::get_var_length_bug39645(const void* value, + Uint32& sigLen, + Uint32& sendLen) const +{ + DBUG_ENTER("NdbColumnImpl::get_var_length_bug39645"); + Uint32 max_len = m_attrSize * m_arraySize; + + sendLen= max_len; + + /* Special code for bug 39645 to help avoid copying too much + * from user space (and potentially reading bad memory + * Not needed when true VARCHAR to disk is implemented (WL4499) + */ + if (unlikely(m_storageType == NDB_STORAGETYPE_DISK)) + { + /* Disk VARCHAR/BINARY have arrayType of NDB_ARRAYTYPE_FIXED, + * but we should treat as VAR types w.r.t. determining + * actual data length + */ + DBUG_PRINT("info", ("DISK based : type=%u", m_type)); + + switch (m_type) { + case NDB_TYPE_VARCHAR: // Fall through + case NDB_TYPE_VARBINARY: + sigLen = 1 + *((Uint8*)value); + DBUG_PRINT("info", ("SHORT_VAR_DISK: sigLen=%u max_len=%u", sigLen, max_len)); + DBUG_RETURN(sigLen <= max_len); + + case NDB_TYPE_LONGVARCHAR: // Fall through + case NDB_TYPE_LONGVARBINARY: + sigLen = 2 + uint2korr((char*)value); + DBUG_PRINT("info", ("MEDIUM_VAR: sigLen=%u max_len=%u", sigLen, max_len)); + DBUG_RETURN(sigLen <= max_len); + default: + // Fixed size disk type - fall through + // to normal processing below + ; + } + } + + /* Normal path */ + switch (m_arrayType) { + case NDB_ARRAYTYPE_SHORT_VAR: + sigLen = 1 + *((Uint8*)value); + DBUG_PRINT("info", ("SHORT_VAR: sigLen=%u max_len=%u", sigLen, max_len)); + break; + case NDB_ARRAYTYPE_MEDIUM_VAR: + sigLen = 2 + uint2korr((char*)value); + DBUG_PRINT("info", ("MEDIUM_VAR: sigLen=%u max_len=%u", sigLen, max_len)); + break; + default: + sigLen = max_len; + DBUG_PRINT("info", ("FIXED: sigLen=%u max_len=%u", sigLen, max_len)); + DBUG_RETURN(true); + } + + sendLen= sigLen; + DBUG_RETURN(sigLen <= max_len); +} + + inline NdbTableImpl & NdbTableImpl::getImpl(NdbDictionary::Table & t){ === modified file 'storage/ndb/src/ndbapi/NdbInterpretedCode.cpp' --- a/storage/ndb/src/ndbapi/NdbInterpretedCode.cpp 2008-11-06 16:35:05 +0000 +++ b/storage/ndb/src/ndbapi/NdbInterpretedCode.cpp 2008-11-11 11:40:42 +0000 @@ -508,13 +508,16 @@ NdbInterpretedCode::branch_col(Uint32 br DBUG_RETURN(error(BadAttributeId)); } + Uint32 sigLen= len; + Uint32 sendLen= len; + if (val == NULL) - len = 0; + sigLen = sendLen = 0; else { if (! col->getStringType()) { /* Fixed size type */ - len= col->m_attrSize * col->m_arraySize; + sigLen= sendLen= col->m_attrSize * col->m_arraySize; } else { @@ -525,7 +528,7 @@ NdbInterpretedCode::branch_col(Uint32 br if ((branch_type != Interpreter::LIKE) && (branch_type != Interpreter::NOT_LIKE)) { - if (! col->get_var_length(val, len)) + if (! col->get_var_length_bug39645(val, sigLen, sendLen)) { DBUG_RETURN(error(BadLength)); } @@ -536,15 +539,24 @@ NdbInterpretedCode::branch_col(Uint32 br if (col->m_storageType == NDB_STORAGETYPE_DISK) m_flags|= UsesDisk; + Uint32 tempData[ NDB_MAX_TUPLE_SIZE_IN_WORDS ]; + + if (sigLen != sendLen) + { + // bug39645 + memcpy(tempData, val, sigLen); + val = tempData; + } + if (add_branch(Interpreter::BranchCol(c, 0, 0, false), Label) != 0) DBUG_RETURN(-1); - if (add1(Interpreter::BranchCol_2(attrId, len)) != 0) + if (add1(Interpreter::BranchCol_2(attrId, sendLen)) != 0) DBUG_RETURN(-1); /* Get value byte length rounded up to nearest 32-bit word */ - Uint32 len2 = Interpreter::mod4(len); - if(len2 == len){ + Uint32 len2 = Interpreter::mod4(sendLen); + if(len2 == sendLen){ /* Whole number of 32-bit words */ DBUG_RETURN(addN((Uint32*)val, len2 >> 2)); } else { @@ -555,7 +567,7 @@ NdbInterpretedCode::branch_col(Uint32 br /* Zero insignificant bytes in last word */ Uint32 tmp = 0; - for (Uint32 i = 0; i < len-len2; i++) { + for (Uint32 i = 0; i < sendLen-len2; i++) { char* p = (char*)&tmp; p[i] = ((char*)val)[len2+i]; } === modified file 'storage/ndb/src/ndbapi/NdbOperationDefine.cpp' --- a/storage/ndb/src/ndbapi/NdbOperationDefine.cpp 2008-11-08 21:06:51 +0000 +++ b/storage/ndb/src/ndbapi/NdbOperationDefine.cpp 2008-11-11 11:40:42 +0000 @@ -577,20 +577,24 @@ NdbOperation::setValue( const NdbColumnI }//if }//if - Uint32 len; - if (! tAttrInfo->get_var_length(aValue, len)) { + Uint32 sigLen = 0; // Length of significant part + Uint32 sendLen = 0; // Length of data to send. Normally sigLen == sendLen + + if (! tAttrInfo->get_var_length_bug39645(aValue, sigLen, sendLen)) { setErrorCodeAbort(4209); DBUG_RETURN(-1); } - - const Uint32 sizeInBytes = len; + + const Uint32 sizeInBytes = sendLen; const Uint32 bitsInLastWord = 8 * (sizeInBytes & 3) ; const int attributeSize = sizeInBytes; const int slack = sizeInBytes & 3; - if (((UintPtr)aValue & 3) != 0 || (slack != 0)){ - memcpy(&tempData[0], aValue, attributeSize); + if (((UintPtr)aValue & 3) != 0 || // Alignment + (slack != 0) || // Slack bits + (sigLen != sendLen)){ // Bug39645 + memcpy(&tempData[0], aValue, sigLen); aValue = (char*)&tempData[0]; if(slack != 0) { char * tmp = (char*)&tempData[0]; === modified file 'storage/ndb/src/ndbapi/NdbOperationExec.cpp' --- a/storage/ndb/src/ndbapi/NdbOperationExec.cpp 2008-08-04 15:49:01 +0000 +++ b/storage/ndb/src/ndbapi/NdbOperationExec.cpp 2008-11-11 11:40:42 +0000 @@ -951,40 +951,42 @@ NdbOperation::buildSignalsNdbRecord(Uint if (extraCol->getStorageType( )== NDB_STORAGETYPE_DISK) no_disk_flag=0; - Uint32 length; + Uint32 sigLen = 0; + Uint32 sendLen = 0; if (pvalue==NULL) - length=0; + sigLen= sendLen= 0; else { - length=extraCol->getSizeInBytes(); - if (extraCol->getArrayType() != NdbDictionary::Column::ArrayTypeFixed) + if (! NdbColumnImpl::getImpl(*extraCol).get_var_length_bug39645(pvalue, + sigLen, + sendLen)) { - Uint32 lengthInfoBytes; - if (!NdbSqlUtil::get_var_length((Uint32) extraCol->getType(), - pvalue, - length, - lengthInfoBytes, - length)) - { - // Length parameter in equal/setValue is incorrect - setErrorCodeAbort(4209); - return -1; - } + // Length parameter in equal/setValue is incorrect + setErrorCodeAbort(4209); + return -1; } } + Uint32 tempData[ NDB_MAX_TUPLE_SIZE_IN_WORDS ]; + + if (sigLen != sendLen) // Bug 39645 + { + memcpy(tempData, pvalue, sigLen); + pvalue= tempData; + } + // Add ATTRINFO res= insertATTRINFOHdr_NdbRecord(aTC_ConnectPtr, aTransId, - extraCol->getAttrId(), length, + extraCol->getAttrId(), sendLen, &attrInfoPtr, &remain); if(res) return res; - if(length>0) + if(sendLen>0) { res=insertATTRINFOData_NdbRecord(aTC_ConnectPtr, aTransId, - (char*)pvalue, length, + (char*)pvalue, sendLen, &attrInfoPtr, &remain); if(res) return res; === modified file 'storage/ndb/src/ndbapi/NdbOperationInt.cpp' --- a/storage/ndb/src/ndbapi/NdbOperationInt.cpp 2008-06-03 10:00:31 +0000 +++ b/storage/ndb/src/ndbapi/NdbOperationInt.cpp 2008-11-11 11:40:42 +0000 @@ -1087,13 +1087,16 @@ NdbOperation::branch_col(Uint32 type, abort(); } + Uint32 sigLen= len; + Uint32 sendLen= len; + if (val == NULL) - len = 0; + sigLen= sendLen = 0; else { if (! col->getStringType()) { /* Fixed size type */ - len= col->m_attrSize * col->m_arraySize; + sigLen= sendLen= col->m_attrSize * col->m_arraySize; } else { @@ -1104,7 +1107,7 @@ NdbOperation::branch_col(Uint32 type, if ((type != Interpreter::LIKE) && (type != Interpreter::NOT_LIKE)) { - if (! col->get_var_length(val, len)) + if (! col->get_var_length_bug39645(val, sigLen, sendLen)) { setErrorCodeAbort(4209); DBUG_RETURN(-1); @@ -1115,9 +1118,15 @@ NdbOperation::branch_col(Uint32 type, m_no_disk_flag &= (col->m_storageType == NDB_STORAGETYPE_DISK ? 0:1); + /* We copy the data if it's not 32-bit aligned, or + * if we need to send more data than the user provides (Bug 39645) + */ + bool needCopy= ( (((UintPtr)val & 3) != 0) || // Not aligned + (sigLen != sendLen)); // Bug 39645 + Uint32 tempData[ NDB_MAX_TUPLE_SIZE_IN_WORDS ]; - if (((UintPtr)val & 3) != 0) { - memcpy(tempData, val, len); + if (needCopy) { + memcpy(tempData, val, sigLen); val = tempData; } @@ -1127,17 +1136,17 @@ NdbOperation::branch_col(Uint32 type, if (insertBranch(Label) == -1) DBUG_RETURN(-1); - if (insertATTRINFO(Interpreter::BranchCol_2(col->m_attrId, len))) + if (insertATTRINFO(Interpreter::BranchCol_2(col->m_attrId, sendLen))) DBUG_RETURN(-1); - Uint32 len2 = Interpreter::mod4(len); - if(len2 == len){ + Uint32 len2 = Interpreter::mod4(sendLen); + if(len2 == sendLen){ insertATTRINFOloop((Uint32*)val, len2 >> 2); } else { len2 -= 4; insertATTRINFOloop((Uint32*)val, len2 >> 2); Uint32 tmp = 0; - for (Uint32 i = 0; i < len-len2; i++) { + for (Uint32 i = 0; i < sendLen-len2; i++) { char* p = (char*)&tmp; p[i] = ((char*)val)[len2+i]; } === modified file 'storage/ndb/src/ndbapi/NdbRecAttr.cpp' --- a/storage/ndb/src/ndbapi/NdbRecAttr.cpp 2008-02-19 15:00:29 +0000 +++ b/storage/ndb/src/ndbapi/NdbRecAttr.cpp 2008-11-11 11:40:42 +0000 @@ -145,6 +145,27 @@ NdbRecAttr::receive_data(const Uint32 * const unsigned char* data = (const unsigned char*)data32; if(sz) { + /* Bug 39645 - correct for Disk Var type mapping to Fixed */ + if (unlikely(m_column->getStorageType() == NDB_STORAGETYPE_DISK)) + { + Uint32 oldSz= sz; + + switch (m_column->getType()) { + case NDB_TYPE_VARCHAR: + case NDB_TYPE_VARBINARY: + sz= data[0] + 1; + break; + case NDB_TYPE_LONGVARCHAR: + case NDB_TYPE_LONGVARBINARY: + sz= data[0] + (data[1] << 8) + 2; + break; + default: + ; + } + assert( sz <= oldSz ); + } + /* End of Bug 39645 */ + if (unlikely(m_getVarValue != NULL)) { // ONLY for blob V2 implementation assert(m_column->getType() == NdbDictionary::Column::Longvarchar || @@ -157,6 +178,7 @@ NdbRecAttr::receive_data(const Uint32 * data += 2; sz -= 2; } + if(!copyoutRequired()) memcpy(theRef, data, sz); else === modified file 'storage/ndb/test/src/NDBT_Tables.cpp' --- a/storage/ndb/test/src/NDBT_Tables.cpp 2008-03-25 14:30:41 +0000 +++ b/storage/ndb/test/src/NDBT_Tables.cpp 2008-11-11 11:40:42 +0000 @@ -411,6 +411,9 @@ NDBT_Attribute D2Attribs[] = { NDBT_Attribute("KOL4", NdbDictionary::Column::Varbinary, 133, false, true, 0, MM, true), NDBT_Attribute("KOL5", NdbDictionary::Column::Char, 199, false, true, 0, NdbDictionary::Column::StorageTypeDisk), NDBT_Attribute("KOL6", NdbDictionary::Column::Bit, 21, false, false, 0, NdbDictionary::Column::StorageTypeDisk), + NDBT_Attribute("KOL7", NdbDictionary::Column::Varbinary, 88, false, true, 0, NdbDictionary::Column::StorageTypeDisk), + NDBT_Attribute("KOL8", NdbDictionary::Column::Longvarbinary, 270, false, true, 0, NdbDictionary::Column::StorageTypeDisk) + }; static const