From a1672d1ae0dd115219849584d6b807a66ea7746d Mon Sep 17 00:00:00 2001 From: Jason Wang Date: Tue, 3 May 2022 20:50:05 -0400 Subject: [PATCH] make struct size parameter compulsory to ensure padding correctness --- bindings/CXX11/adios2/cxx11/ADIOS.cpp | 4 ++-- bindings/CXX11/adios2/cxx11/ADIOS.h | 2 +- source/adios2/core/ADIOS.cpp | 6 ++++-- source/adios2/core/ADIOS.h | 2 +- source/adios2/core/IO.cpp | 6 +++--- source/adios2/core/IO.h | 2 +- source/adios2/core/VariableStruct.cpp | 7 +++---- source/adios2/core/VariableStruct.h | 2 +- source/adios2/engine/ssc/SscHelper.cpp | 3 +-- testing/adios2/engine/ssc/TestSscStruct.cpp | 10 +++++----- testing/adios2/interface/TestADIOSDefineVariable.cpp | 4 ++-- 11 files changed, 24 insertions(+), 24 deletions(-) diff --git a/bindings/CXX11/adios2/cxx11/ADIOS.cpp b/bindings/CXX11/adios2/cxx11/ADIOS.cpp index 9c4e00ad08..3b39dbb6dd 100644 --- a/bindings/CXX11/adios2/cxx11/ADIOS.cpp +++ b/bindings/CXX11/adios2/cxx11/ADIOS.cpp @@ -85,11 +85,11 @@ Operator ADIOS::InquireOperator(const std::string name) } } -StructDefinition ADIOS::DefineStruct(const std::string &name) +StructDefinition ADIOS::DefineStruct(const std::string &name, const size_t size) { CheckPointer("for struct name " + name + ", in call to ADIOS::DefineStruct"); - return StructDefinition(&m_ADIOS->DefineStruct(name)); + return StructDefinition(&m_ADIOS->DefineStruct(name, size)); } StructDefinition ADIOS::InquireStruct(const std::string &name) diff --git a/bindings/CXX11/adios2/cxx11/ADIOS.h b/bindings/CXX11/adios2/cxx11/ADIOS.h index 108d45e842..72a3c4173c 100644 --- a/bindings/CXX11/adios2/cxx11/ADIOS.h +++ b/bindings/CXX11/adios2/cxx11/ADIOS.h @@ -214,7 +214,7 @@ class ADIOS */ Operator InquireOperator(const std::string name); - StructDefinition DefineStruct(const std::string &name); + StructDefinition DefineStruct(const std::string &name, const size_t size); StructDefinition InquireStruct(const std::string &name); /** diff --git a/source/adios2/core/ADIOS.cpp b/source/adios2/core/ADIOS.cpp index cced6c3d6c..27d2a0342f 100644 --- a/source/adios2/core/ADIOS.cpp +++ b/source/adios2/core/ADIOS.cpp @@ -194,7 +194,8 @@ ADIOS::InquireOperator(const std::string &name) noexcept } } -StructDefinition &ADIOS::DefineStruct(const std::string &name) +StructDefinition &ADIOS::DefineStruct(const std::string &name, + const size_t size) { if (m_StructDefinitions.find(name) != m_StructDefinitions.end()) { @@ -202,7 +203,8 @@ StructDefinition &ADIOS::DefineStruct(const std::string &name) "Struct " + name + " defined twice"); } - return m_StructDefinitions[name]; + return m_StructDefinitions.emplace(name, StructDefinition(size)) + .first->second; } StructDefinition *ADIOS::InquireStruct(const std::string &name) diff --git a/source/adios2/core/ADIOS.h b/source/adios2/core/ADIOS.h index 29a89e4be1..df443e4f17 100644 --- a/source/adios2/core/ADIOS.h +++ b/source/adios2/core/ADIOS.h @@ -132,7 +132,7 @@ class ADIOS std::pair * InquireOperator(const std::string &name) noexcept; - StructDefinition &DefineStruct(const std::string &name); + StructDefinition &DefineStruct(const std::string &name, const size_t size); StructDefinition *InquireStruct(const std::string &name); /** diff --git a/source/adios2/core/IO.cpp b/source/adios2/core/IO.cpp index b18a4bb80a..c63a284295 100644 --- a/source/adios2/core/IO.cpp +++ b/source/adios2/core/IO.cpp @@ -964,10 +964,10 @@ VariableStruct *IO::InquireStructVariable(const std::string &name, return ret; } -StructDefinition &IO::DefineStruct(const std::string &name) +StructDefinition &IO::DefineStruct(const std::string &name, const size_t size) { - m_StructDefinitions[name] = StructDefinition(); - return m_StructDefinitions[name]; + return m_StructDefinitions.emplace(name, StructDefinition(size)) + .first->second; } StructDefinition *IO::InquireStruct(const std::string &name) diff --git a/source/adios2/core/IO.h b/source/adios2/core/IO.h index 174a4a55ef..45e2c51de7 100644 --- a/source/adios2/core/IO.h +++ b/source/adios2/core/IO.h @@ -505,7 +505,7 @@ class IO /** Inform about computation block through User->ADIOS */ void ExitComputationBlock() noexcept; - StructDefinition &DefineStruct(const std::string &name); + StructDefinition &DefineStruct(const std::string &name, const size_t size); StructDefinition *InquireStruct(const std::string &name); private: diff --git a/source/adios2/core/VariableStruct.cpp b/source/adios2/core/VariableStruct.cpp index f3abef2784..1e5148b09f 100644 --- a/source/adios2/core/VariableStruct.cpp +++ b/source/adios2/core/VariableStruct.cpp @@ -17,7 +17,7 @@ namespace adios2 namespace core { -void StructDefinition::SetStructSize(const size_t size) { m_StructSize = size; } +StructDefinition::StructDefinition(const size_t size) : m_StructSize(size) {} void StructDefinition::AddItem(const std::string &name, const size_t offset, const DataType type, const size_t size) @@ -28,11 +28,11 @@ void StructDefinition::AddItem(const std::string &name, const size_t offset, "VariableStruct::StructDefinition", "AddItem", "invalid data type"); } - if (offset < m_StructSize) + if (offset + helper::GetDataTypeSize(type) * size > m_StructSize) { helper::Throw("core", "VariableStruct::StructDefinition", - "AddItem", "invalid offset"); + "AddItem", "exceeded struct size"); } m_Definition.emplace_back(); auto &d = m_Definition.back(); @@ -40,7 +40,6 @@ void StructDefinition::AddItem(const std::string &name, const size_t offset, d.Offset = offset; d.Type = type; d.Size = size; - m_StructSize = offset + helper::GetDataTypeSize(type) * size; } size_t StructDefinition::StructSize() const noexcept { return m_StructSize; } diff --git a/source/adios2/core/VariableStruct.h b/source/adios2/core/VariableStruct.h index 1c4cf97cdd..690692503c 100644 --- a/source/adios2/core/VariableStruct.h +++ b/source/adios2/core/VariableStruct.h @@ -30,7 +30,7 @@ class StructDefinition size_t Size; }; - void SetStructSize(const size_t size); + StructDefinition(const size_t size); void AddItem(const std::string &name, const size_t offset, const DataType type, const size_t size = 1); size_t StructSize() const noexcept; diff --git a/source/adios2/engine/ssc/SscHelper.cpp b/source/adios2/engine/ssc/SscHelper.cpp index f29b637c40..f081583094 100644 --- a/source/adios2/engine/ssc/SscHelper.cpp +++ b/source/adios2/engine/ssc/SscHelper.cpp @@ -348,8 +348,7 @@ void DeserializeVariable(const Buffer &input, const ShapeID shapeId, } if (b.shapeId == ShapeID::GlobalArray) { - auto def = io.DefineStruct(b.name); - def.SetStructSize(b.elementSize); + auto def = io.DefineStruct(b.name, b.elementSize); io.DefineStructVariable(b.name, def, vShape, vStart, vShape); } diff --git a/testing/adios2/engine/ssc/TestSscStruct.cpp b/testing/adios2/engine/ssc/TestSscStruct.cpp index e02caf664d..2f80709e75 100644 --- a/testing/adios2/engine/ssc/TestSscStruct.cpp +++ b/testing/adios2/engine/ssc/TestSscStruct.cpp @@ -63,16 +63,16 @@ void Writer(const Dims &shape, const Dims &start, const Dims &count, auto varIntScalar = io.DefineVariable("varIntScalar"); auto varString = io.DefineVariable("varString"); - auto particleDef = adios.DefineStruct("particle"); - particleDef.AddItem("a", 0, adios2::DataType::Int8, 1); - particleDef.AddItem("b", 4, adios2::DataType::Int32, 4); - auto varStruct = - io.DefineStructVariable("particles", particleDef, shape, start, count); struct particle { int8_t a; int b[4]; }; + auto particleDef = adios.DefineStruct("particle", sizeof(particle)); + particleDef.AddItem("a", 0, adios2::DataType::Int8, 1); + particleDef.AddItem("b", 4, adios2::DataType::Int32, 4); + auto varStruct = + io.DefineStructVariable("particles", particleDef, shape, start, count); std::vector myParticles(datasize); for (size_t i = 0; i < datasize; ++i) { diff --git a/testing/adios2/interface/TestADIOSDefineVariable.cpp b/testing/adios2/interface/TestADIOSDefineVariable.cpp index 75c80355f4..e1c73fdba2 100644 --- a/testing/adios2/interface/TestADIOSDefineVariable.cpp +++ b/testing/adios2/interface/TestADIOSDefineVariable.cpp @@ -661,11 +661,11 @@ TEST_F(ADIOSDefineVariableTest, DefineStructVariable) const adios2::Dims start = {0}; const adios2::Dims count = {10}; - auto def1 = adios.DefineStruct("def1"); + auto def1 = adios.DefineStruct("def1", 24); def1.AddItem("a", 0, adios2::DataType::Int8, 1); def1.AddItem("b", 4, adios2::DataType::Int32, 5); - auto def2 = adios.DefineStruct("def2"); + auto def2 = adios.DefineStruct("def2", 28); def2.AddItem("a", 0, adios2::DataType::Int8, 1); def2.AddItem("b", 4, adios2::DataType::Int32, 5); def2.AddItem("c", 24, adios2::DataType::Int32);