Skip to content

Commit

Permalink
HDF5 multidim: fix crash when extracting a field from a compound HDF5…
Browse files Browse the repository at this point in the history
… data type
  • Loading branch information
rouault committed Apr 29, 2021
1 parent 6bcd74a commit 20882b4
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 13 deletions.
5 changes: 4 additions & 1 deletion autotest/gdrivers/hdf5multidim.py
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,9 @@ def test_hdf5_multidim_var_alldatatypes():
assert len(data) == 2 * 8
assert struct.unpack('ihihh', data) == (1, 2, 3, 4, 0)

assert struct.unpack('i' * 2, var['x'].Read()) == (1, 3)
assert struct.unpack('h' * 2, var['y'].Read()) == (2, 4)

var = rg.OpenMDArray('custom_type_3_elts_var')
dt = var.GetDataType()
assert dt.GetClass() == gdal.GEDTC_COMPOUND
Expand Down Expand Up @@ -396,7 +399,7 @@ def test_hdf5_netcdf_dimensions():
rg = ds.GetRootGroup()

assert rg.GetAttribute('CDI')

dims = rg.GetDimensions()
assert len(dims) == 3

Expand Down
71 changes: 59 additions & 12 deletions gdal/frmts/hdf5/hdf5multidim.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1470,12 +1470,44 @@ static void FreeDynamicMemory(GByte* pabyPtr, hid_t hDataType)
}
}

/************************************************************************/
/* CreateMapTargetComponentsToSrc() */
/************************************************************************/

static std::vector<unsigned> CreateMapTargetComponentsToSrc(
hid_t hSrcDataType, const GDALExtendedDataType& dstDataType)
{
CPLAssert( H5Tget_class(hSrcDataType) == H5T_COMPOUND );
CPLAssert( dstDataType.GetClass() == GEDTC_COMPOUND );

const unsigned nMembers = H5Tget_nmembers(hSrcDataType);
std::map<std::string, unsigned> oMapSrcCompNameToIdx;
for(unsigned i = 0; i < nMembers; i++ )
{
char* pszName = H5Tget_member_name(hSrcDataType, i);
if( pszName )
oMapSrcCompNameToIdx[pszName] = i;
}

std::vector<unsigned> ret;
const auto& comps = dstDataType.GetComponents();
ret.reserve(comps.size());
for( const auto& comp: comps )
{
auto oIter = oMapSrcCompNameToIdx.find(comp->GetName());
CPLAssert( oIter != oMapSrcCompNameToIdx.end() );
ret.emplace_back(oIter->second);
}
return ret;
}

/************************************************************************/
/* CopyValue() */
/************************************************************************/

static void CopyValue(const GByte* pabySrcBuffer, hid_t hSrcDataType,
GByte* pabyDstBuffer, const GDALExtendedDataType& dstDataType)
GByte* pabyDstBuffer, const GDALExtendedDataType& dstDataType,
const std::vector<unsigned>& mapDstCompsToSrcComps)
{
const auto klass = H5Tget_class(hSrcDataType);
if( klass == H5T_STRING )
Expand Down Expand Up @@ -1503,7 +1535,6 @@ static void CopyValue(const GByte* pabySrcBuffer, hid_t hSrcDataType,
}
else if( klass == H5T_COMPOUND )
{
const unsigned nMembers = H5Tget_nmembers(hSrcDataType);
if( dstDataType.GetClass() != GEDTC_COMPOUND )
{
// Typically source is complex data type
Expand All @@ -1519,22 +1550,24 @@ static void CopyValue(const GByte* pabySrcBuffer, hid_t hSrcDataType,
else
{
const auto& comps = dstDataType.GetComponents();
CPLAssert( nMembers == comps.size() );
for( unsigned i = 0; i < nMembers; i++ )
CPLAssert( comps.size() == mapDstCompsToSrcComps.size() );
const std::vector<unsigned> empty;
for( size_t iDst = 0; iDst < comps.size(); ++iDst )
{
auto hMemberType = H5Tget_member_type(hSrcDataType, i);
CopyValue( pabySrcBuffer + H5Tget_member_offset(hSrcDataType, i),
hMemberType,
pabyDstBuffer + comps[i]->GetOffset(),
comps[i]->GetType() );
const unsigned iSrc = mapDstCompsToSrcComps[iDst];
auto hMemberType = H5Tget_member_type(hSrcDataType, iSrc);
CopyValue( pabySrcBuffer + H5Tget_member_offset(hSrcDataType, iSrc),
hMemberType,
pabyDstBuffer + comps[iDst]->GetOffset(),
comps[iDst]->GetType(), empty );
H5Tclose(hMemberType);
}
}
}
else if( klass == H5T_ENUM )
{
auto hParent = H5Tget_super(hSrcDataType);
CopyValue(pabySrcBuffer, hParent, pabyDstBuffer, dstDataType);
CopyValue(pabySrcBuffer, hParent, pabyDstBuffer, dstDataType, {});
H5Tclose(hParent);
}
else
Expand Down Expand Up @@ -1591,12 +1624,19 @@ static void CopyToFinalBuffer(void* pDstBuffer,
const GByte* pabySrcBuffer = static_cast<const GByte*>(pTemp);
pabyDstBufferStack[0] = static_cast<GByte*>(pDstBuffer);
size_t iDim = 0;
const auto mapDstCompsToSrcComps =
(H5Tget_class(hSrcDataType) == H5T_COMPOUND &&
bufferDataType.GetClass() == GEDTC_COMPOUND) ?
CreateMapTargetComponentsToSrc(hSrcDataType, bufferDataType) :
std::vector<unsigned>();

lbl_next_depth:
if( iDim == nDims )
{
CopyValue(
pabySrcBuffer, hSrcDataType,
pabyDstBufferStack[nDims], bufferDataType);
pabyDstBufferStack[nDims], bufferDataType,
mapDstCompsToSrcComps);
}
else
{
Expand Down Expand Up @@ -1852,6 +1892,12 @@ static void CopyAllAttrValuesInto(size_t nDims,
std::vector<size_t> anStackCount(nDims);
std::vector<const GByte*> pabySrcBufferStack(nDims + 1);
std::vector<GByte*> pabyDstBufferStack(nDims + 1);
const auto mapDstCompsToSrcComps =
(H5Tget_class(hSrcBufferType) == H5T_COMPOUND &&
bufferDataType.GetClass() == GEDTC_COMPOUND) ?
CreateMapTargetComponentsToSrc(hSrcBufferType, bufferDataType) :
std::vector<unsigned>();

pabySrcBufferStack[0] = static_cast<const GByte*>(pabySrcBuffer);
if( nDims > 0 )
pabySrcBufferStack[0] += arrayStartIdx[0] * nSrcDataTypeSize;
Expand All @@ -1862,7 +1908,8 @@ static void CopyAllAttrValuesInto(size_t nDims,
{
CopyValue(
pabySrcBufferStack[nDims], hSrcBufferType,
pabyDstBufferStack[nDims], bufferDataType);
pabyDstBufferStack[nDims], bufferDataType,
mapDstCompsToSrcComps);
}
else
{
Expand Down

0 comments on commit 20882b4

Please sign in to comment.