From 060afd34dbe70c56b69bb21e6cb6783e3bab371d Mon Sep 17 00:00:00 2001 From: "Robert R. Henry" Date: Wed, 10 Aug 2022 17:15:17 -0700 Subject: [PATCH 1/2] Add guard word before local var CMiniColDef[9] Initialize the guard word so it does not pass the UsesAllocatedMemory test. Found running output of clang14 -fsanitize=address, then inspection --- src/coreclr/md/runtime/metamodel.cpp | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/src/coreclr/md/runtime/metamodel.cpp b/src/coreclr/md/runtime/metamodel.cpp index 14654f7657b95..f5a91097a6598 100644 --- a/src/coreclr/md/runtime/metamodel.cpp +++ b/src/coreclr/md/runtime/metamodel.cpp @@ -717,11 +717,16 @@ CMiniMdBase::InitColsForTable( // should we write the data into the structure { const CMiniTableDef *pTemplate; // Template table definition. - CMiniColDef pCols[9]; // The col defs to init. + struct { + uint32_t holds_memory_marker; // a marker so UsesAllocateMemory knows it is not allocated + CMiniColDef pCols[9]; // The col defs to init. + } p; BYTE iOffset; // Running size of a record. BYTE iSize; // Size of a field. HRESULT hr = S_OK; + p.holds_memory_marker = ~((ALLOCATED_MEMORY_MARKER&0xff) * 0x01010101U); + _ASSERTE((bExtra == 0) || (bExtra == 1)); _ASSERTE(ARRAY_SIZE(pCols) >= pTable->m_cCols); @@ -737,18 +742,18 @@ CMiniMdBase::InitColsForTable( for (ULONG ixCol = 0; ixCol < pTable->m_cCols; ++ixCol) { // Initialize from the template values (type, maybe offset, size). - pCols[ixCol] = pTemplate->m_pColDefs[ixCol]; + p.pCols[ixCol] = pTemplate->m_pColDefs[ixCol]; // Is the field a RID into a table? - if (pCols[ixCol].m_Type <= iRidMax) + if (p.pCols[ixCol].m_Type <= iRidMax) { - iSize = cbRID(Schema.m_cRecs[pCols[ixCol].m_Type] << bExtra); + iSize = cbRID(Schema.m_cRecs[p.pCols[ixCol].m_Type] << bExtra); } else // Is the field a coded token? - if (pCols[ixCol].m_Type <= iCodedTokenMax) + if (p.pCols[ixCol].m_Type <= iCodedTokenMax) { - ULONG iCdTkn = pCols[ixCol].m_Type - iCodedToken; + ULONG iCdTkn = p.pCols[ixCol].m_Type - iCodedToken; ULONG cRecs = 0; _ASSERTE(iCdTkn < ARRAY_SIZE(g_CodedTokens)); @@ -773,7 +778,7 @@ CMiniMdBase::InitColsForTable( } else { // Fixed type. - switch (pCols[ixCol].m_Type) + switch (p.pCols[ixCol].m_Type) { #ifdef FEATURE_METADATA_EMIT_PORTABLE_PDB /* Portable PDB tables */ @@ -826,8 +831,8 @@ CMiniMdBase::InitColsForTable( } // Now save the size and offset. - pCols[ixCol].m_oColumn = iOffset; - pCols[ixCol].m_cbColumn = iSize; + p.pCols[ixCol].m_oColumn = iOffset; + p.pCols[ixCol].m_cbColumn = iSize; // Align to 2 bytes. iSize += iSize & 1; @@ -842,12 +847,12 @@ CMiniMdBase::InitColsForTable( // Can we write to the memory if (!fUsePointers) { - memcpy(pTable->m_pColDefs, pCols, sizeof(CMiniColDef)*pTable->m_cCols); + memcpy(pTable->m_pColDefs, p.pCols, sizeof(CMiniColDef)*pTable->m_cCols); } else { // We'll need to have pTable->m_pColDefs point to some data instead - hr = SetNewColumnDefinition(pTable, pCols, ixTbl); + hr = SetNewColumnDefinition(pTable, p.pCols, ixTbl); } // If no key, set to a distinct value. if (pTable->m_iKey >= pTable->m_cCols) From cccde5bbb1f58597148b59582614dbd509844587 Mon Sep 17 00:00:00 2001 From: "Robert R. Henry" Date: Thu, 11 Aug 2022 17:16:48 -0700 Subject: [PATCH 2/2] Simpolify asan-safe implementation of CMiniMdBasse:;InitColsForTable --- src/coreclr/md/runtime/metamodel.cpp | 41 +++++++++++++++------------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/src/coreclr/md/runtime/metamodel.cpp b/src/coreclr/md/runtime/metamodel.cpp index f5a91097a6598..7a27c4e85e5ba 100644 --- a/src/coreclr/md/runtime/metamodel.cpp +++ b/src/coreclr/md/runtime/metamodel.cpp @@ -717,24 +717,27 @@ CMiniMdBase::InitColsForTable( // should we write the data into the structure { const CMiniTableDef *pTemplate; // Template table definition. - struct { - uint32_t holds_memory_marker; // a marker so UsesAllocateMemory knows it is not allocated - CMiniColDef pCols[9]; // The col defs to init. - } p; + const int MaxCols = 9; + typedef uint32_t markword_t; + BYTE tempCols[sizeof(markword_t) + MaxCols * sizeof(CMiniColDef)]; // keep aligned + + _ASSERTE(MaxCols >= pTable->m_cCols); + // + // Mark the array of columns as not allocated (eg, not ALLOCATED_MEMORY_MARKER) + // for SetNewColumnDefinition call + // + memset(tempCols, 0, sizeof(markword_t)); + CMiniColDef *pCols = BYTEARRAY_TO_COLDES(tempCols + sizeof(markword_t) - 1); // The col defs to init. BYTE iOffset; // Running size of a record. BYTE iSize; // Size of a field. HRESULT hr = S_OK; - p.holds_memory_marker = ~((ALLOCATED_MEMORY_MARKER&0xff) * 0x01010101U); - _ASSERTE((bExtra == 0) || (bExtra == 1)); _ASSERTE(ARRAY_SIZE(pCols) >= pTable->m_cCols); bExtra = 0;//@FUTURE: save in schema header. until then use 0. - iOffset = 0; - - pTemplate = GetTableDefTemplate(ixTbl); + pTemplate = GetTableDefTemplate(ixTbl); PREFIX_ASSUME(pTemplate->m_pColDefs != NULL); @@ -742,18 +745,18 @@ CMiniMdBase::InitColsForTable( for (ULONG ixCol = 0; ixCol < pTable->m_cCols; ++ixCol) { // Initialize from the template values (type, maybe offset, size). - p.pCols[ixCol] = pTemplate->m_pColDefs[ixCol]; + pCols[ixCol] = pTemplate->m_pColDefs[ixCol]; // Is the field a RID into a table? - if (p.pCols[ixCol].m_Type <= iRidMax) + if (pCols[ixCol].m_Type <= iRidMax) { - iSize = cbRID(Schema.m_cRecs[p.pCols[ixCol].m_Type] << bExtra); + iSize = cbRID(Schema.m_cRecs[pCols[ixCol].m_Type] << bExtra); } else // Is the field a coded token? - if (p.pCols[ixCol].m_Type <= iCodedTokenMax) + if (pCols[ixCol].m_Type <= iCodedTokenMax) { - ULONG iCdTkn = p.pCols[ixCol].m_Type - iCodedToken; + ULONG iCdTkn = pCols[ixCol].m_Type - iCodedToken; ULONG cRecs = 0; _ASSERTE(iCdTkn < ARRAY_SIZE(g_CodedTokens)); @@ -778,7 +781,7 @@ CMiniMdBase::InitColsForTable( } else { // Fixed type. - switch (p.pCols[ixCol].m_Type) + switch (pCols[ixCol].m_Type) { #ifdef FEATURE_METADATA_EMIT_PORTABLE_PDB /* Portable PDB tables */ @@ -831,8 +834,8 @@ CMiniMdBase::InitColsForTable( } // Now save the size and offset. - p.pCols[ixCol].m_oColumn = iOffset; - p.pCols[ixCol].m_cbColumn = iSize; + pCols[ixCol].m_oColumn = iOffset; + pCols[ixCol].m_cbColumn = iSize; // Align to 2 bytes. iSize += iSize & 1; @@ -847,12 +850,12 @@ CMiniMdBase::InitColsForTable( // Can we write to the memory if (!fUsePointers) { - memcpy(pTable->m_pColDefs, p.pCols, sizeof(CMiniColDef)*pTable->m_cCols); + memcpy(pTable->m_pColDefs, pCols, sizeof(CMiniColDef)*pTable->m_cCols); } else { // We'll need to have pTable->m_pColDefs point to some data instead - hr = SetNewColumnDefinition(pTable, p.pCols, ixTbl); + hr = SetNewColumnDefinition(pTable, pCols, ixTbl); } // If no key, set to a distinct value. if (pTable->m_iKey >= pTable->m_cCols)