From 11560ecb2317faa22e44b96a259fa9ef7301e8c7 Mon Sep 17 00:00:00 2001 From: Greg Eisenhauer Date: Fri, 12 Jan 2024 19:48:35 -0500 Subject: [PATCH] Backport recent ASAN fixes to 2.10 release branch (#3985) * BP5internal - Switch to name based record lookup * Fix Var reference * ASAN fixes * kill comment --- CTestConfig.cmake | 12 --------- scripts/ci/cmake/adios-asan.supp | 4 ++- .../toolkit/format/bp5/BP5Deserializer.cpp | 2 ++ .../toolkit/format/bp5/BP5Serializer.cpp | 27 ++++++++++++------- .../adios2/toolkit/format/bp5/BP5Serializer.h | 2 +- source/adios2/toolkit/remote/Remote.cpp | 13 +++++++-- source/adios2/toolkit/remote/Remote.h | 3 ++- source/adios2/toolkit/remote/remote_common.h | 1 + source/adios2/toolkit/sst/cp/cp_reader.c | 1 + 9 files changed, 39 insertions(+), 26 deletions(-) diff --git a/CTestConfig.cmake b/CTestConfig.cmake index dae4b567bc..be9c516a6a 100644 --- a/CTestConfig.cmake +++ b/CTestConfig.cmake @@ -14,16 +14,4 @@ set(MEMORYCHECK_SUPPRESSIONS_FILE ${CMAKE_SOURCE_DIR}/scripts/dashboard/nightly/ # Ignore tests that are currently failing, remove tests here as they are fixed list(APPEND CTEST_CUSTOM_MEMCHECK_IGNORE -Engine.BP.BPWriteReadAsStreamTestADIOS2.ReaderWriterDefineVariable.BP5.Serial -Remote.BPWriteReadADIOS2stdio.GetRemote -Remote.BPWriteMemorySelectionRead.GetRemote -Remote.BPWriteMemorySelectionRead.FileRemote -remoteServerCleanup -Engine.SST.SstWriteFails.InvalidBeginStep.Serial -Staging.1x1.Local2.CommMin.BP5.SST -Staging.1x1Struct.CommMin.BP5.SST -Staging.WriteMemorySelectionRead.1x1.CommMin.BP5.SST -Staging.1x1.Local2.CommMin.BP.SST -Staging.WriteMemorySelectionRead.1x1.CommMin.BP.SST -Staging.1x1Struct.BP5 ) diff --git a/scripts/ci/cmake/adios-asan.supp b/scripts/ci/cmake/adios-asan.supp index d36b448f8d..9f9184380e 100644 --- a/scripts/ci/cmake/adios-asan.supp +++ b/scripts/ci/cmake/adios-asan.supp @@ -1,2 +1,4 @@ leak:ps_make_timer_name_ -leak:ibv_get_device_list \ No newline at end of file +leak:ibv_get_device_list +leak:add_transport_to_cm +leak:INT_CMadd_delayed_task diff --git a/source/adios2/toolkit/format/bp5/BP5Deserializer.cpp b/source/adios2/toolkit/format/bp5/BP5Deserializer.cpp index 9df8d55239..0e7574a17c 100644 --- a/source/adios2/toolkit/format/bp5/BP5Deserializer.cpp +++ b/source/adios2/toolkit/format/bp5/BP5Deserializer.cpp @@ -1996,6 +1996,8 @@ BP5Deserializer::~BP5Deserializer() free(VarRec.second->VarName); if (VarRec.second->Operator) free(VarRec.second->Operator); + if (VarRec.second->Def) + delete VarRec.second->Def; delete VarRec.second; } if (m_FreeableMBA) diff --git a/source/adios2/toolkit/format/bp5/BP5Serializer.cpp b/source/adios2/toolkit/format/bp5/BP5Serializer.cpp index 2ff1e5e802..2f618089dc 100644 --- a/source/adios2/toolkit/format/bp5/BP5Serializer.cpp +++ b/source/adios2/toolkit/format/bp5/BP5Serializer.cpp @@ -37,14 +37,16 @@ namespace format BP5Serializer::BP5Serializer() { Init(); } BP5Serializer::~BP5Serializer() { - if (!Info.RecMap.empty()) + if (CurDataBuffer) + delete CurDataBuffer; + if (!Info.RecNameMap.empty()) { - for (auto &rec : Info.RecMap) + for (auto &rec : Info.RecNameMap) { if (rec.second.OperatorType) free(rec.second.OperatorType); } - Info.RecMap.clear(); + Info.RecNameMap.clear(); } if (Info.MetaFieldCount) free_FMfield_list(Info.MetaFields); @@ -82,12 +84,13 @@ void BP5Serializer::Init() ((BP5MetadataInfoStruct *)MetadataBuf)->BitField = (std::size_t *)malloc(sizeof(size_t)); ((BP5MetadataInfoStruct *)MetadataBuf)->DataBlockSize = 0; } -BP5Serializer::BP5WriterRec BP5Serializer::LookupWriterRec(void *Key) const +BP5Serializer::BP5WriterRec BP5Serializer::LookupWriterRec(void *Variable) const { - auto it = Info.RecMap.find(Key); - if (it != Info.RecMap.end()) + core::VariableBase *VB = static_cast(Variable); + auto it2 = Info.RecNameMap.find(VB->m_Name); + if (it2 != Info.RecNameMap.end()) { - return const_cast(&(it->second)); + return const_cast(&(it2->second)); } return NULL; } @@ -467,6 +470,8 @@ void BP5Serializer::AddDoubleArrayField(FMFieldList *FieldP, int *CountP, const void BP5Serializer::ValidateWriterRec(BP5Serializer::BP5WriterRec Rec, void *Variable) { core::VariableBase *VB = static_cast(Variable); + + Rec->Key = Variable; // reset this, because Variable might have been destroyed and recreated if ((VB->m_Operations.size() == 0) && Rec->OperatorType) { // removed operator case @@ -505,7 +510,7 @@ BP5Serializer::BP5WriterRec BP5Serializer::CreateWriterRec(void *Variable, const #ifdef ADIOS2_HAVE_DERIVED_VARIABLE core::VariableDerived *VD = dynamic_cast(VB); #endif - auto obj = Info.RecMap.insert(std::make_pair(Variable, _BP5WriterRec())); + auto obj = Info.RecNameMap.insert(std::make_pair(VB->m_Name, _BP5WriterRec())); BP5WriterRec Rec = &obj.first->second; if (Type == DataType::String) ElemSize = sizeof(char *); @@ -550,6 +555,8 @@ BP5Serializer::BP5WriterRec BP5Serializer::CreateWriterRec(void *Variable, const struct_list[0].struct_size = (int)SD->StructSize(); FMFormat Format = register_data_format(Info.LocalFMContext, &struct_list[0]); + free_FMfield_list(List); + free((void *)struct_list[0].format_name); int IDLength; char *ServerID = get_server_ID_FMformat(Format, &IDLength); @@ -636,6 +643,8 @@ BP5Serializer::BP5WriterRec BP5Serializer::CreateWriterRec(void *Variable, const // Changing the formats renders these invalid Info.MetaFormat = NULL; } + if (TextStructID) + free((void *)TextStructID); Info.RecCount++; return Rec; } @@ -1250,7 +1259,7 @@ BufferV *BP5Serializer::ReinitStepData(BufferV *DataBuffer, bool forceCopyDeferr void BP5Serializer::CollectFinalShapeValues() { - for (auto it : Info.RecMap) + for (auto it : Info.RecNameMap) { BP5WriterRec Rec = &it.second; if (Rec->Shape == ShapeID::GlobalArray) diff --git a/source/adios2/toolkit/format/bp5/BP5Serializer.h b/source/adios2/toolkit/format/bp5/BP5Serializer.h index d07aa727fa..bd5830cfc1 100644 --- a/source/adios2/toolkit/format/bp5/BP5Serializer.h +++ b/source/adios2/toolkit/format/bp5/BP5Serializer.h @@ -167,7 +167,7 @@ class BP5Serializer : virtual public BP5Base FMFormat AttributeFormat = NULL; void *AttributeData = NULL; int AttributeSize = 0; - std::unordered_map RecMap; + std::unordered_map RecNameMap; }; FMFormat GenericAttributeFormat = NULL; diff --git a/source/adios2/toolkit/remote/Remote.cpp b/source/adios2/toolkit/remote/Remote.cpp index 3a397b05a9..29c0da3184 100644 --- a/source/adios2/toolkit/remote/Remote.cpp +++ b/source/adios2/toolkit/remote/Remote.cpp @@ -15,6 +15,12 @@ namespace adios2 Remote::Remote() {} #ifdef ADIOS2_HAVE_SST +Remote::~Remote() +{ + if (m_conn) + CMConnection_close(m_conn); +} + void OpenResponseHandler(CManager cm, CMConnection conn, void *vevent, void *client_data, attr_list attrs) { @@ -86,9 +92,10 @@ void Remote::Open(const std::string hostname, const int32_t port, const std::str atom_t CM_IP_HOSTNAME = -1; CM_IP_HOSTNAME = attr_atom_from_string("IP_HOST"); CM_IP_PORT = attr_atom_from_string("IP_PORT"); - add_attr(contact_list, CM_IP_HOSTNAME, Attr_String, (attr_value)hostname.c_str()); + add_attr(contact_list, CM_IP_HOSTNAME, Attr_String, (attr_value)strdup(hostname.c_str())); add_attr(contact_list, CM_IP_PORT, Attr_Int4, (attr_value)port); m_conn = CMinitiate_conn(ev_state.cm, contact_list); + free_attr_list(contact_list); if (!m_conn) return; @@ -124,9 +131,10 @@ void Remote::OpenSimpleFile(const std::string hostname, const int32_t port, atom_t CM_IP_HOSTNAME = -1; CM_IP_HOSTNAME = attr_atom_from_string("IP_HOST"); CM_IP_PORT = attr_atom_from_string("IP_PORT"); - add_attr(contact_list, CM_IP_HOSTNAME, Attr_String, (attr_value)hostname.c_str()); + add_attr(contact_list, CM_IP_HOSTNAME, Attr_String, (attr_value)strdup(hostname.c_str())); add_attr(contact_list, CM_IP_PORT, Attr_Int4, (attr_value)port); m_conn = CMinitiate_conn(ev_state.cm, contact_list); + free_attr_list(contact_list); if (!m_conn) return; @@ -193,5 +201,6 @@ Remote::GetHandle Remote::Read(size_t Start, size_t Size, void *Dest) { return static_cast(0); }; +Remote::~Remote() {} #endif } // end namespace adios2 diff --git a/source/adios2/toolkit/remote/Remote.h b/source/adios2/toolkit/remote/Remote.h index 4e4f24f579..9e432516fa 100644 --- a/source/adios2/toolkit/remote/Remote.h +++ b/source/adios2/toolkit/remote/Remote.h @@ -32,6 +32,7 @@ class Remote * @param comm passed to m_Comm */ Remote(); + ~Remote(); explicit operator bool() const { return m_Active; } @@ -55,7 +56,7 @@ class Remote #ifdef ADIOS2_HAVE_SST void InitCMData(); RemoteCommon::Remote_evpath_state ev_state; - CMConnection m_conn; + CMConnection m_conn = NULL; std::mutex m_CMInitMutex; #endif bool m_Active = false; diff --git a/source/adios2/toolkit/remote/remote_common.h b/source/adios2/toolkit/remote/remote_common.h index fc75b4a1f0..0d78bd290a 100644 --- a/source/adios2/toolkit/remote/remote_common.h +++ b/source/adios2/toolkit/remote/remote_common.h @@ -95,6 +95,7 @@ typedef struct _CloseFileMsg typedef struct _KillServerMsg { int KillResponseCondition; + size_t unused; // small messages call stack addressing issues? } *KillServerMsg; typedef struct _KillResponseMsg diff --git a/source/adios2/toolkit/sst/cp/cp_reader.c b/source/adios2/toolkit/sst/cp/cp_reader.c index ed58f0da42..13a504f07b 100644 --- a/source/adios2/toolkit/sst/cp/cp_reader.c +++ b/source/adios2/toolkit/sst/cp/cp_reader.c @@ -1221,6 +1221,7 @@ static void waitForMetadataWithTimeout(SstStream Stream, float timeout_secs) if (timercmp(&now, &end, >)) { CP_verbose(Stream, PerRankVerbose, "Returning from wait after timing out\n"); + free(TimeoutTask); return; } /* wait until we get the timestep metadata or something else changes */