Skip to content

Commit

Permalink
Backport recent ASAN fixes to 2.10 release branch (#3985)
Browse files Browse the repository at this point in the history
* BP5internal - Switch to name based record lookup

* Fix Var reference

* ASAN fixes

* kill comment
  • Loading branch information
eisenhauer committed Jan 13, 2024
1 parent 5110209 commit 11560ec
Show file tree
Hide file tree
Showing 9 changed files with 39 additions and 26 deletions.
12 changes: 0 additions & 12 deletions CTestConfig.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
4 changes: 3 additions & 1 deletion scripts/ci/cmake/adios-asan.supp
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@
leak:ps_make_timer_name_
leak:ibv_get_device_list
leak:ibv_get_device_list
leak:add_transport_to_cm
leak:INT_CMadd_delayed_task
2 changes: 2 additions & 0 deletions source/adios2/toolkit/format/bp5/BP5Deserializer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
27 changes: 18 additions & 9 deletions source/adios2/toolkit/format/bp5/BP5Serializer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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<core::VariableBase *>(Variable);
auto it2 = Info.RecNameMap.find(VB->m_Name);
if (it2 != Info.RecNameMap.end())
{
return const_cast<BP5WriterRec>(&(it->second));
return const_cast<BP5WriterRec>(&(it2->second));
}
return NULL;
}
Expand Down Expand Up @@ -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<core::VariableBase *>(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
Expand Down Expand Up @@ -505,7 +510,7 @@ BP5Serializer::BP5WriterRec BP5Serializer::CreateWriterRec(void *Variable, const
#ifdef ADIOS2_HAVE_DERIVED_VARIABLE
core::VariableDerived *VD = dynamic_cast<core::VariableDerived *>(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 *);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion source/adios2/toolkit/format/bp5/BP5Serializer.h
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ class BP5Serializer : virtual public BP5Base
FMFormat AttributeFormat = NULL;
void *AttributeData = NULL;
int AttributeSize = 0;
std::unordered_map<void *, _BP5WriterRec> RecMap;
std::unordered_map<std::string, _BP5WriterRec> RecNameMap;
};

FMFormat GenericAttributeFormat = NULL;
Expand Down
13 changes: 11 additions & 2 deletions source/adios2/toolkit/remote/Remote.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -193,5 +201,6 @@ Remote::GetHandle Remote::Read(size_t Start, size_t Size, void *Dest)
{
return static_cast<GetHandle>(0);
};
Remote::~Remote() {}
#endif
} // end namespace adios2
3 changes: 2 additions & 1 deletion source/adios2/toolkit/remote/Remote.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ class Remote
* @param comm passed to m_Comm
*/
Remote();
~Remote();

explicit operator bool() const { return m_Active; }

Expand All @@ -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;
Expand Down
1 change: 1 addition & 0 deletions source/adios2/toolkit/remote/remote_common.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions source/adios2/toolkit/sst/cp/cp_reader.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down

0 comments on commit 11560ec

Please sign in to comment.